Browse Source

ref(breadcrumbs): Use SearchBarActionV2 (#35226)

* ref(searchBarAction): Add a V2 that uses CompactSelect

* ref(searchBarActionV2): Remove unnecessary button styles

* ref(searchBarActionV2): Type trigger function

* ref(searchBarActionV2): Detach input from button in mobile

* ref(breadcrumbs): Use CompactSelect for filter

* ref(searchBarAction): Allow leadingItems props

* ref(searchBarAction): Clean up types

* ref(breadcrumbsFilter): Refactor filter tests

* ref(searchBarAction): Rename selectedFilters as filterSelections

* Update static/app/components/events/interfaces/breadcrumbs/index.tsx

Co-authored-by: Evan Purkhiser <evanpurkhiser@gmail.com>

* ref(breadcumbs): Use startsWith instead of split

* ref(breadcrumbs): Remove opacity: 0

Co-authored-by: Evan Purkhiser <evanpurkhiser@gmail.com>
Vu Luong 2 years ago
parent
commit
25beeacdd1

+ 2 - 2
static/app/components/events/interfaces/breadcrumbs/breadcrumb/type/index.tsx

@@ -51,8 +51,8 @@ const IconWrapper = styled('div')<Pick<Props, 'color'>>`
   display: flex;
   align-items: center;
   justify-content: center;
-  width: 24px;
-  height: 24px;
+  width: 22px;
+  height: 22px;
   border-radius: 50%;
   color: ${p => p.theme.white};
   background: ${p => p.theme[p.color] ?? p.color};

+ 58 - 57
static/app/components/events/interfaces/breadcrumbs/index.tsx

@@ -9,32 +9,22 @@ import ErrorBoundary from 'sentry/components/errorBoundary';
 import EventDataSection from 'sentry/components/events/eventDataSection';
 import {t} from 'sentry/locale';
 import {Organization} from 'sentry/types';
-import {
-  BreadcrumbLevelType,
-  BreadcrumbType,
-  Crumb,
-  RawCrumb,
-} from 'sentry/types/breadcrumbs';
+import {BreadcrumbLevelType, Crumb, RawCrumb} from 'sentry/types/breadcrumbs';
 import {Event} from 'sentry/types/event';
 import {defined} from 'sentry/utils';
 
-import SearchBarAction from '../searchBarAction';
-import SearchBarActionFilter from '../searchBarAction/searchBarActionFilter';
+import SearchBarAction from '../searchBarActionV2';
 
 import Level from './breadcrumb/level';
 import Type from './breadcrumb/type';
 import Breadcrumbs from './breadcrumbs';
 import {getVirtualCrumb, transformCrumbs} from './utils';
 
-type FilterOptions = React.ComponentProps<typeof SearchBarActionFilter>['options'];
+type FilterOptions = NonNullable<
+  React.ComponentProps<typeof SearchBarAction>['filterOptions']
+>;
 
-type FilterTypes = {
-  description: string;
-  id: BreadcrumbType;
-  isChecked: boolean;
-  levels: BreadcrumbLevelType[];
-  symbol: React.ReactElement;
-};
+type FilterOptionWithLevels = FilterOptions[0] & {levels?: BreadcrumbLevelType[]};
 
 type Props = Pick<React.ComponentProps<typeof Breadcrumbs>, 'route' | 'router'> & {
   data: {
@@ -49,6 +39,7 @@ type State = {
   breadcrumbs: Crumb[];
   displayRelativeTime: boolean;
   filterOptions: FilterOptions;
+  filterSelections: FilterOptions;
   filteredByFilter: Crumb[];
   filteredBySearch: Crumb[];
   searchTerm: string;
@@ -68,7 +59,8 @@ function BreadcrumbsContainer({
     breadcrumbs: [],
     filteredByFilter: [],
     filteredBySearch: [],
-    filterOptions: {},
+    filterOptions: [],
+    filterSelections: [],
     displayRelativeTime: false,
   });
 
@@ -112,32 +104,41 @@ function BreadcrumbsContainer({
     const typeOptions = getFilterTypes(crumbs);
     const levels = getFilterLevels(typeOptions);
 
-    const options = {};
+    const options: FilterOptions = [];
 
     if (!!typeOptions.length) {
-      options[t('Types')] = typeOptions.map(typeOption => omit(typeOption, 'levels'));
+      options.push({
+        value: 'types',
+        label: t('Types'),
+        options: typeOptions.map(typeOption => omit(typeOption, 'levels')),
+      });
     }
 
     if (!!levels.length) {
-      options[t('Levels')] = levels;
+      options.push({
+        value: 'levels',
+        label: t('Levels'),
+        options: levels,
+      });
     }
 
     return options;
   }
 
   function getFilterTypes(crumbs: ReturnType<typeof transformCrumbs>) {
-    const filterTypes: FilterTypes[] = [];
+    const filterTypes: FilterOptionWithLevels[] = [];
 
     for (const index in crumbs) {
       const breadcrumb = crumbs[index];
-      const foundFilterType = filterTypes.findIndex(f => f.id === breadcrumb.type);
+      const foundFilterType = filterTypes.findIndex(
+        f => f.value === `type-${breadcrumb.type}`
+      );
 
       if (foundFilterType === -1) {
         filterTypes.push({
-          id: breadcrumb.type,
-          symbol: <Type type={breadcrumb.type} color={breadcrumb.color} />,
-          isChecked: false,
-          description: breadcrumb.description,
+          value: `type-${breadcrumb.type}`,
+          leadingItems: <Type type={breadcrumb.type} color={breadcrumb.color} />,
+          label: breadcrumb.description,
           levels: breadcrumb?.level ? [breadcrumb.level] : [],
         });
         continue;
@@ -145,30 +146,33 @@ function BreadcrumbsContainer({
 
       if (
         breadcrumb?.level &&
-        !filterTypes[foundFilterType].levels.includes(breadcrumb.level)
+        !filterTypes[foundFilterType].levels?.includes(breadcrumb.level)
       ) {
-        filterTypes[foundFilterType].levels.push(breadcrumb.level);
+        filterTypes[foundFilterType].levels?.push(breadcrumb.level);
       }
     }
 
     return filterTypes;
   }
 
-  function getFilterLevels(types: FilterTypes[]) {
-    const filterLevels: FilterOptions[0] = [];
+  function getFilterLevels(types: FilterOptionWithLevels[]) {
+    const filterLevels: FilterOptions = [];
 
     for (const indexType in types) {
       for (const indexLevel in types[indexType].levels) {
-        const level = types[indexType].levels[indexLevel];
+        const level = types[indexType].levels?.[indexLevel];
 
-        if (filterLevels.some(f => f.id === level)) {
+        if (filterLevels.some(f => f.value === `level-${level}`)) {
           continue;
         }
 
         filterLevels.push({
-          id: level,
-          symbol: <Level level={level} />,
-          isChecked: false,
+          value: `level-${level}`,
+          label: (
+            <LevelWrap>
+              <Level level={level} />
+            </LevelWrap>
+          ),
         });
       }
     }
@@ -207,17 +211,17 @@ function BreadcrumbsContainer({
     );
   }
 
-  function getFilteredCrumbsByFilter(newfilterOptions: FilterOptions) {
+  function getFilteredCrumbsByFilter(selectedFilterOptions: FilterOptions) {
     const checkedTypeOptions = new Set(
-      Object.values(newfilterOptions)[0]
-        .filter(filterOption => filterOption.isChecked)
-        .map(option => option.id)
+      selectedFilterOptions
+        .filter(option => option.value.startsWith('type-'))
+        .map(option => option.value.split('-')[1])
     );
 
     const checkedLevelOptions = new Set(
-      Object.values(newfilterOptions)[1]
-        .filter(filterOption => filterOption.isChecked)
-        .map(option => option.id)
+      selectedFilterOptions
+        .filter(option => option.value.startsWith('level-'))
+        .map(option => option.value.split('-')[1])
     );
 
     if (!![...checkedTypeOptions].length && !![...checkedLevelOptions].length) {
@@ -253,10 +257,9 @@ function BreadcrumbsContainer({
 
   function handleFilter(newfilterOptions: FilterOptions) {
     const newfilteredByFilter = getFilteredCrumbsByFilter(newfilterOptions);
-
     setState({
       ...state,
-      filterOptions: newfilterOptions,
+      filterSelections: newfilterOptions,
       filteredByFilter: newfilteredByFilter,
       filteredBySearch: filterBySearch(searchTerm, newfilteredByFilter),
     });
@@ -272,14 +275,8 @@ function BreadcrumbsContainer({
   function handleResetFilter() {
     setState({
       ...state,
+      filterSelections: [],
       filteredByFilter: breadcrumbs,
-      filterOptions: Object.keys(filterOptions).reduce((accumulator, currentValue) => {
-        accumulator[currentValue] = filterOptions[currentValue].map(filterOption => ({
-          ...filterOption,
-          isChecked: false,
-        }));
-        return accumulator;
-      }, {}),
       filteredBySearch: filterBySearch(searchTerm, breadcrumbs),
     });
   }
@@ -298,9 +295,7 @@ function BreadcrumbsContainer({
     }
 
     if (searchTerm && !filteredBySearch.length) {
-      const hasActiveFilter = Object.values(filterOptions)
-        .flatMap(filterOption => filterOption)
-        .find(filterOption => filterOption.isChecked);
+      const hasActiveFilter = state.filterSelections.length > 0;
 
       return {
         emptyMessage: t('Sorry, no breadcrumbs match your search query'),
@@ -334,9 +329,9 @@ function BreadcrumbsContainer({
           placeholder={t('Search breadcrumbs')}
           onChange={handleSearch}
           query={searchTerm}
-          filter={
-            <SearchBarActionFilter onChange={handleFilter} options={filterOptions} />
-          }
+          filterOptions={filterOptions}
+          filterSelections={state.filterSelections}
+          onFilterChange={handleFilter}
         />
       }
       wrapTitle={false}
@@ -365,3 +360,9 @@ export default BreadcrumbsContainer;
 const StyledSearchBarAction = styled(SearchBarAction)`
   z-index: 2;
 `;
+
+const LevelWrap = styled('span')`
+  height: ${p => p.theme.text.lineHeightBody}em;
+  display: flex;
+  align-items: center;
+`;

+ 1 - 1
tests/js/spec/components/events/interfaces/breadcrumbs/breadcrumbs.spec.tsx

@@ -94,7 +94,7 @@ describe('Breadcrumbs', () => {
 
       // breadcrumbs + filter item
       // TODO(Priscila): Filter should not render in the dom if not open
-      expect(screen.getAllByText(textWithMarkupMatcher('Warning'))).toHaveLength(6);
+      expect(screen.getAllByText(textWithMarkupMatcher('Warning'))).toHaveLength(5);
     });
 
     it('should filter crumbs based on crumb category', function () {

+ 140 - 114
tests/js/spec/components/events/interfaces/breadcrumbs/filter.spec.tsx

@@ -1,64 +1,68 @@
-import {mountWithTheme} from 'sentry-test/enzyme';
+import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import Level from 'sentry/components/events/interfaces/breadcrumbs/breadcrumb/level';
 import Type from 'sentry/components/events/interfaces/breadcrumbs/breadcrumb/type';
-import SearchBarActionFilter from 'sentry/components/events/interfaces/searchBarAction/searchBarActionFilter';
+import SearchBarAction from 'sentry/components/events/interfaces/searchBarActionV2';
 import {BreadcrumbLevelType, BreadcrumbType} from 'sentry/types/breadcrumbs';
 
-const options: React.ComponentProps<typeof SearchBarActionFilter>['options'] = {
-  ['Types']: [
-    {
-      id: BreadcrumbType.HTTP,
-      description: 'HTTP request',
-      symbol: <Type color="green300" type={BreadcrumbType.HTTP} />,
-      isChecked: true,
-    },
-    {
-      id: BreadcrumbType.TRANSACTION,
-      description: 'Transaction',
-      symbol: <Type color="pink300" type={BreadcrumbType.TRANSACTION} />,
-      isChecked: true,
-    },
-    {
-      id: BreadcrumbType.UI,
-      description: 'User Action',
-      symbol: <Type color="purple300" type={BreadcrumbType.UI} />,
-      isChecked: true,
-    },
-    {
-      id: BreadcrumbType.NAVIGATION,
-      description: 'Navigation',
-      symbol: <Type color="green300" type={BreadcrumbType.NAVIGATION} />,
-      isChecked: true,
-    },
-    {
-      id: BreadcrumbType.DEBUG,
-      description: 'Debug',
-      symbol: <Type color="purple300" type={BreadcrumbType.DEBUG} />,
-      isChecked: true,
-    },
-    {
-      id: BreadcrumbType.ERROR,
-      description: 'Error',
-      symbol: <Type color="red300" type={BreadcrumbType.ERROR} />,
-      isChecked: true,
-    },
-  ],
-  ['Levels']: [
-    {
-      id: BreadcrumbLevelType.INFO,
-      symbol: <Level level={BreadcrumbLevelType.INFO} />,
-      isChecked: true,
-    },
-    {
-      id: BreadcrumbLevelType.ERROR,
-      symbol: <Level level={BreadcrumbLevelType.ERROR} />,
-      isChecked: true,
-    },
-  ],
-};
-
-describe('SearchBarActionFilter', () => {
+const options: NonNullable<
+  React.ComponentProps<typeof SearchBarAction>['filterOptions']
+> = [
+  {
+    value: 'types',
+    label: 'Types',
+    options: [
+      {
+        value: BreadcrumbType.HTTP,
+        label: 'HTTP request',
+        leadingItems: <Type color="green300" type={BreadcrumbType.HTTP} />,
+      },
+      {
+        value: BreadcrumbType.TRANSACTION,
+        label: 'Transaction',
+        leadingItems: <Type color="pink300" type={BreadcrumbType.TRANSACTION} />,
+      },
+      {
+        value: BreadcrumbType.UI,
+        label: 'User Action',
+        leadingItems: <Type color="purple300" type={BreadcrumbType.UI} />,
+      },
+      {
+        value: BreadcrumbType.NAVIGATION,
+        label: 'Navigation',
+        leadingItems: <Type color="green300" type={BreadcrumbType.NAVIGATION} />,
+      },
+      {
+        value: BreadcrumbType.DEBUG,
+        label: 'Debug',
+        leadingItems: <Type color="purple300" type={BreadcrumbType.DEBUG} />,
+      },
+      {
+        value: BreadcrumbType.ERROR,
+        label: 'Error',
+        leadingItems: <Type color="red300" type={BreadcrumbType.ERROR} />,
+      },
+    ],
+  },
+  {
+    value: 'levels',
+    label: 'Levels',
+    options: [
+      {
+        value: BreadcrumbLevelType.INFO,
+        label: 'info',
+        leadingItems: <Level level={BreadcrumbLevelType.INFO} />,
+      },
+      {
+        value: BreadcrumbLevelType.ERROR,
+        label: 'error',
+        leadingItems: <Level level={BreadcrumbLevelType.ERROR} />,
+      },
+    ],
+  },
+];
+
+describe('SearchBarAction', () => {
   let handleFilter;
 
   beforeEach(() => {
@@ -66,93 +70,115 @@ describe('SearchBarActionFilter', () => {
   });
 
   it('default render', () => {
-    const wrapper = mountWithTheme(
-      <SearchBarActionFilter options={options} onChange={handleFilter} />
+    const {container} = render(
+      <SearchBarAction
+        filterOptions={options}
+        onFilterChange={handleFilter}
+        onChange={() => null}
+        placeholder=""
+        query=""
+      />
     );
 
-    const filterDropdownMenu = wrapper.find('StyledContent');
+    const filterDropdownMenu = screen.getByText('Filter By');
+    userEvent.click(filterDropdownMenu);
 
-    // Headers
-    const headers = filterDropdownMenu.find('Header');
-    expect(headers).toHaveLength(2);
-    expect(headers.at(0).text()).toBe('Types');
-    expect(headers.at(1).text()).toBe('Levels');
+    // Types
+    expect(screen.getByText('Types')).toBeInTheDocument();
+    expect(screen.getByText('HTTP request')).toBeInTheDocument();
+    expect(screen.getByText('Transaction')).toBeInTheDocument();
+    expect(screen.getByText('User Action')).toBeInTheDocument();
+    expect(screen.getByText('Navigation')).toBeInTheDocument();
+    expect(screen.getByText('Debug')).toBeInTheDocument();
+    expect(screen.getAllByText('Error')[0]).toBeInTheDocument();
 
-    // Lists
-    const lists = filterDropdownMenu.find('StyledList');
-    expect(lists).toHaveLength(2);
-    expect(lists.at(0).find('StyledListItem')).toHaveLength(6);
-    expect(lists.at(1).find('StyledListItem')).toHaveLength(2);
+    // Levels
+    expect(screen.getByText('Levels')).toBeInTheDocument();
+    expect(screen.getByText('info')).toBeInTheDocument();
+    expect(screen.getAllByText('Error')[1]).toBeInTheDocument();
 
-    expect(wrapper).toSnapshot();
+    expect(container).toSnapshot();
   });
 
   it('Without Options', () => {
-    const wrapper = mountWithTheme(
-      <SearchBarActionFilter options={{}} onChange={handleFilter} />
+    render(
+      <SearchBarAction
+        filterOptions={[]}
+        onFilterChange={handleFilter}
+        onChange={() => null}
+        placeholder=""
+        query=""
+      />
     );
-    expect(wrapper.find('Header').exists()).toBe(false);
-    expect(wrapper.find('StyledListItem').exists()).toBe(false);
+
+    expect(screen.queryByText('Types')).not.toBeInTheDocument();
+    expect(screen.queryByText('Levels')).not.toBeInTheDocument();
   });
 
   it('With Option Type only', () => {
-    const {Types} = options;
-    const wrapper = mountWithTheme(
-      <SearchBarActionFilter options={{Types}} onChange={handleFilter} />
+    const typeOptions = options[0];
+    render(
+      <SearchBarAction
+        filterOptions={[typeOptions]}
+        onFilterChange={handleFilter}
+        onChange={() => null}
+        placeholder=""
+        query=""
+      />
     );
 
-    const filterDropdownMenu = wrapper.find('StyledContent');
+    const filterDropdownMenu = screen.getByText('Filter By');
+    userEvent.click(filterDropdownMenu);
 
     // Header
-    const header = filterDropdownMenu.find('Header');
-    expect(header).toHaveLength(1);
-    expect(header.text()).toBe('Types');
-
-    // List
-    const list = filterDropdownMenu.find('StyledList');
-    expect(list).toHaveLength(1);
+    expect(screen.getByText('Types')).toBeInTheDocument();
+    expect(screen.queryByText('Levels')).not.toBeInTheDocument();
 
     // List Items
-    const listItems = list.find('StyledListItem');
-    expect(listItems).toHaveLength(6);
-    const firstItem = listItems.at(0);
-    expect(firstItem.find('Description').text()).toBe(options.Types[0].description);
-
-    // Check Item
-    expect(firstItem.find('CheckboxFancy').props().isChecked).toBeTruthy();
-    firstItem.simulate('click');
-
-    expect(handleFilter).toHaveBeenCalledTimes(1);
+    expect(screen.getByText('HTTP request')).toBeInTheDocument();
+    expect(screen.getByText('Transaction')).toBeInTheDocument();
+    expect(screen.getByText('User Action')).toBeInTheDocument();
+    expect(screen.getByText('Navigation')).toBeInTheDocument();
+    expect(screen.getByText('Debug')).toBeInTheDocument();
+    expect(screen.getAllByText('Error')[0]).toBeInTheDocument();
+
+    const httpRequestItem = screen.getByText('HTTP request');
+    userEvent.click(httpRequestItem);
+
+    const httpRequestOption = (typeOptions.options ?? []).find(
+      opt => opt.label === 'HTTP request'
+    );
+    expect(handleFilter).toHaveBeenCalledWith([httpRequestOption]);
   });
 
   it('With Option Level only', () => {
-    const {Levels} = options;
-    const wrapper = mountWithTheme(
-      <SearchBarActionFilter options={{Levels}} onChange={handleFilter} />
+    const levelOptions = options[1];
+    render(
+      <SearchBarAction
+        filterOptions={[levelOptions]}
+        onFilterChange={handleFilter}
+        onChange={() => null}
+        placeholder=""
+        query=""
+      />
     );
 
-    const filterDropdownMenu = wrapper.find('StyledContent');
+    const filterDropdownMenu = screen.getByText('Filter By');
+    userEvent.click(filterDropdownMenu);
 
     // Header
-    const header = filterDropdownMenu.find('Header');
-    expect(header).toHaveLength(1);
-    expect(header.text()).toBe('Levels');
-
-    // List
-    const list = filterDropdownMenu.find('StyledList');
-    expect(list).toHaveLength(1);
+    expect(screen.getByText('Levels')).toBeInTheDocument();
+    expect(screen.queryByText('Types')).not.toBeInTheDocument();
 
     // List Items
-    const listItems = list.find('StyledListItem');
-    expect(listItems).toHaveLength(2);
-    const firstItem = listItems.at(0);
-    expect(firstItem.text()).toBe('Info');
+    expect(screen.getByText('info')).toBeInTheDocument();
+    expect(screen.getByText('error')).toBeInTheDocument();
 
     // Check Item
-    expect(firstItem.find('CheckboxFancy').props().isChecked).toBeTruthy();
-
-    firstItem.simulate('click');
+    const infoItem = screen.getByText('info');
+    userEvent.click(infoItem);
 
-    expect(handleFilter).toHaveBeenCalledTimes(1);
+    const infoOption = (levelOptions.options ?? []).find(opt => opt.label === 'info');
+    expect(handleFilter).toHaveBeenCalledWith([infoOption]);
   });
 });