Refactor FilteredActionList to address a11y violations and use new Ac…

…tionList. (#3247) * Update FilteredActionList to use non-deprecated ActionList. * Use non-deprecated ActionList in FilteredActionList. * Fix a11y issues in FilteredActionList story. * Add prop to hide selection component if needed. * Remove unused hook import. * Get SavedReplies to look as it did with deprecated ActionList. * Fix failing test. * Create weak-jokes-chew.md * Update generated/components.json * Fix themePreval snapshot. * Linting fixes. * Fix type-check errors. * Update themePreval snapshot again. * Fix themePreval snapshot to match origin. Unsure why it's not generating the same way. * Hide selections in MarkdownEditor saved replies. * Remove hideSelection prop and add defaultRenderFn to FilteredActionList so these don't have to be defined manually everywhere. * Fix selection rendering (needed explicit selected boolean) and fix SelectPanel docs. * Pass selectionVariant illegally to SelectPanel in src/MarkdownEditor/_SavedReplies.tsx so Selection components don't render. * Remove remaining references of hideSelection prop. * Update changeset to reflect that changes impact SelectPanel. * Remove renderFn prop from SelectPanel and use default for FilteredActionList, which seems to cover most cases. * Fix truncation in SavedReplies descriptions. * Update generated/components.json * Fix linting error. * Don't make renderFn a prop (if we need to make this configurable, we can expose it later. Use maxWidth 100% for SavedReplies truncation. * Use showDividers prop in SelectPanel story. * Formatting. * Add temporary support for showItemDividers prop to SelectPanel to keep backwards compatibility. * Support passing deprecated showItemDividers prop in ActionList. --------- Co-authored-by: radglob <radglob@users.noreply.github.com>
primer · May 26, 2023 · e865e3e · e865e3e
1 parent 1fd6d32
commit e865e3e
Show file tree

Hide file tree

Showing 12 changed files with 165 additions and 127 deletions.
diff --git a/.changeset/weak-jokes-chew.md b/.changeset/weak-jokes-chew.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+FilteredActionList now uses new ActionList as a base, and SelectPanel reflects those changes.
diff --git a/docs/content/SelectPanel.mdx b/docs/content/SelectPanel.mdx
@@ -12,20 +12,18 @@ A `SelectPanel` provides an anchor that will open an overlay with a list of sele
 
 ```javascript live noinline
 function getColorCircle(color) {
-  return function () {
-    return (
-      <Box
-        borderWidth="1px"
-        borderStyle="solid"
-        bg={color}
-        borderColor={color}
-        width={14}
-        height={14}
-        borderRadius={10}
-        margin="auto"
-      />
-    )
-  }
+  return (
+    <Box
+      borderWidth="1px"
+      borderStyle="solid"
+      bg={color}
+      borderColor={color}
+      width={14}
+      height={14}
+      borderRadius={10}
+      margin="auto"
+    />
+  )
 }
 
 const items = [

diff --git a/generated/components.json b/generated/components.json
@@ -3626,7 +3626,7 @@
       "stories": [
         {
           "id": "components-selectpanel--default",
-          "code": "() => {\n  const [selected, setSelected] = React.useState<ItemInput[]>([\n    items[0],\n    items[1],\n  ])\n  const [filter, setFilter] = React.useState('')\n  const filteredItems = items.filter((item) =>\n    item.text.toLowerCase().startsWith(filter.toLowerCase()),\n  )\n  const [open, setOpen] = useState(false)\n  return (\n    <>\n      <h1>Multi Select Panel</h1>\n      <div>Please select labels that describe your issue:</div>\n      <SelectPanel\n        title=\"Select labels\"\n        renderAnchor={({\n          children,\n          'aria-labelledby': ariaLabelledBy,\n          ...anchorProps\n        }) => (\n          <Button\n            trailingAction={TriangleDownIcon}\n            aria-labelledby={` ${ariaLabelledBy}`}\n            {...anchorProps}\n          >\n            {children ?? 'Select Labels'}\n          </Button>\n        )}\n        placeholderText=\"Filter labels\"\n        open={open}\n        onOpenChange={setOpen}\n        items={filteredItems}\n        selected={selected}\n        onSelectedChange={setSelected}\n        onFilterChange={setFilter}\n        showItemDividers={true}\n        overlayProps={{\n          width: 'small',\n          height: 'xsmall',\n        }}\n      />\n    </>\n  )\n}"
+          "code": "() => {\n  const [selected, setSelected] = React.useState<ItemInput[]>([\n    items[0],\n    items[1],\n  ])\n  const [filter, setFilter] = React.useState('')\n  const filteredItems = items.filter((item) =>\n    item.text.toLowerCase().startsWith(filter.toLowerCase()),\n  )\n  const [open, setOpen] = useState(false)\n  return (\n    <>\n      <h1>Multi Select Panel</h1>\n      <div>Please select labels that describe your issue:</div>\n      <SelectPanel\n        title=\"Select labels\"\n        renderAnchor={({\n          children,\n          'aria-labelledby': ariaLabelledBy,\n          ...anchorProps\n        }) => (\n          <Button\n            trailingAction={TriangleDownIcon}\n            aria-labelledby={` ${ariaLabelledBy}`}\n            {...anchorProps}\n          >\n            {children ?? 'Select Labels'}\n          </Button>\n        )}\n        placeholderText=\"Filter labels\"\n        open={open}\n        onOpenChange={setOpen}\n        items={filteredItems}\n        selected={selected}\n        onSelectedChange={setSelected}\n        onFilterChange={setFilter}\n        overlayProps={{\n          width: 'small',\n          height: 'xsmall',\n        }}\n      />\n    </>\n  )\n}"
         }
       ],
       "props": [

diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx
@@ -69,7 +69,8 @@ export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
           value={{
             variant,
             selectionVariant: selectionVariant || containerSelectionVariant,
-            showDividers,
+            // @ts-ignore showItemDividers may be passed by some components until next major.
+            showDividers: showDividers || !!props.showItemDividers,
             role: role || listRole,
             headingId,
           }}

diff --git a/src/FilteredActionList/FilteredActionList.stories.tsx b/src/FilteredActionList/FilteredActionList.stories.tsx
@@ -1,7 +1,7 @@
 import {Meta} from '@storybook/react'
 import React from 'react'
 import {ThemeProvider} from '..'
-import {FilteredActionList} from '../FilteredActionList'
+import {FilteredActionList, ItemInput} from '../FilteredActionList'
 import BaseStyles from '../BaseStyles'
 import Box from '../Box'
 
@@ -26,35 +26,33 @@ const meta: Meta = {
 export default meta
 
 function getColorCircle(color: string) {
-  return function () {
-    return (
-      <Box
-        bg={color}
-        borderColor={color}
-        width={14}
-        height={14}
-        borderRadius={10}
-        margin="auto"
-        borderWidth="1px"
-        borderStyle="solid"
-      />
-    )
-  }
+  return (
+    <Box
+      bg={color}
+      borderColor={color}
+      width={14}
+      height={14}
+      borderRadius={10}
+      margin="auto"
+      borderWidth="1px"
+      borderStyle="solid"
+    />
+  )
 }
 
 const items = [
-  {leadingVisual: getColorCircle('#a2eeef'), text: 'enhancement', id: 1},
-  {leadingVisual: getColorCircle('#d73a4a'), text: 'bug', id: 2},
-  {leadingVisual: getColorCircle('#0cf478'), text: 'good first issue', id: 3},
-  {leadingVisual: getColorCircle('#ffd78e'), text: 'design', id: 4},
-  {leadingVisual: getColorCircle('#ff0000'), text: 'blocker', id: 5},
-  {leadingVisual: getColorCircle('#a4f287'), text: 'backend', id: 6},
-  {leadingVisual: getColorCircle('#8dc6fc'), text: 'frontend', id: 7},
-]
+  {leadingVisual: getColorCircle('#a2eeef'), text: 'enhancement', id: '1'},
+  {leadingVisual: getColorCircle('#d73a4a'), text: 'bug', id: '2'},
+  {leadingVisual: getColorCircle('#0cf478'), text: 'good first issue', id: '3'},
+  {leadingVisual: getColorCircle('#ffd78e'), text: 'design', id: '4'},
+  {leadingVisual: getColorCircle('#ff0000'), text: 'blocker', id: '5'},
+  {leadingVisual: getColorCircle('#a4f287'), text: 'backend', id: '6'},
+  {leadingVisual: getColorCircle('#8dc6fc'), text: 'frontend', id: '7'},
+] as ItemInput[]
 
 export function Default(): JSX.Element {
   const [filter, setFilter] = React.useState('')
-  const filteredItems = items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))
+  const filteredItems = items.filter(item => item.text?.toLowerCase().startsWith(filter.toLowerCase()))
 
   return (
     <>

diff --git a/src/FilteredActionList/FilteredActionList.tsx b/src/FilteredActionList/FilteredActionList.tsx
@@ -1,40 +1,70 @@
-import type {ScrollIntoViewOptions} from '@primer/behaviors'
-import {scrollIntoView} from '@primer/behaviors'
 import React, {KeyboardEventHandler, useCallback, useEffect, useRef} from 'react'
-import styled from 'styled-components'
+import TextInput, {TextInputProps} from '../TextInput'
 import Box from '../Box'
+import {ActionList, ActionListProps, ActionListItemProps} from '../ActionList'
 import Spinner from '../Spinner'
-import TextInput, {TextInputProps} from '../TextInput'
-import {get} from '../constants'
-import {ActionList} from '../deprecated/ActionList'
-import {GroupedListProps, ListPropsBase} from '../deprecated/ActionList/List'
 import {useFocusZone} from '../hooks/useFocusZone'
-import {useId} from '../hooks/useId'
-import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
 import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
+import styled from 'styled-components'
+import {get} from '../constants'
+import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
 import useScrollFlash from '../hooks/useScrollFlash'
+import {scrollIntoView} from '@primer/behaviors'
+import type {ScrollIntoViewOptions} from '@primer/behaviors'
+import {useId} from '../hooks/useId'
 import {VisuallyHidden} from '../internal/components/VisuallyHidden'
 import {SxProp} from '../sx'
 
 const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
 
-export interface FilteredActionListProps
-  extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
-    ListPropsBase,
-    SxProp {
+export type ItemInput = Partial<
+  ActionListItemProps & {
+    description?: string | React.ReactElement
+    descriptionVariant?: 'inline' | 'block'
+    leadingVisual?: JSX.Element
+    onAction?: (itemFromAction: ItemInput, event: React.MouseEvent) => void
+    selected?: boolean
+    text?: string
+    trailingVisual?: string
+  }
+>
+
+export interface FilteredActionListProps extends ActionListProps, SxProp {
   loading?: boolean
   placeholderText?: string
   filterValue?: string
   onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
   textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
   inputRef?: React.RefObject<HTMLInputElement>
+  items: ItemInput[]
 }
 
 const StyledHeader = styled.div`
   box-shadow: 0 1px 0 ${get('colors.border.default')};
   z-index: 1;
 `
 
+const renderFn = ({
+  description,
+  descriptionVariant,
+  id,
+  sx,
+  text,
+  trailingVisual,
+  leadingVisual,
+  onSelect,
+  selected,
+}: ItemInput): React.ReactElement => {
+  return (
+    <ActionList.Item key={id} sx={sx} role="option" onSelect={onSelect} selected={selected}>
+      {!!leadingVisual && <ActionList.LeadingVisual>{leadingVisual}</ActionList.LeadingVisual>}
+      <Box>{text ? text : null}</Box>
+      {description ? <ActionList.Description variant={descriptionVariant}>{description}</ActionList.Description> : null}
+      {!!trailingVisual && <ActionList.TrailingVisual>{trailingVisual}</ActionList.TrailingVisual>}
+    </ActionList.Item>
+  )
+}
+
 export function FilteredActionList({
   loading = false,
   placeholderText,
@@ -57,7 +87,7 @@ export function FilteredActionList({
   )
 
   const scrollContainerRef = useRef<HTMLDivElement>(null)
-  const listContainerRef = useRef<HTMLDivElement>(null)
+  const listContainerRef = useRef<HTMLUListElement>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
   const activeDescendantRef = useRef<HTMLElement>()
   const listId = useId()
@@ -84,7 +114,7 @@ export function FilteredActionList({
         return !(element instanceof HTMLInputElement)
       },
       activeDescendantFocus: inputRef,
-      onActiveDescendantChanged: (current, previous, directlyActivated) => {
+      onActiveDescendantChanged: (current, _previous, directlyActivated) => {
         activeDescendantRef.current = current
 
         if (current && scrollContainerRef.current && directlyActivated) {
@@ -132,7 +162,15 @@ export function FilteredActionList({
             <Spinner />
           </Box>
         ) : (
-          <ActionList ref={listContainerRef} items={items} {...listProps} role="listbox" id={listId} />
+          <ActionList
+            ref={listContainerRef}
+            {...listProps}
+            role="listbox"
+            id={listId}
+            aria-label={`${placeholderText} options`}
+          >
+            {items.map(i => renderFn(i))}
+          </ActionList>
         )}
       </Box>
     </Box>

diff --git a/src/FilteredActionList/index.ts b/src/FilteredActionList/index.ts
@@ -1,2 +1,2 @@
 export {FilteredActionList} from './FilteredActionList'
-export type {FilteredActionListProps} from './FilteredActionList'
+export type {FilteredActionListProps, ItemInput} from './FilteredActionList'
diff --git a/src/SelectPanel/SelectPanel.features.stories.tsx b/src/SelectPanel/SelectPanel.features.stories.tsx
@@ -3,7 +3,7 @@ import {ComponentMeta} from '@storybook/react'
 
 import Box from '../Box'
 import {Button} from '../Button'
-import {ItemInput} from '../deprecated/ActionList/List'
+import {ItemInput} from '../FilteredActionList'
 import {SelectPanel} from './SelectPanel'
 import {TriangleDownIcon} from '@primer/octicons-react'
 import type {OverlayProps} from '../Overlay'
@@ -14,30 +14,28 @@ export default {
 } as ComponentMeta<typeof SelectPanel>
 
 function getColorCircle(color: string) {
-  return function () {
-    return (
-      <Box
-        bg={color}
-        borderColor={color}
-        width={14}
-        height={14}
-        borderRadius={10}
-        margin="auto"
-        borderWidth="1px"
-        borderStyle="solid"
-      />
-    )
-  }
+  return (
+    <Box
+      bg={color}
+      borderColor={color}
+      width={14}
+      height={14}
+      borderRadius={10}
+      margin="auto"
+      borderWidth="1px"
+      borderStyle="solid"
+    />
+  )
 }
 
 const items = [
-  {leadingVisual: getColorCircle('#a2eeef'), text: 'enhancement', id: 1},
-  {leadingVisual: getColorCircle('#d73a4a'), text: 'bug', id: 2},
-  {leadingVisual: getColorCircle('#0cf478'), text: 'good first issue', id: 3},
-  {leadingVisual: getColorCircle('#ffd78e'), text: 'design', id: 4},
-  {leadingVisual: getColorCircle('#ff0000'), text: 'blocker', id: 5},
-  {leadingVisual: getColorCircle('#a4f287'), text: 'backend', id: 6},
-  {leadingVisual: getColorCircle('#8dc6fc'), text: 'frontend', id: 7},
+  {leadingVisual: getColorCircle('#a2eeef'), text: 'enhancement', id: '1'},
+  {leadingVisual: getColorCircle('#d73a4a'), text: 'bug', id: '2'},
+  {leadingVisual: getColorCircle('#0cf478'), text: 'good first issue', id: '3'},
+  {leadingVisual: getColorCircle('#ffd78e'), text: 'design', id: '4'},
+  {leadingVisual: getColorCircle('#ff0000'), text: 'blocker', id: '5'},
+  {leadingVisual: getColorCircle('#a4f287'), text: 'backend', id: '6'},
+  {leadingVisual: getColorCircle('#8dc6fc'), text: 'frontend', id: '7'},
 ]
 
 export const SingleSelectStory = () => {
@@ -63,6 +61,7 @@ export const SingleSelectStory = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
+        showDividers={true}
         showItemDividers={true}
         overlayProps={{width: 'small', height: 'xsmall'}}
       />
@@ -94,7 +93,6 @@ export const ExternalAnchorStory = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
-        showItemDividers={true}
         overlayProps={{width: 'small', height: 'xsmall'}}
       />
     </>
@@ -125,7 +123,6 @@ export const SelectPanelHeightInitialWithOverflowingItemsStory = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
-        showItemDividers={true}
         overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
       />
     </>
@@ -157,7 +154,6 @@ export const SelectPanelHeightInitialWithUnderflowingItemsStory = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
-        showItemDividers={true}
         overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
       />
     </>
@@ -202,7 +198,6 @@ export const SelectPanelHeightInitialWithUnderflowingItemsAfterFetch = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
-        showItemDividers={true}
         overlayProps={{width: 'small', height, maxHeight: 'xsmall'}}
       />
     </>
@@ -234,7 +229,6 @@ export const SelectPanelAboveTallBody = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
-        showItemDividers={true}
         overlayProps={{width: 'small', height: 'xsmall'}}
       />
       <div
@@ -276,7 +270,6 @@ export const SelectPanelHeightAndScroll = () => {
         selected={selectedA}
         onSelectedChange={setSelectedA}
         onFilterChange={setFilter}
-        showItemDividers={true}
         overlayProps={{height: 'medium'}}
       />
       <h2>With height:auto, maxheight:medium</h2>
@@ -293,7 +286,6 @@ export const SelectPanelHeightAndScroll = () => {
         selected={selectedB}
         onSelectedChange={setSelectedB}
         onFilterChange={setFilter}
-        showItemDividers={true}
         overlayProps={{
           height: 'auto',
           maxHeight: 'medium',