-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ActionsList filtering #9907
ActionsList filtering #9907
Changes from all commits
e0d813a
cc5a435
c4ee485
3353f29
42ab0e6
0ada929
50756c0
c326e8f
f4461ec
0ec5cda
879494e
bcedf45
5f10916
deb201d
29a773d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@shopify/polaris': minor | ||
--- | ||
|
||
Add a search field to filter ActionList that have more than 10 items |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,13 @@ | |
"rollupButton": "View actions" | ||
} | ||
}, | ||
"ActionList": { | ||
"SearchField": { | ||
"clearButtonLabel": "Clear", | ||
"search": "Search", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Localization quality issue found The following issues may affect the quality of localized translations if they are not addressed:
Please look out for other instances of this issue in your PR and fix them as well if possible. Questions about these messages? Hop in the #help-localization Slack channel. |
||
"placeholder": "Search actions" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Localization quality issue found The following issues may affect the quality of localized translations if they are not addressed:
Please look out for other instances of this issue in your PR and fix them as well if possible. Questions about these messages? Hop in the #help-localization Slack channel. |
||
} | ||
}, | ||
"Avatar": { | ||
"label": "Avatar", | ||
"labelWithInitials": "Avatar with initials {initials}" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,16 @@ | ||
import React, {useRef} from 'react'; | ||
import React, {useMemo, useRef, useState} from 'react'; | ||
|
||
import type {ActionListItemDescriptor, ActionListSection} from '../../types'; | ||
import {Key} from '../../types'; | ||
import { | ||
wrapFocusNextFocusableMenuItem, | ||
wrapFocusPreviousFocusableMenuItem, | ||
} from '../../utilities/focus'; | ||
import {KeypressListener} from '../KeypressListener'; | ||
import {Key} from '../../types'; | ||
import type {ActionListItemDescriptor, ActionListSection} from '../../types'; | ||
import {Box} from '../Box'; | ||
import {KeypressListener} from '../KeypressListener'; | ||
import {useI18n} from '../../utilities/i18n'; | ||
|
||
import {Section, Item} from './components'; | ||
import {SearchField, Item, Section} from './components'; | ||
import type {ItemProps} from './components'; | ||
|
||
export interface ActionListProps { | ||
|
@@ -31,8 +32,11 @@ export function ActionList({ | |
actionRole, | ||
onActionAnyItem, | ||
}: ActionListProps) { | ||
const i18n = useI18n(); | ||
|
||
let finalSections: readonly ActionListSection[] = []; | ||
const actionListRef = useRef<HTMLDivElement & HTMLUListElement>(null); | ||
const [searchText, setSeachText] = useState(''); | ||
|
||
if (items) { | ||
finalSections = [{items}, ...sections]; | ||
|
@@ -46,7 +50,14 @@ export function ActionList({ | |
const elementTabIndex = | ||
hasMultipleSections && actionRole === 'menuitem' ? -1 : undefined; | ||
|
||
const sectionMarkup = finalSections.map((section, index) => { | ||
const filteredSections = finalSections?.map((section) => ({ | ||
...section, | ||
items: section.items.filter((item) => | ||
item.content?.toLowerCase().includes(searchText.toLowerCase()), | ||
), | ||
})); | ||
|
||
const sectionMarkup = filteredSections.map((section, index) => { | ||
return section.items.length > 0 ? ( | ||
<Section | ||
key={typeof section.title === 'string' ? section.title : index} | ||
|
@@ -99,16 +110,47 @@ export function ActionList({ | |
</> | ||
) : null; | ||
|
||
const totalActions = | ||
finalSections?.reduce( | ||
(acc: number, section) => acc + section.items.length, | ||
0, | ||
) || 0; | ||
|
||
const totalFilteredActions = useMemo(() => { | ||
const totalSectionItems = | ||
filteredSections?.reduce( | ||
(acc: number, section) => acc + section.items.length, | ||
0, | ||
) || 0; | ||
|
||
return totalSectionItems; | ||
}, [filteredSections]); | ||
|
||
const showSearch = totalActions >= 8; | ||
|
||
return ( | ||
<Box | ||
as={hasMultipleSections ? 'ul' : 'div'} | ||
ref={actionListRef} | ||
role={elementRole} | ||
tabIndex={elementTabIndex} | ||
> | ||
{listeners} | ||
{sectionMarkup} | ||
</Box> | ||
<> | ||
{showSearch && ( | ||
<Box padding="2" paddingBlockEnd={totalFilteredActions > 0 ? '0' : '2'}> | ||
<SearchField | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have a generic text input component that could be leveraged? Seems like we shouldn't need to recreate a basic input for the ActionList specifically 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TextField and SearchField were both giving me circular dependency issues because one or more of their child components is using ActionList. They also didn't have the magnifying glass icon or the option to add it. The closest was the Topbar SearchField, but it was not meant to be reusable and the style wasn't quite what was needed either unfortunately |
||
placeholder={i18n.translate( | ||
'Polaris.ActionList.SearchField.placeholder', | ||
)} | ||
value={searchText} | ||
onChange={(value) => setSeachText(value)} | ||
/> | ||
</Box> | ||
)} | ||
<Box | ||
as={hasMultipleSections ? 'ul' : 'div'} | ||
ref={actionListRef} | ||
role={elementRole} | ||
tabIndex={elementTabIndex} | ||
> | ||
{listeners} | ||
{sectionMarkup} | ||
</Box> | ||
</> | ||
); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Localization quality issue found
The following issues may affect the quality of localized translations if they are not addressed:
Clear
for keyPolaris.ActionList.SearchField.clearButtonLabel
is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.Please look out for other instances of this issue in your PR and fix them as well if possible.
Questions about these messages? Hop in the #help-localization Slack channel.