Browse Source

ref(saved-search): Use CompactSelect (#34239)

* ref(saved-search): Use DropdownMenuV2

* ref(saved-search): Use CompactSelect

* ref(saved-search): Remove redundant CSS

* fix(saved-search): Fix organizationSavedSearchMenu test
Vu Luong 2 years ago
parent
commit
1fe1dcaedb

+ 4 - 0
static/app/components/layouts/thirds.tsx

@@ -112,8 +112,12 @@ export const HeaderNavTabs = styled(NavTabs)`
     margin-right: ${space(3)};
   }
   & > li > a {
+    display: flex;
+    align-items: center;
+    height: 1.25rem;
     padding: ${space(1)} 0;
     margin-bottom: 4px;
+    box-sizing: content-box;
   }
   & > li.active > a {
     margin-bottom: 0;

+ 0 - 4
static/app/styles/global.tsx

@@ -104,10 +104,6 @@ const styles = (theme: Theme, isDark: boolean) => css`
           border-left-color: ${theme.purple300};
         }
 
-        .saved-search-tab {
-          border-bottom-color: ${theme.active} !important;
-        }
-
         .nav-tabs {
           & > li {
             &.active {

+ 1 - 1
static/app/types/core.tsx

@@ -48,7 +48,7 @@ export type SelectValue<T> = {
   label: string | number | React.ReactElement;
   value: T;
   disabled?: boolean;
-  tooltip?: string;
+  tooltip?: React.ReactNode;
 };
 
 /**

+ 0 - 238
static/app/views/issueList/savedSearchMenu.tsx

@@ -1,238 +0,0 @@
-import {Fragment} from 'react';
-import styled from '@emotion/styled';
-
-import Access from 'sentry/components/acl/access';
-import Button from 'sentry/components/button';
-import Confirm from 'sentry/components/confirm';
-import MenuItem from 'sentry/components/menuItem';
-import Tooltip from 'sentry/components/tooltip';
-import {IconDelete} from 'sentry/icons';
-import {t} from 'sentry/locale';
-import overflowEllipsis from 'sentry/styles/overflowEllipsis';
-import space from 'sentry/styles/space';
-import {Organization, SavedSearch} from 'sentry/types';
-
-import {getSortLabel} from './utils';
-
-type MenuItemProps = Omit<Props, 'savedSearchList'> & {
-  isLast: boolean;
-  search: SavedSearch;
-};
-
-function SavedSearchMenuItem({
-  organization,
-  onSavedSearchSelect,
-  onSavedSearchDelete,
-  search,
-  query,
-  sort,
-  isLast,
-}: MenuItemProps) {
-  return (
-    <Tooltip
-      title={
-        <Fragment>
-          {`${search.name} \u2022 `}
-          <TooltipSearchQuery>{search.query}</TooltipSearchQuery>
-          {` \u2022 `}
-          {t('Sort: ')}
-          {getSortLabel(search.sort)}
-        </Fragment>
-      }
-      containerDisplayMode="block"
-      delay={1000}
-    >
-      <StyledMenuItem
-        isActive={search.query === query && search.sort === sort}
-        isLast={isLast}
-        data-test-id={`saved-search-${search.id}`}
-      >
-        <MenuItemLink tabIndex={-1} onClick={() => onSavedSearchSelect(search)}>
-          <SearchTitle>{search.name}</SearchTitle>
-          <SearchQueryContainer>
-            <SearchQuery>{search.query}</SearchQuery>
-            <SearchSort>
-              {t('Sort: ')}
-              {getSortLabel(search.sort)}
-            </SearchSort>
-          </SearchQueryContainer>
-        </MenuItemLink>
-        {search.isGlobal === false && search.isPinned === false && (
-          <Access
-            organization={organization}
-            access={['org:write']}
-            renderNoAccessMessage={false}
-          >
-            <Confirm
-              onConfirm={() => onSavedSearchDelete(search)}
-              message={t('Are you sure you want to delete this saved search?')}
-              stopPropagation
-            >
-              <DeleteButton
-                borderless
-                title={t('Delete this saved search')}
-                icon={<IconDelete />}
-                aria-label={t('delete')}
-                size="zero"
-              />
-            </Confirm>
-          </Access>
-        )}
-      </StyledMenuItem>
-    </Tooltip>
-  );
-}
-
-type Props = {
-  onSavedSearchDelete: (savedSearch: SavedSearch) => void;
-  onSavedSearchSelect: (savedSearch: SavedSearch) => void;
-  organization: Organization;
-  savedSearchList: SavedSearch[];
-  sort: string;
-  query?: string;
-};
-
-function SavedSearchMenu({savedSearchList, ...props}: Props) {
-  const savedSearches = savedSearchList.filter(search => !search.isGlobal);
-  let globalSearches = savedSearchList.filter(search => search.isGlobal);
-  // Hide "Unresolved Issues" since they have a unresolved tab
-  globalSearches = globalSearches.filter(
-    search => !search.isPinned && search.query !== 'is:unresolved'
-  );
-
-  return (
-    <Fragment>
-      <MenuHeader>{t('Saved Searches')}</MenuHeader>
-      {savedSearches.length === 0 ? (
-        <EmptyItem>{t('No saved searches yet.')}</EmptyItem>
-      ) : (
-        savedSearches.map((search, index) => (
-          <SavedSearchMenuItem
-            key={search.id}
-            search={search}
-            isLast={index === savedSearches.length - 1}
-            {...props}
-          />
-        ))
-      )}
-      <SecondaryMenuHeader>{t('Recommended Searches')}</SecondaryMenuHeader>
-      {/* Could only happen on self-hosted */}
-      {globalSearches.length === 0 ? (
-        <EmptyItem>{t('No recommended searches yet.')}</EmptyItem>
-      ) : (
-        globalSearches.map((search, index) => (
-          <SavedSearchMenuItem
-            key={search.id}
-            search={search}
-            isLast={index === globalSearches.length - 1}
-            {...props}
-          />
-        ))
-      )}
-    </Fragment>
-  );
-}
-
-export default SavedSearchMenu;
-
-const SearchTitle = styled('div')`
-  color: ${p => p.theme.textColor};
-  ${overflowEllipsis}
-`;
-
-const SearchQueryContainer = styled('div')`
-  font-size: ${p => p.theme.fontSizeExtraSmall};
-  ${overflowEllipsis}
-`;
-
-const SearchQuery = styled('code')`
-  color: ${p => p.theme.subText};
-  font-size: ${p => p.theme.fontSizeExtraSmall};
-  padding: 0;
-  background: inherit;
-`;
-
-const SearchSort = styled('span')`
-  color: ${p => p.theme.subText};
-  font-size: ${p => p.theme.fontSizeExtraSmall};
-  &:before {
-    font-size: ${p => p.theme.fontSizeExtraSmall};
-    color: ${p => p.theme.textColor};
-    content: ' \u2022 ';
-  }
-`;
-
-const TooltipSearchQuery = styled('span')`
-  color: ${p => p.theme.subText};
-  font-weight: normal;
-  font-family: ${p => p.theme.text.familyMono};
-`;
-
-const DeleteButton = styled(Button)`
-  color: ${p => p.theme.gray200};
-  background: transparent;
-  flex-shrink: 0;
-  padding: ${space(1)} 0;
-
-  &:hover {
-    background: transparent;
-    color: ${p => p.theme.blue300};
-  }
-`;
-
-const MenuHeader = styled('div')`
-  align-items: center;
-  color: ${p => p.theme.gray400};
-  background: ${p => p.theme.backgroundSecondary};
-  line-height: 0.75;
-  padding: ${space(1.5)} ${space(2)};
-  border-bottom: 1px solid ${p => p.theme.innerBorder};
-  border-radius: ${p => p.theme.borderRadius} ${p => p.theme.borderRadius} 0 0;
-`;
-
-const SecondaryMenuHeader = styled(MenuHeader)`
-  border-top: 1px solid ${p => p.theme.innerBorder};
-  border-radius: 0;
-`;
-
-const StyledMenuItem = styled(MenuItem)<{isActive: boolean; isLast: boolean}>`
-  border-bottom: ${p => (!p.isLast ? `1px solid ${p.theme.innerBorder}` : null)};
-  font-size: ${p => p.theme.fontSizeMedium};
-
-  & > span {
-    padding: ${space(1)} ${space(2)};
-  }
-
-  ${p =>
-    p.isActive &&
-    `
-  ${SearchTitle}, ${SearchQuery}, ${SearchSort} {
-    color: ${p.theme.white};
-  }
-  ${SearchSort}:before {
-    color: ${p.theme.white};
-  }
-  &:hover {
-    ${SearchTitle}, ${SearchQuery}, ${SearchSort} {
-      color: ${p.theme.black};
-    }
-    ${SearchSort}:before {
-      color: ${p.theme.black};
-    }
-  }
-  `}
-`;
-
-const MenuItemLink = styled('a')`
-  display: block;
-  flex-grow: 1;
-  /* Nav tabs style override */
-  border: 0;
-
-  ${overflowEllipsis}
-`;
-
-const EmptyItem = styled('li')`
-  padding: ${space(1)} ${space(1.5)};
-  color: ${p => p.theme.subText};
-`;

+ 154 - 79
static/app/views/issueList/savedSearchTab.tsx

@@ -1,15 +1,22 @@
-import {Fragment} from 'react';
+import {Fragment, useCallback, useMemo} from 'react';
 import styled from '@emotion/styled';
 
+import Access from 'sentry/components/acl/access';
 import Badge from 'sentry/components/badge';
-import DropdownLink from 'sentry/components/dropdownLink';
+import Button from 'sentry/components/button';
+import Confirm from 'sentry/components/confirm';
+import DropdownButtonV2 from 'sentry/components/dropdownButtonV2';
+import CompactSelect from 'sentry/components/forms/compactSelect';
+import {ControlProps} from 'sentry/components/forms/selectControl';
 import QueryCount from 'sentry/components/queryCount';
+import {IconDelete} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import overflowEllipsis from 'sentry/styles/overflowEllipsis';
 import space from 'sentry/styles/space';
 import {Organization, SavedSearch} from 'sentry/types';
+import {defined} from 'sentry/utils';
 
-import SavedSearchMenu from './savedSearchMenu';
+import {getSortLabel} from './utils';
 
 type Props = {
   onSavedSearchDelete: (savedSearch: SavedSearch) => void;
@@ -35,111 +42,179 @@ function SavedSearchTab({
   const savedSearch = savedSearchList.find(
     search => search.query === query && search.sort === sort
   );
+  const savedSearchValue = savedSearch
+    ? savedSearch.isGlobal
+      ? `global-search-${savedSearch.id}`
+      : `saved-search-${savedSearch.id}`
+    : '';
 
-  const title = (
-    <TitleWrapper>
+  const options: ControlProps['options'] = useMemo(() => {
+    const savedSearches = savedSearchList.filter(search => !search.isGlobal);
+    const globalSearches = savedSearchList.filter(
+      search => search.isGlobal && !search.isPinned && search.query !== 'is:unresolved'
+    );
+
+    function getSearchOption(search, keyPrefix) {
+      return {
+        value: `${keyPrefix}-${search.id}`,
+        label: search.name,
+        details: (
+          <Details>
+            {search.query}
+            {` \u2022 ${t('Sort:')} ${getSortLabel(search.sort)}`}
+          </Details>
+        ),
+        tooltip: (
+          <Fragment>
+            {`${search.name} \u2022 `}
+            <TooltipSearchQuery>{search.query}</TooltipSearchQuery>
+            {` \u2022 `}
+            {t('Sort: ')}
+            {getSortLabel(search.sort)}
+          </Fragment>
+        ),
+        tooltipOptions: {delay: 1000},
+        ...(!search.isPinned &&
+          !search.isGlobal && {
+            trailingItems: (
+              <Access
+                organization={organization}
+                access={['org:write']}
+                renderNoAccessMessage={false}
+              >
+                <Confirm
+                  onConfirm={() => onSavedSearchDelete(search)}
+                  message={t('Are you sure you want to delete this saved search?')}
+                  stopPropagation
+                >
+                  <DeleteButton
+                    borderless
+                    icon={<IconDelete />}
+                    aria-label={t('delete')}
+                    size="zero"
+                  />
+                </Confirm>
+              </Access>
+            ),
+          }),
+        trailingItemsSpanFullHeight: true,
+      };
+    }
+
+    const searchOptions: ControlProps['options'] = [];
+
+    if (savedSearches.length > 0) {
+      searchOptions.push({
+        value: 'saved-searches',
+        label: t('Saved Searches'),
+        options: savedSearches.map(search => getSearchOption(search, 'saved-search')),
+      });
+    }
+
+    if (globalSearches.length > 0) {
+      searchOptions.push({
+        value: 'global-searches',
+        label: t('Recommended Searches'),
+        options: globalSearches.map(search => getSearchOption(search, 'global-search')),
+      });
+    }
+
+    return searchOptions;
+  }, []);
+
+  const trigger = ({props, ref}) => (
+    <StyledDropdownTrigger
+      ref={ref}
+      {...props}
+      isActive={isActive}
+      borderless
+      priority="link"
+      size="zero"
+    >
       {isActive ? (
         <Fragment>
-          <TitleTextOverflow>
-            {savedSearch ? savedSearch.name : t('Custom Search')}{' '}
-          </TitleTextOverflow>
-          {queryCount !== undefined && queryCount > 0 && (
-            <div>
-              <Badge>
-                <QueryCount hideParens count={queryCount} max={1000} />
-              </Badge>
-            </div>
+          <span>{savedSearch ? savedSearch.name : t('Custom Search')}&nbsp;</span>
+          {defined(queryCount) && queryCount > 0 && (
+            <Badge>
+              <QueryCount hideParens count={queryCount} max={1000} />
+            </Badge>
           )}
         </Fragment>
       ) : (
         t('Saved Searches')
       )}
-    </TitleWrapper>
+    </StyledDropdownTrigger>
+  );
+
+  const onChange = useCallback(
+    option => {
+      const searchObj = savedSearchList.find(s =>
+        s.isGlobal
+          ? `global-search-${s.id}` === option.value
+          : `saved-search-${s.id}` === option.value
+      );
+      searchObj && onSavedSearchSelect(searchObj);
+    },
+    [savedSearchList, onSavedSearchSelect]
   );
 
   return (
-    <TabWrapper
+    <StyledCompactSelect
+      renderWrapAs="li"
+      trigger={trigger}
+      options={options}
+      value={savedSearchValue}
       isActive={isActive}
-      className="saved-search-tab"
-      data-test-id="saved-search-tab"
-    >
-      <StyledDropdownLink
-        alwaysRenderMenu={false}
-        anchorMiddle
-        caret
-        title={title}
-        isActive={isActive}
-      >
-        <SavedSearchMenu
-          organization={organization}
-          savedSearchList={savedSearchList}
-          onSavedSearchSelect={onSavedSearchSelect}
-          onSavedSearchDelete={onSavedSearchDelete}
-          query={query}
-          sort={sort}
-        />
-      </StyledDropdownLink>
-    </TabWrapper>
+      onChange={onChange}
+      offset={-4}
+      maxMenuHeight={800}
+    />
   );
 }
 
 export default SavedSearchTab;
 
-const TabWrapper = styled('li')<{isActive?: boolean}>`
-  /* Color matches nav-tabs - overwritten using dark mode class saved-search-tab */
-  border-bottom: ${p => (p.isActive ? `4px solid #6c5fc7` : 0)};
-  /* Reposition menu under caret */
-  & > span {
-    display: block;
-  }
-  & > span > .dropdown-menu {
-    padding: 0;
-    margin-top: ${space(1)};
-    min-width: 20vw;
-    max-width: 25vw;
-    z-index: ${p => p.theme.zIndex.globalSelectionHeader};
-
-    :after {
-      border-bottom-color: ${p => p.theme.backgroundSecondary};
-    }
+const StyledCompactSelect = styled(CompactSelect)<{isActive?: boolean}>`
+  && {
+    position: static;
   }
+  border-bottom-width: 4px;
+  border-bottom-style: solid;
+  border-bottom-color: ${p => (p.isActive ? p.theme.active : 'transparent')};
+`;
 
-  @media (max-width: ${p => p.theme.breakpoints[4]}) {
-    & > span > .dropdown-menu {
-      max-width: 30vw;
-    }
-  }
+const StyledDropdownTrigger = styled(DropdownButtonV2)<{isActive?: boolean}>`
+  display: flex;
+  height: calc(1.25rem - 2px);
+  align-items: center;
+  color: ${p => (p.isActive ? p.theme.textColor : p.theme.subText)};
+  box-sizing: content-box;
+  padding: ${space(1)} 0;
 
-  @media (max-width: ${p => p.theme.breakpoints[1]}) {
-    & > span > .dropdown-menu {
-      max-width: 50vw;
-    }
+  &:hover {
+    color: ${p => p.theme.textColor};
   }
 `;
 
-const TitleWrapper = styled('span')`
-  margin-right: ${space(0.5)};
-  user-select: none;
-  display: flex;
-  align-items: center;
+const Details = styled('span')`
+  font-family: ${p => p.theme.text.familyMono};
+  font-size: ${p => p.theme.fontSizeExtraSmall};
+  max-width: 16rem;
+  ${overflowEllipsis}
 `;
 
-const TitleTextOverflow = styled('span')`
-  margin-right: ${space(0.5)};
-  max-width: 150px;
-  ${overflowEllipsis};
+const TooltipSearchQuery = styled('span')`
+  color: ${p => p.theme.subText};
+  font-weight: normal;
+  font-family: ${p => p.theme.text.familyMono};
 `;
 
-const StyledDropdownLink = styled(DropdownLink)<{isActive?: boolean}>`
-  position: relative;
-  display: block;
+const DeleteButton = styled(Button)`
+  color: ${p => p.theme.subText};
+  flex-shrink: 0;
   padding: ${space(1)} 0;
-  text-align: center;
-  text-transform: capitalize;
-  /* TODO(scttcper): Replace hex color when nav-tabs is replaced */
-  color: ${p => (p.isActive ? p.theme.textColor : '#7c6a8e')};
 
   :hover {
-    color: #2f2936;
+    color: ${p => p.theme.error};
   }
 `;

+ 2 - 2
tests/js/spec/views/issueList/header.spec.jsx

@@ -268,7 +268,7 @@ describe('IssueListHeader', () => {
         queryCount={13}
       />
     );
-    expect(screen.getByTestId('saved-search-tab')).toHaveTextContent('Custom Search 13');
+    expect(screen.getByRole('button', {name: 'Custom Search 13'})).toBeInTheDocument();
   });
 
   it('should display saved search name and count', () => {
@@ -291,7 +291,7 @@ describe('IssueListHeader', () => {
         queryCount={13}
       />
     );
-    expect(screen.getByTestId('saved-search-tab')).toHaveTextContent('Saved Search 13');
+    expect(screen.getByRole('button', {name: 'Saved Search 13'})).toBeInTheDocument();
   });
 
   it('lists saved searches in dropdown', () => {

+ 8 - 5
tests/js/spec/views/issueList/organizationSavedSearchMenu.spec.jsx

@@ -6,9 +6,9 @@ import {
   within,
 } from 'sentry-test/reactTestingLibrary';
 
-import IssueListSavedSearchMenu from 'sentry/views/issueList/savedSearchMenu';
+import IssueListSavedSearchTab from 'sentry/views/issueList/savedSearchTab';
 
-describe('IssueListSavedSearchMenu', () => {
+describe('IssueListSavedSearchTab', () => {
   const savedSearchList = [
     {
       id: '789',
@@ -39,8 +39,8 @@ describe('IssueListSavedSearchMenu', () => {
   const onDelete = jest.fn();
 
   function renderSavedSearch({organization} = {}) {
-    return render(
-      <IssueListSavedSearchMenu
+    render(
+      <IssueListSavedSearchTab
         organization={organization ?? TestStubs.Organization({access: ['org:write']})}
         savedSearchList={savedSearchList}
         onSavedSearchSelect={onSelect}
@@ -49,6 +49,9 @@ describe('IssueListSavedSearchMenu', () => {
       />,
       {context: TestStubs.routerContext()}
     );
+
+    // Open the saved searches menu
+    screen.getByRole('button', {name: 'Saved Searches'}).click();
   }
 
   afterEach(() => {
@@ -74,7 +77,7 @@ describe('IssueListSavedSearchMenu', () => {
   it('does not show a delete button for global search', () => {
     renderSavedSearch();
     // Should not have a delete button as it is a global search
-    const globalSearch = screen.getByTestId('saved-search-122');
+    const globalSearch = screen.getByTestId('global-search-122');
     expect(
       within(globalSearch).queryByRole('button', {name: 'delete'})
     ).not.toBeInTheDocument();

+ 26 - 8
tests/js/spec/views/issueList/overview.spec.jsx

@@ -5,6 +5,7 @@ import range from 'lodash/range';
 import {mountWithTheme, shallow} from 'sentry-test/enzyme';
 import {initializeOrg} from 'sentry-test/initializeOrg';
 import {act} from 'sentry-test/reactTestingLibrary';
+import {triggerPress} from 'sentry-test/utils';
 
 import StreamGroup from 'sentry/components/stream/group';
 import GroupStore from 'sentry/stores/groupStore';
@@ -195,8 +196,7 @@ describe('IssueList', function () {
     let issuesRequest;
 
     /* helpers */
-    const getSavedSearchTitle = w =>
-      w.find('SavedSearchTab DropdownMenu a').text().trim();
+    const getSavedSearchTitle = w => w.find('SavedSearchTab ButtonLabel').text().trim();
 
     const getSearchBarValue = w =>
       w.find('SmartSearchBarContainer textarea').prop('value').trim();
@@ -504,8 +504,14 @@ describe('IssueList', function () {
       await tick();
       wrapper.update();
 
-      wrapper.find('SavedSearchTab DropdownMenu a').simulate('click');
-      wrapper.find('SavedSearchMenuItem a').last().simulate('click');
+      await act(async () => {
+        triggerPress(wrapper.find('SavedSearchTab StyledDropdownTrigger'));
+
+        await tick();
+        wrapper.update();
+      });
+
+      wrapper.find('SelectOption').last().simulate('click');
 
       expect(browserHistory.push).toHaveBeenLastCalledWith(
         expect.objectContaining({
@@ -684,8 +690,14 @@ describe('IssueList', function () {
         },
       });
 
-      wrapper.find('SavedSearchTab DropdownMenu a').simulate('click');
-      wrapper.find('SavedSearchMenuItem a').first().simulate('click');
+      await act(async () => {
+        triggerPress(wrapper.find('SavedSearchTab StyledDropdownTrigger'));
+
+        await tick();
+        wrapper.update();
+      });
+
+      wrapper.find('SelectOption').first().simulate('click');
 
       await tick();
 
@@ -736,8 +748,14 @@ describe('IssueList', function () {
       expect(getSavedSearchTitle(wrapper)).toBe('Unresolved TypeErrors');
 
       // Select other saved search
-      wrapper.find('SavedSearchTab DropdownMenu a').simulate('click');
-      wrapper.find('SavedSearchMenuItem a').last().simulate('click');
+      await act(async () => {
+        triggerPress(wrapper.find('SavedSearchTab StyledDropdownTrigger'));
+
+        await tick();
+        wrapper.update();
+      });
+
+      wrapper.find('SelectOption').last().simulate('click');
 
       expect(browserHistory.push).toHaveBeenLastCalledWith(
         expect.objectContaining({