feat(workflow): Restyle saved search menu (#24691)

src/sentry/static/sentry/app/components/layouts/thirds.tsx

@@ -108,15 +108,15 @@ export const HeaderNavTabs = styled(NavTabs)`
   margin: 0;
   border-bottom: 0 !important;
 
-  li {
+  & > li {
     margin-right: ${space(3)};
   }
-  li > a {
+  & > li > a {
     padding: ${space(1)} 0;
     font-size: ${p => p.theme.fontSizeLarge};
     margin-bottom: 4px;
   }
-  li.active > a {
+  & > li.active > a {
     margin-bottom: 0;
   }
 `;

src/sentry/static/sentry/app/views/issueList/savedSearchMenu.tsx

@@ -14,106 +14,145 @@ import {Organization, SavedSearch} from 'app/types';
 
 import {getSortLabel} from './utils';
 
+type MenuItemProps = Omit<Props, 'savedSearchList'> & {
+  search: SavedSearch;
+  isLast: boolean;
+};
+
+function SavedSearchMenuItem({
+  organization,
+  onSavedSearchSelect,
+  onSavedSearchDelete,
+  search,
+  query,
+  sort,
+  isLast,
+}: MenuItemProps) {
+  return (
+    <Tooltip
+      title={
+        <React.Fragment>
+          {`${search.name} \u2022 `}
+          <TooltipSearchQuery>{search.query}</TooltipSearchQuery>
+          {` \u2022 `}
+          {t('Sort: ')}
+          {getSortLabel(search.sort)}
+        </React.Fragment>
+      }
+      containerDisplayMode="block"
+      delay={1000}
+    >
+      <StyledMenuItem
+        isActive={search.query === query && search.sort === sort}
+        isLast={isLast}
+      >
+        <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 />}
+                label={t('delete')}
+                size="zero"
+              />
+            </Confirm>
+          </Access>
+        )}
+      </StyledMenuItem>
+    </Tooltip>
+  );
+}
+
 type Props = {
-  organization: Organization;
   savedSearchList: SavedSearch[];
+  organization: Organization;
   onSavedSearchSelect: (savedSearch: SavedSearch) => void;
   onSavedSearchDelete: (savedSearch: SavedSearch) => void;
   sort: string;
   query?: string;
 };
 
-function SavedSearchMenu({
-  savedSearchList,
-  onSavedSearchDelete,
-  onSavedSearchSelect,
-  organization,
-  sort,
-  query,
-}: Props) {
-  if (savedSearchList.length === 0) {
-    return <EmptyItem>{t("There don't seem to be any saved searches yet.")}</EmptyItem>;
-  }
+function SavedSearchMenu({savedSearchList, ...props}: Props) {
+  const savedSearches = savedSearchList.filter(search => !search.isGlobal);
+  const commonSearches = savedSearchList.filter(search => search.isGlobal);
 
   return (
     <React.Fragment>
-      {savedSearchList.map((search, index) => (
-        <Tooltip
-          title={
-            <React.Fragment>
-              {`${search.name} \u2022 `}
-              <TooltipSearchQuery>{search.query}</TooltipSearchQuery>
-              {` \u2022 `}
-              {t('Sort: ')}
-              {getSortLabel(search.sort)}
-            </React.Fragment>
-          }
-          containerDisplayMode="block"
-          delay={1000}
-          key={search.id}
-        >
-          <StyledMenuItem
-            isActive={search.query === query && search.sort === sort}
-            last={index === savedSearchList.length - 1}
-          >
-            <MenuItemLink tabIndex={-1} onClick={() => onSavedSearchSelect(search)}>
-              <SearchTitle>{search.name}</SearchTitle>
-              <SearchQuery>{search.query}</SearchQuery>
-              <SearchSort>
-                {t('Sort: ')}
-                {getSortLabel(search.sort)}
-              </SearchSort>
-            </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 />}
-                    label={t('delete')}
-                    size="zero"
-                  />
-                </Confirm>
-              </Access>
-            )}
-          </StyledMenuItem>
-        </Tooltip>
-      ))}
+      <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('Pre-built searches')}</SecondaryMenuHeader>
+      {/* Could only happen on self-hosted */}
+      {commonSearches.length === 0 ? (
+        <EmptyItem>{t('No pre-built searches yet.')}</EmptyItem>
+      ) : (
+        commonSearches.map((search, index) => (
+          <SavedSearchMenuItem
+            key={search.id}
+            search={search}
+            isLast={index === commonSearches.length - 1}
+            {...props}
+          />
+        ))
+      )}
     </React.Fragment>
   );
 }
 
 export default SavedSearchMenu;
 
-const SearchTitle = styled('strong')`
+const SearchTitle = styled('div')`
   color: ${p => p.theme.textColor};
+  ${overflowEllipsis}
+`;
 
-  &:after {
-    content: ' \u2022 ';
-  }
+const SearchQueryContainer = styled('div')`
+  font-size: ${p => p.theme.fontSizeExtraSmall};
+  line-height: 1;
+  ${overflowEllipsis}
 `;
 
 const SearchQuery = styled('code')`
-  color: ${p => p.theme.textColor};
+  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.fontSizeSmall};
-
+  font-size: ${p => p.theme.fontSizeExtraSmall};
   &:before {
-    font-size: ${p => p.theme.fontSizeMedium};
+    font-size: ${p => p.theme.fontSizeExtraSmall};
     color: ${p => p.theme.textColor};
     content: ' \u2022 ';
   }
@@ -137,11 +176,31 @@ const DeleteButton = styled(Button)`
   }
 `;
 
-const StyledMenuItem = styled(MenuItem)<{isActive: boolean; last: boolean}>`
-  border-bottom: ${p => (!p.last ? `1px solid ${p.theme.innerBorder}` : null)};
+const MenuHeader = styled('div')`
+  align-items: center;
+  color: ${p => p.theme.gray400};
+  font-weight: 400;
+  background: ${p => p.theme.backgroundSecondary};
+  line-height: 0.75;
+  padding: ${space(1.5)};
+  border-bottom: 1px solid ${p => p.theme.border};
+  border-radius: ${p => p.theme.borderRadius} ${p => p.theme.borderRadius} 0 0;
+`;
+
+const SecondaryMenuHeader = styled(MenuHeader)`
+  border-top: 1px solid ${p => p.theme.border};
+  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};
   padding: 0;
 
+  & > span > a {
+    padding: ${space(0.75)} 0 ${space(1)} 0;
+  }
+
   ${p =>
     p.isActive &&
     `
@@ -173,6 +232,6 @@ const MenuItemLink = styled('a')`
 `;
 
 const EmptyItem = styled('li')`
-  padding: 8px 10px 5px;
-  font-style: italic;
+  padding: ${space(1)} ${space(1.5)};
+  color: ${p => p.theme.subText};
 `;

src/sentry/static/sentry/app/views/issueList/savedSearchTab.tsx

@@ -99,10 +99,15 @@ const TabWrapper = styled('li')<{isActive?: boolean}>`
     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};
+    }
   }
 
   @media (max-width: ${p => p.theme.breakpoints[4]}) {
@@ -116,11 +121,6 @@ const TabWrapper = styled('li')<{isActive?: boolean}>`
       max-width: 50vw;
     }
   }
-
-  /* Fix nav tabs style leaking into menu */
-  * > li {
-    margin: 0;
-  }
 `;
 
 const TitleWrapper = styled('span')`

tests/js/spec/views/issueList/organizationSavedSearchMenu.spec.jsx

@@ -49,8 +49,7 @@ describe('IssueListSavedSearchMenu', () => {
 
   describe('removing a saved search', () => {
     it('shows a delete button with access', () => {
-      // Second item should have a delete button as it is not a global search
-      const button = wrapper.find('MenuItem').at(1).find('button[aria-label="delete"]');
+      const button = wrapper.find('MenuItem').first().find('button[aria-label="delete"]');
       expect(button).toHaveLength(1);
     });
 
@@ -58,19 +57,19 @@ describe('IssueListSavedSearchMenu', () => {
       organization.access = [];
       wrapper.setProps({organization});
 
-      const button = wrapper.find('MenuItem').at(1).find('button[aria-label="delete"]');
-      expect(button).toHaveLength(0);
+      const button = wrapper.find('MenuItem').first().find('button[aria-label="delete"]');
+      expect(button.exists()).toBe(false);
     });
 
     it('does not show a delete button for global search', () => {
       // First item should not have a delete button as it is a global search
-      const button = wrapper.find('MenuItem').first().find('button[aria-label="delete"]');
+      const button = wrapper.find('MenuItem').at(1).find('button[aria-label="delete"]');
       expect(button).toHaveLength(0);
     });
 
     it('sends a request when delete button is clicked', async () => {
       // Second item should have a delete button as it is not a global search
-      const button = wrapper.find('MenuItem').at(1).find('button[aria-label="delete"]');
+      const button = wrapper.find('MenuItem').at(0).find('button[aria-label="delete"]');
       button.simulate('click');
       await wrapper.update();