From 4c6950b5d8c222b274f8b31908aa2f804d07939a Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Fri, 20 Jan 2023 10:36:00 +0100 Subject: [PATCH 01/11] add aria labels to mobile navbar actions Signed-off-by: Emma Indal --- packages/core-components/src/layout/Sidebar/SidebarGroup.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core-components/src/layout/Sidebar/SidebarGroup.tsx b/packages/core-components/src/layout/Sidebar/SidebarGroup.tsx index 5254199563..15d22599b5 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarGroup.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarGroup.tsx @@ -100,6 +100,7 @@ const MobileSidebarGroup = (props: SidebarGroupProps) => { return ( // Material UI issue: https://github.com/mui-org/material-ui/issues/27820 Date: Fri, 20 Jan 2023 10:40:59 +0100 Subject: [PATCH 02/11] user primary color from palette to improve color contrast of tabs in Search modal Signed-off-by: Emma Indal --- plugins/search/src/components/SearchType/SearchType.Tabs.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/search/src/components/SearchType/SearchType.Tabs.tsx b/plugins/search/src/components/SearchType/SearchType.Tabs.tsx index fc0380e23a..b686e2e1f1 100644 --- a/plugins/search/src/components/SearchType/SearchType.Tabs.tsx +++ b/plugins/search/src/components/SearchType/SearchType.Tabs.tsx @@ -28,7 +28,7 @@ const useStyles = makeStyles((theme: BackstageTheme) => ({ height: '50px', fontWeight: theme.typography.fontWeightBold, fontSize: theme.typography.pxToRem(13), - color: theme.palette.textSubtle, + color: theme.palette.text.primary, minWidth: '130px', }, })); From a0619a5b492ff9f9050f334391a024b37bac1371 Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Fri, 20 Jan 2023 16:23:23 +0100 Subject: [PATCH 03/11] make sure result list is structured with list elements only Signed-off-by: Emma Indal --- .../app/src/components/search/SearchModal.tsx | 14 +-- .../CatalogSearchResultListItem.tsx | 75 +++++++-------- .../ToolSearchResultListItem.tsx | 71 +++++++------- .../DefaultResultListItem.tsx | 94 +++++++++---------- .../TechDocsSearchResultListItem.tsx | 12 +-- 5 files changed, 119 insertions(+), 147 deletions(-) diff --git a/packages/app/src/components/search/SearchModal.tsx b/packages/app/src/components/search/SearchModal.tsx index 4694d56390..e994262e66 100644 --- a/packages/app/src/components/search/SearchModal.tsx +++ b/packages/app/src/components/search/SearchModal.tsx @@ -23,17 +23,11 @@ import { Grid, makeStyles, Paper, - useTheme, } from '@material-ui/core'; import Typography from '@material-ui/core/Typography'; import BuildIcon from '@material-ui/icons/Build'; import LaunchIcon from '@material-ui/icons/Launch'; -import { - CatalogIcon, - DocsIcon, - Link, - useContent, -} from '@backstage/core-components'; +import { CatalogIcon, DocsIcon, Link } from '@backstage/core-components'; import { useApi, useRouteRef } from '@backstage/core-plugin-api'; import { CatalogSearchResultListItem } from '@internal/plugin-catalog-customized'; import { @@ -78,9 +72,6 @@ const rootRouteRef = searchPlugin.routes.root; export const SearchModal = ({ toggleModal }: { toggleModal: () => void }) => { const classes = useStyles(); const navigate = useNavigate(); - const { transitions } = useTheme(); - const { focusContent } = useContent(); - const catalogApi = useApi(catalogApiRef); const { term, types } = useSearch(); @@ -91,11 +82,14 @@ export const SearchModal = ({ toggleModal }: { toggleModal: () => void }) => { searchBarRef?.current?.focus(); }); +<<<<<<< HEAD const handleSearchResultClick = useCallback(() => { toggleModal(); setTimeout(focusContent, transitions.duration.leavingScreen); }, [toggleModal, focusContent, transitions]); +======= +>>>>>>> c224202137 (make sure result list is structured with list elements only) const handleSearchBarKeyDown = useCallback( (e: KeyboardEvent) => { if (e.key === 'Enter') { diff --git a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx index cf60138d99..2a4ec300cd 100644 --- a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx +++ b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx @@ -18,7 +18,6 @@ import React, { ReactNode } from 'react'; import { Box, Chip, - Divider, ListItem, ListItemIcon, ListItemText, @@ -52,6 +51,7 @@ export interface CatalogSearchResultListItemProps { result?: IndexableDocument; highlight?: ResultHighlight; rank?: number; + toggleModal?: () => void; } /** @public */ @@ -66,49 +66,44 @@ export function CatalogSearchResultListItem( if (!result) return null; return ( - <> - - {props.icon && {props.icon}} -
- - {highlight?.fields.title ? ( - - ) : ( - result.title - )} - - } - secondary={ - highlight?.fields.text ? ( + + {props.icon && {props.icon}} +
+ + {highlight?.fields.title ? ( ) : ( - result.text - ) - } - /> - - {result.kind && ( - - )} - {result.lifecycle && ( - - )} - -
- - - + result.title + )} + + } + secondary={ + highlight?.fields.text ? ( + + ) : ( + result.title + ) + } + /> + + {result.kind && } + {result.lifecycle && ( + + )} + +
+ ); } diff --git a/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx b/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx index c9e8946f26..fa57b93822 100644 --- a/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx +++ b/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx @@ -18,7 +18,6 @@ import React, { ReactNode } from 'react'; import { Box, Chip, - Divider, ListItem, ListItemIcon, ListItemText, @@ -52,6 +51,7 @@ export interface ToolSearchResultListItemProps { result?: IndexableDocument; highlight?: ResultHighlight; rank?: number; + toggleModal?: () => void; } /** @public */ @@ -63,47 +63,42 @@ export function ToolSearchResultListItem(props: ToolSearchResultListItemProps) { if (!result) return null; return ( - <> - - {props.icon && {props.icon}} -
- - {props.highlight?.fields.title ? ( - - ) : ( - result.title - )} - - } - secondary={ - props.highlight?.fields.text ? ( + + {props.icon && {props.icon}} +
+ + {props.highlight?.fields.title ? ( ) : ( - result.text - ) - } - /> - - {result.tags && - result.tags.map((tag: string) => ( - - ))} - -
- - - + result.title + )} + + } + secondary={ + props.highlight?.fields.text ? ( + + ) : ( + result.text + ) + } + /> + + {result.tags && + result.tags.map((tag: string) => )} + +
+ ); } diff --git a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx index 6ea5e4d1a2..bb43db352c 100644 --- a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx +++ b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx @@ -21,13 +21,7 @@ import { SearchDocument, } from '@backstage/plugin-search-common'; import { HighlightedSearchResultText } from '../HighlightedSearchResultText'; -import { - ListItem, - ListItemIcon, - ListItemText, - Box, - Divider, -} from '@material-ui/core'; +import { ListItem, ListItemIcon, ListItemText, Box } from '@material-ui/core'; import Typography from '@material-ui/core/Typography'; import { Link } from '@backstage/core-components'; @@ -43,6 +37,7 @@ export type DefaultResultListItemProps = { highlight?: ResultHighlight; rank?: number; lineClamp?: number; + toggleModal?: () => void; }; /** @@ -60,50 +55,47 @@ export const DefaultResultListItemComponent = ({ if (!result) return null; return ( - <> - - {icon && {icon}} - - {highlight?.fields.title ? ( - - ) : ( - result.title - )} - - } - secondary={ - - {highlight?.fields.text ? ( - - ) : ( - result.text - )} - - } - /> - {secondaryAction && {secondaryAction}} - - - + + {icon && {icon}} + + {highlight?.fields.title ? ( + + ) : ( + result.title + )} + + } + secondary={ + + {highlight?.fields.text ? ( + + ) : ( + result.text + )} + + } + /> + {secondaryAction && {secondaryAction}} + ); }; diff --git a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx index f058fd0a9e..50a4695b96 100644 --- a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx +++ b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx @@ -16,7 +16,6 @@ import React, { PropsWithChildren, ReactNode } from 'react'; import { - Divider, ListItem, ListItemIcon, ListItemText, @@ -156,13 +155,10 @@ export const TechDocsSearchResultListItem = ( const ListItemWrapper = ({ children }: PropsWithChildren<{}>) => asListItem ? ( - <> - - {icon && {icon}} -
{children}
- - - + + {icon && {icon}} +
{children}
+ ) : ( <>{children} ); From 77c85b4467efeeb7a456d8a555bae33f403c72e4 Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Fri, 20 Jan 2023 16:23:51 +0100 Subject: [PATCH 04/11] add aria label to tabs, remove disableRipple prop to indicate focus on tab when navigating through keyboard Signed-off-by: Emma Indal --- plugins/search/src/components/SearchType/SearchType.Tabs.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/search/src/components/SearchType/SearchType.Tabs.tsx b/plugins/search/src/components/SearchType/SearchType.Tabs.tsx index b686e2e1f1..9c741004e6 100644 --- a/plugins/search/src/components/SearchType/SearchType.Tabs.tsx +++ b/plugins/search/src/components/SearchType/SearchType.Tabs.tsx @@ -72,6 +72,7 @@ export const SearchTypeTabs = (props: SearchTypeTabsProps) => { return ( { From 8ce32c1ab8210855bec409ebfb88a2182693c0ec Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Fri, 20 Jan 2023 16:24:01 +0100 Subject: [PATCH 05/11] remove tabs padding Signed-off-by: Emma Indal --- plugins/search/src/components/SearchType/SearchType.Tabs.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/search/src/components/SearchType/SearchType.Tabs.tsx b/plugins/search/src/components/SearchType/SearchType.Tabs.tsx index 9c741004e6..57a311b00c 100644 --- a/plugins/search/src/components/SearchType/SearchType.Tabs.tsx +++ b/plugins/search/src/components/SearchType/SearchType.Tabs.tsx @@ -22,7 +22,6 @@ import { makeStyles, Tab, Tabs } from '@material-ui/core'; const useStyles = makeStyles((theme: BackstageTheme) => ({ tabs: { borderBottom: `1px solid ${theme.palette.textVerySubtle}`, - padding: theme.spacing(0, 4), }, tab: { height: '50px', From 66e2aab4c44fc36f7144501b5aa2b7fab62e5beb Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Mon, 23 Jan 2023 10:21:13 +0100 Subject: [PATCH 06/11] add changesets Signed-off-by: Emma Indal --- .changeset/serious-pigs-watch.md | 8 ++++++++ .changeset/tall-falcons-teach.md | 5 +++++ .changeset/violet-tips-rhyme.md | 5 +++++ 3 files changed, 18 insertions(+) create mode 100644 .changeset/serious-pigs-watch.md create mode 100644 .changeset/tall-falcons-teach.md create mode 100644 .changeset/violet-tips-rhyme.md diff --git a/.changeset/serious-pigs-watch.md b/.changeset/serious-pigs-watch.md new file mode 100644 index 0000000000..3deb3724b4 --- /dev/null +++ b/.changeset/serious-pigs-watch.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-techdocs': patch +'@backstage/plugin-catalog': patch +'@backstage/plugin-explore': patch +'@backstage/plugin-search-react': patch +--- + +`*SearchResultListItems` now accept optional ´toggleModal´ prop that is used to toggle the search modal on result click. diff --git a/.changeset/tall-falcons-teach.md b/.changeset/tall-falcons-teach.md new file mode 100644 index 0000000000..d6608a6597 --- /dev/null +++ b/.changeset/tall-falcons-teach.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-components': patch +--- + +Navigation items in mobile sidebar now have aria label. diff --git a/.changeset/violet-tips-rhyme.md b/.changeset/violet-tips-rhyme.md new file mode 100644 index 0000000000..c2d95cfbf9 --- /dev/null +++ b/.changeset/violet-tips-rhyme.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-search': patch +--- + +Updated colors for each tab used on search modal, to improve color contrast. Aria label added to tabs wrapper component. `disableRipple` property on the `Tab` component removed to improve keyboard navigation indicator. From f7e698636a208f8a19909616561c395dcec044df Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Tue, 31 Jan 2023 15:43:57 +0100 Subject: [PATCH 07/11] updates based on result list items extension changes Signed-off-by: Emma Indal --- .../app/src/components/search/SearchModal.tsx | 19 +--- .../CatalogSearchResultListItem.tsx | 10 +-- .../ToolSearchResultListItem.tsx | 5 +- .../DefaultResultListItem.test.tsx | 7 +- .../DefaultResultListItem.tsx | 8 +- .../SearchResult/SearchResult.test.tsx | 5 +- .../SearchResultGroup.test.tsx | 5 +- .../SearchResultList.test.tsx | 3 +- plugins/search-react/src/extensions.test.tsx | 16 ++-- plugins/search-react/src/extensions.tsx | 10 ++- .../StackOverflowSearchResultListItem.tsx | 86 ++++++++----------- .../TechDocsSearchResultListItem.tsx | 11 +-- 12 files changed, 77 insertions(+), 108 deletions(-) diff --git a/packages/app/src/components/search/SearchModal.tsx b/packages/app/src/components/search/SearchModal.tsx index e994262e66..ea5a156c55 100644 --- a/packages/app/src/components/search/SearchModal.tsx +++ b/packages/app/src/components/search/SearchModal.tsx @@ -82,22 +82,14 @@ export const SearchModal = ({ toggleModal }: { toggleModal: () => void }) => { searchBarRef?.current?.focus(); }); -<<<<<<< HEAD - const handleSearchResultClick = useCallback(() => { - toggleModal(); - setTimeout(focusContent, transitions.duration.leavingScreen); - }, [toggleModal, focusContent, transitions]); - -======= ->>>>>>> c224202137 (make sure result list is structured with list elements only) const handleSearchBarKeyDown = useCallback( (e: KeyboardEvent) => { if (e.key === 'Enter') { - handleSearchResultClick(); + toggleModal(); navigate(searchPagePath); } }, - [navigate, searchPagePath, handleSearchResultClick], + [navigate, searchPagePath, toggleModal], ); return ( @@ -181,7 +173,7 @@ export const SearchModal = ({ toggleModal }: { toggleModal: () => void }) => { alignItems="center" > - + void }) => { - + } /> } /> } /> diff --git a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx index 2a4ec300cd..541d7c7d3a 100644 --- a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx +++ b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx @@ -18,7 +18,6 @@ import React, { ReactNode } from 'react'; import { Box, Chip, - ListItem, ListItemIcon, ListItemText, makeStyles, @@ -51,7 +50,6 @@ export interface CatalogSearchResultListItemProps { result?: IndexableDocument; highlight?: ResultHighlight; rank?: number; - toggleModal?: () => void; } /** @public */ @@ -66,7 +64,7 @@ export function CatalogSearchResultListItem( if (!result) return null; return ( - + <> {props.icon && {props.icon}}
) : ( - result.title + result.text ) } /> @@ -104,6 +102,6 @@ export function CatalogSearchResultListItem( )}
- + ); } diff --git a/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx b/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx index fa57b93822..f3573007fc 100644 --- a/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx +++ b/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx @@ -18,7 +18,6 @@ import React, { ReactNode } from 'react'; import { Box, Chip, - ListItem, ListItemIcon, ListItemText, makeStyles, @@ -63,7 +62,7 @@ export function ToolSearchResultListItem(props: ToolSearchResultListItemProps) { if (!result) return null; return ( - + <> {props.icon && {props.icon}}
)}
- + ); } diff --git a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.test.tsx b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.test.tsx index e94d394639..aab105df48 100644 --- a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.test.tsx +++ b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.test.tsx @@ -35,9 +35,10 @@ describe('DefaultResultListItem', () => { it('Includes primary/secondary text (title / text)', async () => { await renderInTestApp(); - expect(screen.getByRole('listitem')).toHaveTextContent( - result.title + result.text, - ); + expect( + screen.getByRole('link', { name: result.title }), + ).toBeInTheDocument(); + expect(screen.getByText(result.text)).toBeInTheDocument(); }); it('should render icon if prop is specified', async () => { diff --git a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx index bb43db352c..eef7f799a6 100644 --- a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx +++ b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx @@ -21,12 +21,12 @@ import { SearchDocument, } from '@backstage/plugin-search-common'; import { HighlightedSearchResultText } from '../HighlightedSearchResultText'; -import { ListItem, ListItemIcon, ListItemText, Box } from '@material-ui/core'; +import { ListItemIcon, ListItemText, Box } from '@material-ui/core'; import Typography from '@material-ui/core/Typography'; import { Link } from '@backstage/core-components'; /** - * Props for {@link DefaultResultListItem} + * Props for {@link DefaultResultListItemComponent} * * @public */ @@ -55,7 +55,7 @@ export const DefaultResultListItemComponent = ({ if (!result) return null; return ( - + <> {icon && {icon}} {secondaryAction && {secondaryAction}} - + ); }; diff --git a/plugins/search-react/src/components/SearchResult/SearchResult.test.tsx b/plugins/search-react/src/components/SearchResult/SearchResult.test.tsx index 6a5a683295..4b8caa2780 100644 --- a/plugins/search-react/src/components/SearchResult/SearchResult.test.tsx +++ b/plugins/search-react/src/components/SearchResult/SearchResult.test.tsx @@ -16,8 +16,6 @@ import React from 'react'; import { waitFor } from '@testing-library/react'; - -import { ListItem } from '@material-ui/core'; import { wrapInTestApp, renderInTestApp, @@ -217,8 +215,7 @@ describe('SearchResult', () => { }).provide( createSearchResultListItemExtension({ name: 'SearchResultExtension', - component: async () => props => - Result: {props.result?.title}, + component: async () => props => <>Result: {props.result?.title}, }), ); diff --git a/plugins/search-react/src/components/SearchResultGroup/SearchResultGroup.test.tsx b/plugins/search-react/src/components/SearchResultGroup/SearchResultGroup.test.tsx index 728bd9113a..8a9e8986b8 100644 --- a/plugins/search-react/src/components/SearchResultGroup/SearchResultGroup.test.tsx +++ b/plugins/search-react/src/components/SearchResultGroup/SearchResultGroup.test.tsx @@ -18,7 +18,7 @@ import React from 'react'; import { screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { ListItem, MenuItem } from '@material-ui/core'; +import { MenuItem } from '@material-ui/core'; import DocsIcon from '@material-ui/icons/InsertDriveFile'; import { @@ -143,8 +143,7 @@ describe('SearchResultGroup', () => { }).provide( createSearchResultListItemExtension({ name: 'SearchResultGroupItemExtension', - component: async () => props => - Result: {props.result?.title}, + component: async () => props => <>Result: {props.result?.title}, }), ); diff --git a/plugins/search-react/src/components/SearchResultList/SearchResultList.test.tsx b/plugins/search-react/src/components/SearchResultList/SearchResultList.test.tsx index 7e8aa7bb33..941493aea1 100644 --- a/plugins/search-react/src/components/SearchResultList/SearchResultList.test.tsx +++ b/plugins/search-react/src/components/SearchResultList/SearchResultList.test.tsx @@ -226,8 +226,7 @@ describe('SearchResultList', () => { }).provide( createSearchResultListItemExtension({ name: 'SearchResultListItemExtension', - component: async () => props => - Result: {props.result?.title}, + component: async () => props => <>Result: {props.result?.title}, }), ); diff --git a/plugins/search-react/src/extensions.test.tsx b/plugins/search-react/src/extensions.test.tsx index 24c115e572..7e5070c738 100644 --- a/plugins/search-react/src/extensions.test.tsx +++ b/plugins/search-react/src/extensions.test.tsx @@ -18,7 +18,7 @@ import React from 'react'; import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { ListItem, ListItemText } from '@material-ui/core'; +import { ListItemText } from '@material-ui/core'; import { wrapInTestApp, @@ -75,9 +75,9 @@ const createExtension = ( predicate, component = async () => (props: { result?: SearchDocument }) => ( - + <> - + ), } = options; return plugin.provide( @@ -120,7 +120,7 @@ describe('extensions', () => { ); await userEvent.click( - screen.getByRole('button', { name: /Search Result 1/ }), + screen.getByRole('link', { name: /Search Result 1/ }), ); expect(analyticsApiMock.getEvents()[0]).toMatchObject({ @@ -146,9 +146,7 @@ describe('extensions', () => { expect(screen.getByText('Default')).toBeInTheDocument(); expect(screen.getByText('Search Result 1')).toBeInTheDocument(); - await userEvent.click( - screen.getByRole('button', { name: /Search Result 1/ }), - ); + await userEvent.click(screen.getByRole('listitem')); expect(analyticsApiMock.getEvents()[0]).toMatchObject({ action: 'discover', @@ -184,9 +182,9 @@ describe('extensions', () => { predicate: (result: SearchResult) => result.type === 'explore', component: async () => (props: { result?: SearchDocument }) => ( - + <> - + ), }); diff --git a/plugins/search-react/src/extensions.tsx b/plugins/search-react/src/extensions.tsx index 1e0619b14c..30611b0879 100644 --- a/plugins/search-react/src/extensions.tsx +++ b/plugins/search-react/src/extensions.tsx @@ -33,7 +33,7 @@ import { } from '@backstage/core-plugin-api'; import { SearchDocument, SearchResult } from '@backstage/plugin-search-common'; -import { Box, List, ListProps } from '@material-ui/core'; +import { ListItem, List, ListProps } from '@material-ui/core'; import { DefaultResultListItem } from './components'; @@ -105,9 +105,13 @@ const SearchResultListItemExtension = ({ }, [rank, result, noTrack, analytics]); return ( - + {children} - + ); }; diff --git a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx index 9971b0e7e5..932995b895 100644 --- a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx +++ b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx @@ -17,14 +17,7 @@ import React from 'react'; import _unescape from 'lodash/unescape'; import { Link } from '@backstage/core-components'; -import { - Divider, - ListItem, - ListItemText, - ListItemIcon, - Box, - Chip, -} from '@material-ui/core'; +import { ListItemText, ListItemIcon, Box, Chip } from '@material-ui/core'; import { useAnalytics } from '@backstage/core-plugin-api'; import { ResultHighlight } from '@backstage/plugin-search-common'; import { HighlightedSearchResultText } from '@backstage/plugin-search-react'; @@ -52,47 +45,44 @@ export const StackOverflowSearchResultListItem = ( return ( <> - - {props.icon && {props.icon}} - - - {highlight?.fields?.title ? ( - - ) : ( - _unescape(title) - )} - - } - secondary={ - highlight?.fields?.text ? ( - <> - Author:{' '} - - + {props.icon && {props.icon}} + + + {highlight?.fields?.title ? ( + ) : ( - `Author: ${text}` - ) - } - /> - - {tags && - tags.map((tag: string) => ( - - ))} - - - + _unescape(title) + )} + + } + secondary={ + highlight?.fields?.text ? ( + <> + Author:{' '} + + + ) : ( + `Author: ${text}` + ) + } + /> + + {tags && + tags.map((tag: string) => ( + + ))} + ); }; diff --git a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx index 50a4695b96..9bf8b76092 100644 --- a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx +++ b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx @@ -15,12 +15,7 @@ */ import React, { PropsWithChildren, ReactNode } from 'react'; -import { - ListItem, - ListItemIcon, - ListItemText, - makeStyles, -} from '@material-ui/core'; +import { ListItemIcon, ListItemText, makeStyles } from '@material-ui/core'; import Typography from '@material-ui/core/Typography'; import { Link } from '@backstage/core-components'; import { ResultHighlight } from '@backstage/plugin-search-common'; @@ -155,10 +150,10 @@ export const TechDocsSearchResultListItem = ( const ListItemWrapper = ({ children }: PropsWithChildren<{}>) => asListItem ? ( - + <> {icon && {icon}}
{children}
- + ) : ( <>{children} ); From 37e9ba8a7da8a7afe33c06ad7b22554a17253d65 Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Tue, 31 Jan 2023 15:44:28 +0100 Subject: [PATCH 08/11] adjust changeset for list items Signed-off-by: Emma Indal --- .changeset/serious-pigs-watch.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/serious-pigs-watch.md b/.changeset/serious-pigs-watch.md index 3deb3724b4..6736c64557 100644 --- a/.changeset/serious-pigs-watch.md +++ b/.changeset/serious-pigs-watch.md @@ -5,4 +5,6 @@ '@backstage/plugin-search-react': patch --- -`*SearchResultListItems` now accept optional ´toggleModal´ prop that is used to toggle the search modal on result click. +`ListItem` wrapper component moved to `SearchResultListItemExtension` for all `*SearchResultListItems`. This is to make sure the list only contains list elements. + +Note: If you have implemented a custom result list item, you can remove the list item wrapper to avoid duplicated `
  • ` elements. From c3798fd6bdfcc89ebdac5d61a61db0f0efdfc7ae Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Wed, 1 Feb 2023 09:17:32 +0100 Subject: [PATCH 09/11] update api reports Signed-off-by: Emma Indal --- plugins/explore/api-report.md | 2 ++ plugins/search-react/api-report.md | 1 + .../components/DefaultResultListItem/DefaultResultListItem.tsx | 2 +- .../src/components/SearchResultList/SearchResultList.test.tsx | 1 - 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/explore/api-report.md b/plugins/explore/api-report.md index e7adde4b77..02b917b9ed 100644 --- a/plugins/explore/api-report.md +++ b/plugins/explore/api-report.md @@ -139,5 +139,7 @@ export interface ToolSearchResultListItemProps { rank?: number; // (undocumented) result?: IndexableDocument; + // (undocumented) + toggleModal?: () => void; } ``` diff --git a/plugins/search-react/api-report.md b/plugins/search-react/api-report.md index ccf7a72e33..0d6598feca 100644 --- a/plugins/search-react/api-report.md +++ b/plugins/search-react/api-report.md @@ -55,6 +55,7 @@ export type DefaultResultListItemProps = { highlight?: ResultHighlight; rank?: number; lineClamp?: number; + toggleModal?: () => void; }; // @public (undocumented) diff --git a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx index eef7f799a6..cf89b6a9cd 100644 --- a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx +++ b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx @@ -26,7 +26,7 @@ import Typography from '@material-ui/core/Typography'; import { Link } from '@backstage/core-components'; /** - * Props for {@link DefaultResultListItemComponent} + * Props for {@link DefaultResultListItem} * * @public */ diff --git a/plugins/search-react/src/components/SearchResultList/SearchResultList.test.tsx b/plugins/search-react/src/components/SearchResultList/SearchResultList.test.tsx index 941493aea1..a0f5bc7830 100644 --- a/plugins/search-react/src/components/SearchResultList/SearchResultList.test.tsx +++ b/plugins/search-react/src/components/SearchResultList/SearchResultList.test.tsx @@ -17,7 +17,6 @@ import React from 'react'; import { screen, waitFor } from '@testing-library/react'; -import { ListItem } from '@material-ui/core'; import { TestApiProvider, renderWithEffects, From b031b821d509d5df7ecdac824191c4800fda6ab8 Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Wed, 1 Feb 2023 12:58:54 +0100 Subject: [PATCH 10/11] Apply suggestions from code review Co-authored-by: Camila Belo Signed-off-by: Emma Indal --- .changeset/serious-pigs-watch.md | 4 +- plugins/explore/api-report.md | 2 - .../ToolSearchResultListItem.tsx | 6 +- .../StackOverflowSearchResultListItem.tsx | 86 +++++++++++-------- 4 files changed, 53 insertions(+), 45 deletions(-) diff --git a/.changeset/serious-pigs-watch.md b/.changeset/serious-pigs-watch.md index 6736c64557..2efd8697f3 100644 --- a/.changeset/serious-pigs-watch.md +++ b/.changeset/serious-pigs-watch.md @@ -5,6 +5,6 @@ '@backstage/plugin-search-react': patch --- -`ListItem` wrapper component moved to `SearchResultListItemExtension` for all `*SearchResultListItems`. This is to make sure the list only contains list elements. +`ListItem` wrapper component moved to `SearchResultListItemExtension` for all `*SearchResultListItems` that are exported as extensions. This is to make sure the list only contains list elements. -Note: If you have implemented a custom result list item, you can remove the list item wrapper to avoid duplicated `
  • ` elements. +Note: If you have implemented a custom result list item, we recommend you to remove the list item wrapper to avoid nested `
  • ` elements. diff --git a/plugins/explore/api-report.md b/plugins/explore/api-report.md index 02b917b9ed..e7adde4b77 100644 --- a/plugins/explore/api-report.md +++ b/plugins/explore/api-report.md @@ -139,7 +139,5 @@ export interface ToolSearchResultListItemProps { rank?: number; // (undocumented) result?: IndexableDocument; - // (undocumented) - toggleModal?: () => void; } ``` diff --git a/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx b/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx index f3573007fc..eb12ba45d8 100644 --- a/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx +++ b/plugins/explore/src/components/ToolSearchResultListItem/ToolSearchResultListItem.tsx @@ -50,7 +50,6 @@ export interface ToolSearchResultListItemProps { result?: IndexableDocument; highlight?: ResultHighlight; rank?: number; - toggleModal?: () => void; } /** @public */ @@ -94,8 +93,9 @@ export function ToolSearchResultListItem(props: ToolSearchResultListItemProps) { } /> - {result.tags && - result.tags.map((tag: string) => )} + {result.tags?.map((tag: string) => ( + + ))} diff --git a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx index 932995b895..9971b0e7e5 100644 --- a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx +++ b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx @@ -17,7 +17,14 @@ import React from 'react'; import _unescape from 'lodash/unescape'; import { Link } from '@backstage/core-components'; -import { ListItemText, ListItemIcon, Box, Chip } from '@material-ui/core'; +import { + Divider, + ListItem, + ListItemText, + ListItemIcon, + Box, + Chip, +} from '@material-ui/core'; import { useAnalytics } from '@backstage/core-plugin-api'; import { ResultHighlight } from '@backstage/plugin-search-common'; import { HighlightedSearchResultText } from '@backstage/plugin-search-react'; @@ -45,44 +52,47 @@ export const StackOverflowSearchResultListItem = ( return ( <> - {props.icon && {props.icon}} - - - {highlight?.fields?.title ? ( - + + {props.icon && {props.icon}} + + + {highlight?.fields?.title ? ( + + ) : ( + _unescape(title) + )} + + } + secondary={ + highlight?.fields?.text ? ( + <> + Author:{' '} + + ) : ( - _unescape(title) - )} - - } - secondary={ - highlight?.fields?.text ? ( - <> - Author:{' '} - - - ) : ( - `Author: ${text}` - ) - } - /> - - {tags && - tags.map((tag: string) => ( - - ))} - + `Author: ${text}` + ) + } + /> + + {tags && + tags.map((tag: string) => ( + + ))} + + + ); }; From ce1220a4b8c9e7138c00ae3a18acae9898d8d7fb Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Wed, 1 Feb 2023 14:56:07 +0100 Subject: [PATCH 11/11] support custom list item props on search extensions Co-authored-by: Camila Belo Signed-off-by: Emma Indal --- docs/features/search/how-to-guides.md | 14 +++++++++ plugins/catalog/api-report.md | 3 +- plugins/catalog/src/plugin.ts | 7 +++-- plugins/explore/api-report.md | 3 +- plugins/explore/src/plugin.ts | 7 +++-- plugins/search-react/api-report.md | 11 +++++++ plugins/search-react/src/extensions.tsx | 39 +++++++++++++++---------- plugins/techdocs/api-report.md | 3 +- plugins/techdocs/src/plugin.ts | 7 +++-- 9 files changed, 70 insertions(+), 24 deletions(-) diff --git a/docs/features/search/how-to-guides.md b/docs/features/search/how-to-guides.md index 6f113630f6..b464168724 100644 --- a/docs/features/search/how-to-guides.md +++ b/docs/features/search/how-to-guides.md @@ -235,6 +235,20 @@ export const YourSearchResultListItemExtension = plugin.provide( ); ``` +If your list item accept props, you can extend the `SearchResultListItemExtensionProps` with your component specific props: + +```tsx +export const YourSearchResultListItemExtension: ( + props: SearchResultListItemExtensionProps, +) => JSX.Element | null = plugin.provide( + createSearchResultListItemExtension({ + name: 'YourSearchResultListItem', + component: () => + import('./components').then(m => m.YourSearchResultListItem), + }), +); +``` + Additionally, you can define a predicate function that receives a result and returns whether your extension should be used to render it or not: ```tsx diff --git a/plugins/catalog/api-report.md b/plugins/catalog/api-report.md index 901120bd22..6d746f6a7c 100644 --- a/plugins/catalog/api-report.md +++ b/plugins/catalog/api-report.md @@ -19,6 +19,7 @@ import { default as React_2 } from 'react'; import { ReactNode } from 'react'; import { ResultHighlight } from '@backstage/plugin-search-common'; import { RouteRef } from '@backstage/core-plugin-api'; +import { SearchResultListItemExtensionProps } from '@backstage/plugin-search-react'; import { StarredEntitiesApi } from '@backstage/plugin-catalog-react'; import { StorageApi } from '@backstage/core-plugin-api'; import { StyleRules } from '@material-ui/core/styles/withStyles'; @@ -108,7 +109,7 @@ export const catalogPlugin: BackstagePlugin< // @public (undocumented) export const CatalogSearchResultListItem: ( - props: CatalogSearchResultListItemProps, + props: SearchResultListItemExtensionProps, ) => JSX.Element | null; // @public diff --git a/plugins/catalog/src/plugin.ts b/plugins/catalog/src/plugin.ts index 12c22eb537..ea6ddcaff2 100644 --- a/plugins/catalog/src/plugin.ts +++ b/plugins/catalog/src/plugin.ts @@ -31,7 +31,10 @@ import { fetchApiRef, storageApiRef, } from '@backstage/core-plugin-api'; -import { createSearchResultListItemExtension } from '@backstage/plugin-search-react'; +import { + createSearchResultListItemExtension, + SearchResultListItemExtensionProps, +} from '@backstage/plugin-search-react'; import { DefaultStarredEntitiesApi } from './apis'; import { AboutCardProps } from './components/AboutCard'; import { DefaultCatalogPageProps } from './components/CatalogPage'; @@ -254,7 +257,7 @@ export const RelatedEntitiesCard: ( /** @public */ export const CatalogSearchResultListItem: ( - props: CatalogSearchResultListItemProps, + props: SearchResultListItemExtensionProps, ) => JSX.Element | null = catalogPlugin.provide( createSearchResultListItemExtension({ name: 'CatalogSearchResultListItem', diff --git a/plugins/explore/api-report.md b/plugins/explore/api-report.md index e7adde4b77..da960ae951 100644 --- a/plugins/explore/api-report.md +++ b/plugins/explore/api-report.md @@ -19,6 +19,7 @@ import { IndexableDocument } from '@backstage/plugin-search-common'; import { ReactNode } from 'react'; import { ResultHighlight } from '@backstage/plugin-search-common'; import { RouteRef } from '@backstage/core-plugin-api'; +import { SearchResultListItemExtensionProps } from '@backstage/plugin-search-react'; import { TabProps } from '@material-ui/core'; // @public @deprecated (undocumented) @@ -126,7 +127,7 @@ export const ToolExplorerContent: (props: { // @public (undocumented) export const ToolSearchResultListItem: ( - props: ToolSearchResultListItemProps, + props: SearchResultListItemExtensionProps, ) => JSX.Element | null; // @public diff --git a/plugins/explore/src/plugin.ts b/plugins/explore/src/plugin.ts index 5a4bec53c5..84e7657355 100644 --- a/plugins/explore/src/plugin.ts +++ b/plugins/explore/src/plugin.ts @@ -22,7 +22,10 @@ import { discoveryApiRef, fetchApiRef, } from '@backstage/core-plugin-api'; -import { createSearchResultListItemExtension } from '@backstage/plugin-search-react'; +import { + createSearchResultListItemExtension, + SearchResultListItemExtensionProps, +} from '@backstage/plugin-search-react'; import { ExploreClient, exploreApiRef } from './api'; import { ToolSearchResultListItemProps } from './components/ToolSearchResultListItem'; // import { exampleTools } from './util/examples'; @@ -70,7 +73,7 @@ export const explorePlugin = createPlugin({ /** @public */ export const ToolSearchResultListItem: ( - props: ToolSearchResultListItemProps, + props: SearchResultListItemExtensionProps, ) => JSX.Element | null = explorePlugin.provide( createSearchResultListItemExtension({ name: 'ToolSearchResultListItem', diff --git a/plugins/search-react/api-report.md b/plugins/search-react/api-report.md index 0d6598feca..375af20d5c 100644 --- a/plugins/search-react/api-report.md +++ b/plugins/search-react/api-report.md @@ -14,6 +14,7 @@ import { InputBaseProps } from '@material-ui/core'; import { JsonObject } from '@backstage/types'; import { JsonValue } from '@backstage/types'; import { LinkProps } from '@backstage/core-components'; +import { ListItemProps } from '@material-ui/core'; import { ListItemTextProps } from '@material-ui/core'; import { ListProps } from '@material-ui/core'; import { PropsWithChildren } from 'react'; @@ -392,6 +393,16 @@ export type SearchResultListItemExtensionOptions< predicate?: (result: SearchResult_2) => boolean; }; +// @public +export type SearchResultListItemExtensionProps = Props & + PropsWithChildren< + { + rank?: number; + result?: SearchDocument; + noTrack?: boolean; + } & Omit + >; + // @public export const SearchResultListItemExtensions: ( props: SearchResultListItemExtensionsProps, diff --git a/plugins/search-react/src/extensions.tsx b/plugins/search-react/src/extensions.tsx index 30611b0879..6c854e24e0 100644 --- a/plugins/search-react/src/extensions.tsx +++ b/plugins/search-react/src/extensions.tsx @@ -33,7 +33,7 @@ import { } from '@backstage/core-plugin-api'; import { SearchDocument, SearchResult } from '@backstage/plugin-search-common'; -import { ListItem, List, ListProps } from '@material-ui/core'; +import { ListItem, List, ListProps, ListItemProps } from '@material-ui/core'; import { DefaultResultListItem } from './components'; @@ -73,26 +73,34 @@ const findSearchResultListItemExtensionElement = ( }; /** - * @internal - * Props for {@link SearchResultListItemExtension}. + * @public + * Extends props for any search result list item extension */ -type SearchResultListItemExtensionProps = PropsWithChildren<{ - rank?: number; - result?: SearchDocument; - noTrack?: boolean; -}>; +export type SearchResultListItemExtensionProps = Props & + PropsWithChildren< + { + rank?: number; + result?: SearchDocument; + noTrack?: boolean; + } & Omit + >; /** * @internal * Extends children with extension capabilities. * @param props - see {@link SearchResultListItemExtensionProps}. */ -const SearchResultListItemExtension = ({ - rank, - result, - noTrack, - children, -}: SearchResultListItemExtensionProps) => { +const SearchResultListItemExtension = ( + props: SearchResultListItemExtensionProps, +) => { + const { + rank, + result, + noTrack, + children, + alignItems = 'flex-start', + ...rest + } = props; const analytics = useAnalytics(); const handleClickCapture = useCallback(() => { @@ -107,8 +115,9 @@ const SearchResultListItemExtension = ({ return ( {children} diff --git a/plugins/techdocs/api-report.md b/plugins/techdocs/api-report.md index ddc90d07bb..9452b38feb 100644 --- a/plugins/techdocs/api-report.md +++ b/plugins/techdocs/api-report.md @@ -19,6 +19,7 @@ import { default as React_2 } from 'react'; import { ReactNode } from 'react'; import { ResultHighlight } from '@backstage/plugin-search-common'; import { RouteRef } from '@backstage/core-plugin-api'; +import { SearchResultListItemExtensionProps } from '@backstage/plugin-search-react'; import { TableColumn } from '@backstage/core-components'; import { TableOptions } from '@backstage/core-components'; import { TableProps } from '@backstage/core-components'; @@ -400,7 +401,7 @@ export type TechDocsSearchProps = { // @public export const TechDocsSearchResultListItem: ( - props: TechDocsSearchResultListItemProps, + props: SearchResultListItemExtensionProps, ) => JSX.Element | null; // @public diff --git a/plugins/techdocs/src/plugin.ts b/plugins/techdocs/src/plugin.ts index 331eefe121..bdd4814898 100644 --- a/plugins/techdocs/src/plugin.ts +++ b/plugins/techdocs/src/plugin.ts @@ -33,7 +33,10 @@ import { fetchApiRef, identityApiRef, } from '@backstage/core-plugin-api'; -import { createSearchResultListItemExtension } from '@backstage/plugin-search-react'; +import { + createSearchResultListItemExtension, + SearchResultListItemExtensionProps, +} from '@backstage/plugin-search-react'; import { TechDocsSearchResultListItemProps } from './search/components/TechDocsSearchResultListItem'; /** @@ -162,7 +165,7 @@ export const TechDocsReaderPage = techdocsPlugin.provide( * @public */ export const TechDocsSearchResultListItem: ( - props: TechDocsSearchResultListItemProps, + props: SearchResultListItemExtensionProps, ) => JSX.Element | null = techdocsPlugin.provide( createSearchResultListItemExtension({ name: 'TechDocsSearchResultListItem',