Revert "Refactor FilteredActionList to address a11y violations and us…

…e new ActionList. (#3247)" (#3349) This reverts commit e865e3e.
primer · Jun 6, 2023 · a6b4169 · a6b4169
1 parent 9669267
commit a6b4169
Show file tree

Hide file tree

Showing 12 changed files with 127 additions and 165 deletions.
diff --git a/.changeset/weak-jokes-chew.md b/.changeset/weak-jokes-chew.md
diff --git a/docs/content/SelectPanel.mdx b/docs/content/SelectPanel.mdx
@@ -12,18 +12,20 @@ A `SelectPanel` provides an anchor that will open an overlay with a list of sele
 
 ```javascript live noinline
 function getColorCircle(color) {
-  return (
-    <Box
-      borderWidth="1px"
-      borderStyle="solid"
-      bg={color}
-      borderColor={color}
-      width={14}
-      height={14}
-      borderRadius={10}
-      margin="auto"
-    />
-  )
+  return function () {
+    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        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        showItemDividers={true}\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,8 +69,7 @@ export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
           value={{
             variant,
             selectionVariant: selectionVariant || containerSelectionVariant,
-            // @ts-ignore showItemDividers may be passed by some components until next major.
-            showDividers: showDividers || !!props.showItemDividers,
+            showDividers,
             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, ItemInput} from '../FilteredActionList'
+import {FilteredActionList} from '../FilteredActionList'
 import BaseStyles from '../BaseStyles'
 import Box from '../Box'
 
@@ -26,33 +26,35 @@ const meta: Meta = {
 export default meta
 
 function getColorCircle(color: string) {
-  return (
-    <Box
-      bg={color}
-      borderColor={color}
-      width={14}
-      height={14}
-      borderRadius={10}
-      margin="auto"
-      borderWidth="1px"
-      borderStyle="solid"
-    />
-  )
+  return function () {
+    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'},
-] as ItemInput[]
+  {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 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,70 +1,40 @@
+import type {ScrollIntoViewOptions} from '@primer/behaviors'
+import {scrollIntoView} from '@primer/behaviors'
 import React, {KeyboardEventHandler, useCallback, useEffect, useRef} from 'react'
-import TextInput, {TextInputProps} from '../TextInput'
+import styled from 'styled-components'
 import Box from '../Box'
-import {ActionList, ActionListProps, ActionListItemProps} from '../ActionList'
 import Spinner from '../Spinner'
-import {useFocusZone} from '../hooks/useFocusZone'
-import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
-import styled from 'styled-components'
+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 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 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 {
+export interface FilteredActionListProps
+  extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
+    ListPropsBase,
+    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,
@@ -87,7 +57,7 @@ export function FilteredActionList({
   )
 
   const scrollContainerRef = useRef<HTMLDivElement>(null)
-  const listContainerRef = useRef<HTMLUListElement>(null)
+  const listContainerRef = useRef<HTMLDivElement>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
   const activeDescendantRef = useRef<HTMLElement>()
   const listId = useId()
@@ -114,7 +84,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) {
@@ -162,15 +132,7 @@ export function FilteredActionList({
             <Spinner />
           </Box>
         ) : (
-          <ActionList
-            ref={listContainerRef}
-            {...listProps}
-            role="listbox"
-            id={listId}
-            aria-label={`${placeholderText} options`}
-          >
-            {items.map(i => renderFn(i))}
-          </ActionList>
+          <ActionList ref={listContainerRef} items={items} {...listProps} role="listbox" id={listId} />
         )}
       </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, ItemInput} from './FilteredActionList'
+export type {FilteredActionListProps} 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 '../FilteredActionList'
+import {ItemInput} from '../deprecated/ActionList/List'
 import {SelectPanel} from './SelectPanel'
 import {TriangleDownIcon} from '@primer/octicons-react'
 import type {OverlayProps} from '../Overlay'
@@ -14,28 +14,30 @@ export default {
 } as ComponentMeta<typeof SelectPanel>
 
 function getColorCircle(color: string) {
-  return (
-    <Box
-      bg={color}
-      borderColor={color}
-      width={14}
-      height={14}
-      borderRadius={10}
-      margin="auto"
-      borderWidth="1px"
-      borderStyle="solid"
-    />
-  )
+  return function () {
+    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 = () => {
@@ -61,7 +63,6 @@ export const SingleSelectStory = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
-        showDividers={true}
         showItemDividers={true}
         overlayProps={{width: 'small', height: 'xsmall'}}
       />
@@ -93,6 +94,7 @@ export const ExternalAnchorStory = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
+        showItemDividers={true}
         overlayProps={{width: 'small', height: 'xsmall'}}
       />
     </>
@@ -123,6 +125,7 @@ export const SelectPanelHeightInitialWithOverflowingItemsStory = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
+        showItemDividers={true}
         overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
       />
     </>
@@ -154,6 +157,7 @@ export const SelectPanelHeightInitialWithUnderflowingItemsStory = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
+        showItemDividers={true}
         overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
       />
     </>
@@ -198,6 +202,7 @@ export const SelectPanelHeightInitialWithUnderflowingItemsAfterFetch = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
+        showItemDividers={true}
         overlayProps={{width: 'small', height, maxHeight: 'xsmall'}}
       />
     </>
@@ -229,6 +234,7 @@ export const SelectPanelAboveTallBody = () => {
         selected={selected}
         onSelectedChange={setSelected}
         onFilterChange={setFilter}
+        showItemDividers={true}
         overlayProps={{width: 'small', height: 'xsmall'}}
       />
       <div
@@ -270,6 +276,7 @@ export const SelectPanelHeightAndScroll = () => {
         selected={selectedA}
         onSelectedChange={setSelectedA}
         onFilterChange={setFilter}
+        showItemDividers={true}
         overlayProps={{height: 'medium'}}
       />
       <h2>With height:auto, maxheight:medium</h2>
@@ -286,6 +293,7 @@ export const SelectPanelHeightAndScroll = () => {
         selected={selectedB}
         onSelectedChange={setSelectedB}
         onFilterChange={setFilter}
+        showItemDividers={true}
         overlayProps={{
           height: 'auto',
           maxHeight: 'medium',