ActionList.Item: Bug fix to support both inline and block description at the same time

.changeset/pretty-trainers-rhyme.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+ActionList: Fix bug that did not allow both inline and block description at the same time

src/ActionList/Item.tsx

@@ -36,8 +36,10 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
  const [slots, childrenWithoutSlots] = useSlots(props.children, {
  leadingVisual: LeadingVisual,
  trailingVisual: TrailingVisual,
- description: Description,
+ blockDescription: [Description, props => props.variant === 'block'],
+ inlineDescription: [Description, props => props.variant !== 'block'],
  })
+
  const {
  variant: listVariant,
  role: listRole,
@@ -204,10 +206,8 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
  onKeyPress: keyPressHandler,
  'aria-disabled': disabled ? true : undefined,
  tabIndex: disabled ? undefined : 0,
- 'aria-labelledby': `${labelId} ${
- slots.description && slots.description.props.variant !== 'block' ? inlineDescriptionId : ''
- }`,
- 'aria-describedby': slots.description?.props.variant === 'block' ? blockDescriptionId : undefined,
+ 'aria-labelledby': `${labelId} ${slots.inlineDescription ? inlineDescriptionId : ''}`,
+ 'aria-describedby': slots.blockDescription ? blockDescriptionId : undefined,
  ...(selectionAttribute && {[selectionAttribute]: selected}),
  role: role || itemRole,
  id: itemId,
@@ -235,26 +235,25 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
  >
  <ConditionalBox if={Boolean(slots.trailingVisual)} sx={{display: 'flex', flexGrow: 1}}>
  <ConditionalBox
- if={!!slots.description && slots.description.props.variant !== 'block'}
+ if={!!slots.inlineDescription}
  sx={{display: 'flex', flexGrow: 1, alignItems: 'baseline', minWidth: 0}}
  >
  <Box
  as="span"
  id={labelId}
  sx={{
- flexGrow: slots.description && slots.description.props.variant !== 'block' ? 0 : 1,
- fontWeight: slots.description ? 'bold' : 'normal',
- marginBlockEnd:
- slots.description && slots.description.props.variant !== 'inline' ? '4px' : undefined,
+ flexGrow: slots.inlineDescription ? 0 : 1,
+ fontWeight: slots.inlineDescription || slots.blockDescription ? 'bold' : 'normal',
+ marginBlockEnd: slots.blockDescription ? '4px' : undefined,
  }}
  >
  {childrenWithoutSlots}
  </Box>
- {slots.description?.props.variant !== 'block' ? slots.description : null}
+ {slots.inlineDescription}
  </ConditionalBox>
  {slots.trailingVisual}
  </ConditionalBox>
- {slots.description?.props.variant === 'block' ? slots.description : null}
+ {slots.blockDescription}
  </Box>
  </ItemWrapper>
  </LiBox>

src/hooks/__tests__/useSlots.test.tsx

@@ -2,7 +2,8 @@ import {renderHook} from '@testing-library/react-hooks'
 import React from 'react'
 import {useSlots} from '../useSlots'
 
-function TestComponentA(props: React.PropsWithChildren<unknown>) {
+type TestComponentAProps = React.PropsWithChildren<{variant?: 'a' | 'b'}>
+function TestComponentA(props: TestComponentAProps) {
  return <div {...props} />
 }
 
@@ -114,3 +115,35 @@ test('warns about duplicate slots', () => {
  `)
  expect(warnSpy).toHaveBeenCalledTimes(1)
 })
+
+test('extracts elements based on condition in config object', () => {
+ const children = [
+ <TestComponentA key="a" variant="a" />,
+ <TestComponentA key="b" variant="b" />,
+ <div key="hello">Hello World</div>,
+ ]
+
+ const {result} = renderHook(() =>
+ useSlots(children, {
+ a: [TestComponentA, (props: TestComponentAProps) => props.variant === 'a'],
+ b: [TestComponentA, (props: TestComponentAProps) => props.variant === 'b'],
+ }),
+ )
+ expect(result.current).toMatchInlineSnapshot(`
+ [
+ {
+ "a": <TestComponentA
+ variant="a"
+ />,
+ "b": <TestComponentA
+ variant="b"
+ />,
+ },
+ [
+ <div>
+ Hello World
+ </div>,
+ ],
+ ]
+ `)
+})

src/hooks/useSlots.ts

@@ -1,29 +1,48 @@
 import React from 'react'
 import {warning} from '../utils/warning'
 
+// slot config allows 2 options:
+// 1. Component to match, example: { leadingVisual: LeadingVisual }
+type ComponentMatcher = React.ElementType<Props>
+// 2. Component to match + a test function, example: { blockDescription: [Description, props => props.variant === 'block'] }
+type ComponentAndPropsMatcher = [ComponentMatcher, (props: Props) => boolean]
+
+export type SlotConfig = Record<string, ComponentMatcher | ComponentAndPropsMatcher>
+
+// We don't know what the props are yet, we set them later based on slot config
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
-export type SlotConfig = Record<string, React.ElementType<any>>
+type Props = any
 
-type SlotElements<Type extends SlotConfig> = {
- [Property in keyof Type]: React.ReactElement<React.ComponentPropsWithoutRef<Type[Property]>, Type[Property]>
+type SlotElements<Config extends SlotConfig> = {
+ [Property in keyof Config]: SlotValue<Config, Property>
 }
 
+type SlotValue<Config, Property extends keyof Config> = Config[Property] extends React.ElementType // config option 1
+ ? React.ReactElement<React.ComponentPropsWithoutRef<Config[Property]>, Config[Property]>
+ : Config[Property] extends readonly [
+ infer ElementType extends React.ElementType, // config option 2, infer array[0] as component
+ // eslint-disable-next-line @typescript-eslint/no-unused-vars
+ infer _testFn, // even though we don't use testFn, we need to infer it to support types for slots.*.props
+ ]
+ ? React.ReactElement<React.ComponentPropsWithoutRef<ElementType>, ElementType>
+ : never // useful for narrowing types, third option is not possible
+
 /**
  * Extract components from `children` so we can render them in different places,
  * allowing us to implement components with SSR-compatible slot APIs.
  * Note: We can only extract direct children, not nested ones.
  */
-export function useSlots<T extends SlotConfig>(
+export function useSlots<Config extends SlotConfig>(
  children: React.ReactNode,
- config: T,
-): [Partial<SlotElements<T>>, React.ReactNode[]] {
+ config: Config,
+): [Partial<SlotElements<Config>>, React.ReactNode[]] {
  // Object mapping slot names to their elements
- const slots: Partial<SlotElements<T>> = mapValues(config, () => undefined)
+ const slots: Partial<SlotElements<Config>> = mapValues(config, () => undefined)
 
  // Array of elements that are not slots
  const rest: React.ReactNode[] = []
 
- const keys = Object.keys(config) as Array<keyof T>
+ const keys = Object.keys(config) as Array<keyof Config>
  const values = Object.values(config)
 
  // eslint-disable-next-line github/array-foreach
@@ -34,7 +53,12 @@ export function useSlots<T extends SlotConfig>(
  }
 
  const index = values.findIndex(value => {
- return child.type === value
+ if (Array.isArray(value)) {
+ const [component, testFn] = value
+ return child.type === component && testFn(child.props)
+ } else {
+ return child.type === value
+ }
  })
 
  // If the child is not a slot, add it to the `rest` array
@@ -52,7 +76,8 @@ export function useSlots<T extends SlotConfig>(
  }
 
  // If the child is a slot, add it to the `slots` object
- slots[slotKey] = child as React.ReactElement<React.ComponentPropsWithoutRef<T[keyof T]>, T[keyof T]>
+
+ slots[slotKey] = child as SlotValue<Config, keyof Config>
  })
 
  return [slots, rest]