From 09230dd5d83d4b0b030d857443159779a06b6f2a Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Fri, 21 May 2021 20:22:39 -0600 Subject: [PATCH] Switch tag picker to Autocomplete Fix UserList counts not applying other filters Signed-off-by: Tim Hansen --- plugins/catalog-react/package.json | 1 + .../EntityTagPicker/EntityTagPicker.test.tsx | 6 +- .../EntityTagPicker/EntityTagPicker.tsx | 73 ++++++++----------- .../UserListPicker/UserListPicker.test.tsx | 26 ++++++- .../UserListPicker/UserListPicker.tsx | 52 ++++++------- .../src/hooks/useEntityListProvider.tsx | 7 ++ .../catalog-react/src/testUtils/providers.tsx | 7 ++ 7 files changed, 98 insertions(+), 74 deletions(-) diff --git a/plugins/catalog-react/package.json b/plugins/catalog-react/package.json index 9e2730da57..3b411518fd 100644 --- a/plugins/catalog-react/package.json +++ b/plugins/catalog-react/package.json @@ -33,6 +33,7 @@ "@backstage/core": "^0.7.9", "@material-ui/core": "^4.11.0", "@material-ui/icons": "^4.9.1", + "@material-ui/lab": "4.0.0-alpha.45", "@types/react": "^16.9", "lodash": "^4.17.15", "react": "^16.13.1", diff --git a/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.test.tsx b/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.test.tsx index cbd18da592..e601d1b8ab 100644 --- a/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.test.tsx @@ -50,6 +50,8 @@ describe('', () => { , ); expect(rendered.getByText('Tags')).toBeInTheDocument(); + + fireEvent.click(rendered.getByTestId('tag-picker-expand')); taggedEntities .flatMap(e => e.metadata.tags!) .forEach(tag => { @@ -72,6 +74,7 @@ describe('', () => { ); expect(updateFilters).not.toHaveBeenCalled(); + fireEvent.click(rendered.getByTestId('tag-picker-expand')); fireEvent.click(rendered.getByText('tag1')); expect(updateFilters).toHaveBeenLastCalledWith({ tags: new EntityTagFilter(['tag1']), @@ -93,9 +96,10 @@ describe('', () => { , ); expect(updateFilters).not.toHaveBeenCalled(); + fireEvent.click(rendered.getByTestId('tag-picker-expand')); expect(rendered.getByLabelText('tag1')).toBeChecked(); - fireEvent.click(rendered.getByText('tag1')); + fireEvent.click(rendered.getByLabelText('tag1')); expect(updateFilters).toHaveBeenLastCalledWith({ tags: undefined, }); diff --git a/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.tsx b/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.tsx index 9b52677ac7..d57700ca7d 100644 --- a/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.tsx +++ b/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.tsx @@ -17,31 +17,22 @@ import React, { useMemo } from 'react'; import { Checkbox, - List, - ListItem, - ListItemText, - makeStyles, - Theme, + FormControlLabel, + TextField, Typography, } from '@material-ui/core'; +import { Autocomplete } from '@material-ui/lab'; +import CheckBoxOutlineBlankIcon from '@material-ui/icons/CheckBoxOutlineBlank'; +import CheckBoxIcon from '@material-ui/icons/CheckBox'; +import ExpandMoreIcon from '@material-ui/icons/ExpandMore'; import { Entity } from '@backstage/catalog-model'; import { EntityTagFilter } from '../../types'; import { useEntityListProvider } from '../../hooks/useEntityListProvider'; -const useStyles = makeStyles(theme => ({ - title: { - margin: theme.spacing(1, 0, 0, 1), - textTransform: 'uppercase', - fontSize: 12, - fontWeight: 'bold', - }, - checkbox: { - padding: theme.spacing(0, 1, 0, 1), - }, -})); +const icon = ; +const checkedIcon = ; export const EntityTagPicker = () => { - const classes = useStyles(); const { updateFilters, backendEntities, filters } = useEntityListProvider(); const availableTags = useMemo( () => [ @@ -56,40 +47,36 @@ export const EntityTagPicker = () => { if (!availableTags.length) return null; - const onClick = (tag: string) => { - const tags = filters.tags?.values ?? []; - const newTags = tags.includes(tag) - ? [...tags.filter((t: string) => t !== tag)] - : [...tags, tag]; + const onChange = (tags: string[]) => { updateFilters({ - tags: newTags.length ? new EntityTagFilter(newTags) : undefined, + tags: tags.length ? new EntityTagFilter(tags) : undefined, }); }; return ( <> - - Tags - - - {availableTags.map(tag => { - const labelId = `checkbox-list-label-${tag}`; - return ( - onClick(tag)}> + Tags + + multiple + options={availableTags} + value={filters.tags?.values ?? []} + onChange={(_: object, value: string[]) => onChange(value)} + renderOption={(option, { selected }) => ( + - - - ); - })} - + } + label={option} + /> + )} + size="small" + popupIcon={} + renderInput={params => } + /> ); }; diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index 0d16371f6a..9809dbfbda 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -25,6 +25,7 @@ import { ConfigApi, configApiRef, } from '@backstage/core-api'; +import { EntityTagFilter } from '../../types'; const apis = ApiRegistry.from([ [ @@ -68,7 +69,7 @@ describe('', () => { metadata: { namespace: 'namespace-1', name: 'component-1', - tags: [], + tags: ['tag1'], }, relations: [ { @@ -83,7 +84,7 @@ describe('', () => { metadata: { namespace: 'namespace-2', name: 'component-2', - tags: [], + tags: ['tag1'], }, }, { @@ -157,6 +158,27 @@ describe('', () => { ).toEqual(['2', '1', '4']); }); + it('respects other frontend filters in counts', () => { + const { getAllByRole } = render( + + + + + , + ); + + expect( + getAllByRole('menuitem').map( + ({ nextSibling }) => nextSibling?.textContent, + ), + ).toEqual(['1', '0', '2']); + }); + it('updates user filter when a menuitem is selected', () => { const updateFilters = jest.fn(); const { getByText } = render( diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx index 6d1e77ca43..4ef57b7a48 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx @@ -14,18 +14,11 @@ * limitations under the License. */ -import React, { Fragment } from 'react'; +import React, { Fragment, useEffect, useState } from 'react'; +import { compact } from 'lodash'; import { configApiRef, IconComponent, useApi } from '@backstage/core'; -import { - FilterEnvironment, - UserListFilter, - UserListFilterKind, -} from '../../types'; -import { - useEntityListProvider, - useOwnUser, - useStarredEntities, -} from '../../hooks'; +import { UserListFilter, UserListFilterKind } from '../../types'; +import { useEntityListProvider } from '../../hooks'; import { Card, List, @@ -39,6 +32,7 @@ import { } from '@material-ui/core'; import SettingsIcon from '@material-ui/icons/Settings'; import StarIcon from '@material-ui/icons/Star'; +import { reduceEntityFilters } from '../../utils'; const useStyles = makeStyles(theme => ({ root: { @@ -62,9 +56,6 @@ const useStyles = makeStyles(theme => ({ groupWrapper: { margin: theme.spacing(1, 1, 2, 1), }, - menuTitle: { - fontWeight: 500, - }, })); export type ButtonGroup = { @@ -115,19 +106,25 @@ export const UserListPicker = () => { const orgName = configApi.getOptionalString('organization.name') ?? 'Company'; const filterGroups = getFilterGroups(orgName); - // Unfortunate FilterEnvironment duplication for static filters used for counts - const { value: user } = useOwnUser(); - const { isStarredEntity } = useStarredEntities(); - const filterEnv: FilterEnvironment = { - user: user, - isStarredEntity: isStarredEntity, - }; - const { - filters: { user: userFilter }, + filters, updateFilters, backendEntities, + filterEnv, } = useEntityListProvider(); + + // To show proper counts for each section, apply all other frontend filters _except_ the user + // filter that's controlled by this picker. + const [entitiesWithoutUserFilter, setEntitiesWithoutUserFilter] = useState( + backendEntities, + ); + useEffect(() => { + const filterFn = reduceEntityFilters( + compact(Object.values({ ...filters, user: undefined })), + filterEnv, + ); + setEntitiesWithoutUserFilter(backendEntities.filter(filterFn)); + }, [filters, backendEntities, filterEnv]); function setSelectedFilter({ id }: { id: UserListFilterKind }) { updateFilters({ user: new UserListFilter(id) }); } @@ -135,15 +132,15 @@ export const UserListPicker = () => { function getFilterCount(id: UserListFilterKind) { switch (id) { case 'owned': - return backendEntities.filter(entity => + return entitiesWithoutUserFilter.filter(entity => ownedFilter.filterEntity(entity, filterEnv), ).length; case 'starred': - return backendEntities.filter(entity => + return entitiesWithoutUserFilter.filter(entity => starredFilter.filterEntity(entity, filterEnv), ).length; default: - return backendEntities.length; + return entitiesWithoutUserFilter.length; } } @@ -162,7 +159,7 @@ export const UserListPicker = () => { button divider onClick={() => setSelectedFilter(item)} - selected={item.id === userFilter?.value} + selected={item.id === filters.user?.value} className={classes.menuItem} > {item.icon && ( @@ -173,7 +170,6 @@ export const UserListPicker = () => { {item.label} diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx index a25828ab1e..99cebd6310 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx @@ -56,6 +56,12 @@ export type EntityListContextProps< */ filters: EntityFilters; + /** + * The filter environment passed to frontend filters; contains information available in the React + * tree. + */ + filterEnv: FilterEnvironment; + /** * The resolved list of catalog entities, after all filters are applied. */ @@ -171,6 +177,7 @@ export const EntityListProvider = ({ }>) => { + const { value: user } = useOwnUser(); + const { isStarredEntity } = useStarredEntities(); const defaultContext: EntityListContextProps = { entities: [], backendEntities: [], updateFilters: jest.fn(), filters: {}, + filterEnv: { + user, + isStarredEntity, + }, loading: false, };