Use SSR-compatible slot implementation in ActionList/NavList (#3173)

* Migrate ActionList to use new slots impl * Remove ActionList slots * Remove unused import * Create lemon-berries-run.md * Update snapshots * Fix slot description logic * Fix style logic * Fix conditional box logic * Fix aria-labelledby logic * Fix flexbox logic
primer · Apr 20, 2023 · 831c0d3 · 831c0d3
1 parent d5534a2
commit 831c0d3
Show file tree

Hide file tree

Showing 6 changed files with 131 additions and 132 deletions.
diff --git a/.changeset/lemon-berries-run.md b/.changeset/lemon-berries-run.md
@@ -0,0 +1,7 @@
+---
+"@primer/react": patch
+---
+
+`ActionList` and `NavList` are now SSR-compatible. 
+
+Warning: In this new implementation, `ActionList.LeadingVisual`, `ActionList.TrailingVisual,` and `ActionList.Description` must be direct children of `ActionList`. The same applies to `NavList`.
diff --git a/src/ActionList/Description.tsx b/src/ActionList/Description.tsx
@@ -1,8 +1,8 @@
 import React from 'react'
 import Box from '../Box'
-import {SxProp, merge} from '../sx'
 import Truncate from '../Truncate'
-import {Slot, ItemContext} from './shared'
+import {SxProp, merge} from '../sx'
+import {ItemContext} from './shared'
 
 export type ActionListDescriptionProps = {
   /**
@@ -28,29 +28,25 @@ export const Description: React.FC<React.PropsWithChildren<ActionListDescription
     marginLeft: variant === 'block' ? 0 : 2,
   }
 
-  return (
-    <Slot name={variant === 'block' ? 'BlockDescription' : 'InlineDescription'}>
-      {({blockDescriptionId, inlineDescriptionId, disabled}: ItemContext) =>
-        variant === 'block' ? (
-          <Box
-            as="span"
-            sx={merge({...styles, color: disabled ? 'fg.disabled' : 'fg.muted'}, sx as SxProp)}
-            id={blockDescriptionId}
-          >
-            {props.children}
-          </Box>
-        ) : (
-          <Truncate
-            id={inlineDescriptionId}
-            sx={merge({...styles, color: disabled ? 'fg.disabled' : 'fg.muted'}, sx as SxProp)}
-            title={props.children as string}
-            inline={true}
-            maxWidth="100%"
-          >
-            {props.children}
-          </Truncate>
-        )
-      }
-    </Slot>
+  const {blockDescriptionId, inlineDescriptionId, disabled} = React.useContext(ItemContext)
+
+  return variant === 'block' ? (
+    <Box
+      as="span"
+      sx={merge({...styles, color: disabled ? 'fg.disabled' : 'fg.muted'}, sx as SxProp)}
+      id={blockDescriptionId}
+    >
+      {props.children}
+    </Box>
+  ) : (
+    <Truncate
+      id={inlineDescriptionId}
+      sx={merge({...styles, color: disabled ? 'fg.disabled' : 'fg.muted'}, sx as SxProp)}
+      title={props.children as string}
+      inline={true}
+      maxWidth="100%"
+    >
+      {props.children}
+    </Truncate>
   )
 }
diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx
@@ -1,15 +1,18 @@
-import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import React from 'react'
 import styled from 'styled-components'
 import Box, {BoxProps} from '../Box'
+import {useId} from '../hooks/useId'
+import {useSlots} from '../hooks/useSlots'
 import sx, {BetterSystemStyleObject, merge, SxProp} from '../sx'
 import {useTheme} from '../ThemeProvider'
-import {useId} from '../hooks/useId'
+import {defaultSxProp} from '../utils/defaultSxProp'
+import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import {ActionListContainerContext} from './ActionListContainerContext'
+import {Description} from './Description'
 import {ActionListProps, ListContext} from './List'
 import {Selection} from './Selection'
-import {ActionListItemProps, Slots, TEXT_ROW_HEIGHT, getVariantStyles} from './shared'
-import {defaultSxProp} from '../utils/defaultSxProp'
+import {ActionListItemProps, getVariantStyles, ItemContext, TEXT_ROW_HEIGHT} from './shared'
+import {LeadingVisual, TrailingVisual} from './Visuals'
 
 const LiBox = styled.li<SxProp>(sx)
 
@@ -29,6 +32,11 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     },
     forwardedRef,
   ): JSX.Element => {
+    const [slots, childrenWithoutSlots] = useSlots(props.children, {
+      leadingVisual: LeadingVisual,
+      trailingVisual: TrailingVisual,
+      description: Description,
+    })
     const {variant: listVariant, showDividers, selectionVariant: listSelectionVariant} = React.useContext(ListContext)
     const {container, afterSelect, selectionAttribute} = React.useContext(ActionListContainerContext)
 
@@ -165,66 +173,57 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
 
     const ItemWrapper = _PrivateItemWrapper || React.Fragment
 
+    const menuItemProps = {
+      onClick: clickHandler,
+      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,
+      ...(selectionAttribute && {[selectionAttribute]: selected}),
+      role: role || itemRole,
+    }
+
+    const containerProps = _PrivateItemWrapper ? {role: role || itemRole ? 'none' : undefined} : menuItemProps
+
+    const wrapperProps = _PrivateItemWrapper ? menuItemProps : {}
+
     return (
-      <Slots context={{variant, disabled, inlineDescriptionId, blockDescriptionId}}>
-        {slots => {
-          const menuItemProps = {
-            onClick: clickHandler,
-            onKeyPress: keyPressHandler,
-            'aria-disabled': disabled ? true : undefined,
-            tabIndex: disabled ? undefined : 0,
-            'aria-labelledby': `${labelId} ${slots.InlineDescription ? inlineDescriptionId : ''}`,
-            'aria-describedby': slots.BlockDescription ? blockDescriptionId : undefined,
-            ...(selectionAttribute && {[selectionAttribute]: selected}),
-            role: role || itemRole,
-          }
-          const containerProps = _PrivateItemWrapper
-            ? {
-                role: role || itemRole ? 'none' : undefined,
-              }
-            : menuItemProps
-          const wrapperProps = _PrivateItemWrapper ? menuItemProps : {}
-
-          return (
-            <LiBox
-              ref={forwardedRef}
-              sx={merge<BetterSystemStyleObject>(styles, sxProp)}
-              {...containerProps}
-              {...props}
+      <ItemContext.Provider value={{variant, disabled, inlineDescriptionId, blockDescriptionId}}>
+        <LiBox ref={forwardedRef} sx={merge<BetterSystemStyleObject>(styles, sxProp)} {...containerProps} {...props}>
+          <ItemWrapper {...wrapperProps}>
+            <Selection selected={selected} />
+            {slots.leadingVisual}
+            <Box
+              data-component="ActionList.Item--DividerContainer"
+              sx={{display: 'flex', flexDirection: 'column', flexGrow: 1, minWidth: 0}}
             >
-              <ItemWrapper {...wrapperProps}>
-                <Selection selected={selected} />
-                {slots.LeadingVisual}
-                <Box
-                  data-component="ActionList.Item--DividerContainer"
-                  sx={{display: 'flex', flexDirection: 'column', flexGrow: 1, minWidth: 0}}
+              <ConditionalBox if={Boolean(slots.trailingVisual)} sx={{display: 'flex', flexGrow: 1}}>
+                <ConditionalBox
+                  if={!!slots.description && slots.description.props.variant !== 'block'}
+                  sx={{display: 'flex', flexGrow: 1, alignItems: 'baseline', minWidth: 0}}
                 >
-                  <ConditionalBox if={Boolean(slots.TrailingVisual)} sx={{display: 'flex', flexGrow: 1}}>
-                    <ConditionalBox
-                      if={Boolean(slots.InlineDescription)}
-                      sx={{display: 'flex', flexGrow: 1, alignItems: 'baseline', minWidth: 0}}
-                    >
-                      <Box
-                        as="span"
-                        id={labelId}
-                        sx={{
-                          flexGrow: slots.InlineDescription ? 0 : 1,
-                          fontWeight: slots.InlineDescription ? 'bold' : 'normal',
-                        }}
-                      >
-                        {props.children}
-                      </Box>
-                      {slots.InlineDescription}
-                    </ConditionalBox>
-                    {slots.TrailingVisual}
-                  </ConditionalBox>
-                  {slots.BlockDescription}
-                </Box>
-              </ItemWrapper>
-            </LiBox>
-          )
-        }}
-      </Slots>
+                  <Box
+                    as="span"
+                    id={labelId}
+                    sx={{
+                      flexGrow: slots.description && slots.description.props.variant !== 'block' ? 0 : 1,
+                      fontWeight: slots.description && slots.description.props.variant !== 'block' ? 'bold' : 'normal',
+                    }}
+                  >
+                    {childrenWithoutSlots}
+                  </Box>
+                  {slots.description?.props.variant !== 'block' ? slots.description : null}
+                </ConditionalBox>
+                {slots.trailingVisual}
+              </ConditionalBox>
+              {slots.description?.props.variant === 'block' ? slots.description : null}
+            </Box>
+          </ItemWrapper>
+        </LiBox>
+      </ItemContext.Provider>
     )
   },
 ) as PolymorphicForwardRefComponent<'li', ActionListItemProps>

diff --git a/src/ActionList/Visuals.tsx b/src/ActionList/Visuals.tsx
@@ -1,8 +1,8 @@
 import React from 'react'
 import Box from '../Box'
-import {SxProp, merge} from '../sx'
 import {get} from '../constants'
-import {getVariantStyles, Slot, ItemContext, TEXT_ROW_HEIGHT} from './shared'
+import {SxProp, merge} from '../sx'
+import {ItemContext, TEXT_ROW_HEIGHT, getVariantStyles} from './shared'
 
 type VisualProps = SxProp & React.HTMLAttributes<HTMLSpanElement>
 
@@ -30,48 +30,42 @@ export const LeadingVisualContainer: React.FC<React.PropsWithChildren<VisualProp
 
 export type ActionListLeadingVisualProps = VisualProps
 export const LeadingVisual: React.FC<React.PropsWithChildren<VisualProps>> = ({sx = {}, ...props}) => {
+  const {variant, disabled} = React.useContext(ItemContext)
   return (
-    <Slot name="LeadingVisual">
-      {({variant, disabled}: ItemContext) => (
-        <LeadingVisualContainer
-          sx={merge(
-            {
-              color: getVariantStyles(variant, disabled).iconColor,
-              svg: {fontSize: 0},
-            },
-            sx as SxProp,
-          )}
-          {...props}
-        >
-          {props.children}
-        </LeadingVisualContainer>
+    <LeadingVisualContainer
+      sx={merge(
+        {
+          color: getVariantStyles(variant, disabled).iconColor,
+          svg: {fontSize: 0},
+        },
+        sx as SxProp,
       )}
-    </Slot>
+      {...props}
+    >
+      {props.children}
+    </LeadingVisualContainer>
   )
 }
 
 export type ActionListTrailingVisualProps = VisualProps
 export const TrailingVisual: React.FC<React.PropsWithChildren<VisualProps>> = ({sx = {}, ...props}) => {
+  const {variant, disabled} = React.useContext(ItemContext)
   return (
-    <Slot name="TrailingVisual">
-      {({variant, disabled}: ItemContext) => (
-        <Box
-          as="span"
-          sx={merge(
-            {
-              height: '20px', // match height of text row
-              flexShrink: 0,
-              color: getVariantStyles(variant, disabled).annotationColor,
-              marginLeft: 2,
-              fontWeight: 'initial',
-            },
-            sx as SxProp,
-          )}
-          {...props}
-        >
-          {props.children}
-        </Box>
+    <Box
+      as="span"
+      sx={merge(
+        {
+          height: '20px', // match height of text row
+          flexShrink: 0,
+          color: getVariantStyles(variant, disabled).annotationColor,
+          marginLeft: 2,
+          fontWeight: 'initial',
+        },
+        sx as SxProp,
       )}
-    </Slot>
+      {...props}
+    >
+      {props.children}
+    </Box>
   )
 }
diff --git a/src/ActionList/shared.ts b/src/ActionList/shared.ts
@@ -1,7 +1,8 @@
 import React from 'react'
+
+import {AriaRole} from '../utils/types'
 import {SxProp} from '../sx'
 import createSlots from '../utils/create-slots'
-import {AriaRole} from '../utils/types'
 
 export type ActionListItemProps = {
   /**
@@ -56,10 +57,12 @@ type MenuItemProps = {
 }
 
 export type ItemContext = Pick<ActionListItemProps, 'variant' | 'disabled'> & {
-  inlineDescriptionId: string
-  blockDescriptionId: string
+  inlineDescriptionId?: string
+  blockDescriptionId?: string
 }
 
+export const ItemContext = React.createContext<ItemContext>({})
+
 export const getVariantStyles = (
   variant: ActionListItemProps['variant'],
   disabled: ActionListItemProps['disabled'],

diff --git a/src/NavList/__snapshots__/NavList.test.tsx.snap b/src/NavList/__snapshots__/NavList.test.tsx.snap
@@ -1302,12 +1302,6 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
   list-style: none;
 }
 
-.c10 {
-  padding: 0;
-  margin: 0;
-  display: none;
-}
-
 .c6 {
   display: -webkit-box;
   display: -webkit-flex;
@@ -1319,6 +1313,12 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
   flex-grow: 1;
 }
 
+.c10 {
+  padding: 0;
+  margin: 0;
+  display: none;
+}
+
 .c8 {
   height: 20px;
   -webkit-flex-shrink: 0;