From a7834379281d951af94c9fac06fee23d2e35342c Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 17 Sep 2025 11:05:46 -0400 Subject: [PATCH 01/16] pulls changes over from mp/rm-box-and-sx-from-components --- packages/react/src/ActionList/Item.tsx | 541 +++++++++--------- packages/react/src/ActionList/List.tsx | 128 +++-- packages/react/src/ActionList/shared.ts | 180 +++--- packages/react/src/NavList/NavList.docs.json | 42 +- packages/react/src/NavList/NavList.module.css | 8 + packages/react/src/NavList/NavList.tsx | 100 +--- .../styled-react/src/components/NavList.tsx | 73 +++ 7 files changed, 542 insertions(+), 530 deletions(-) create mode 100644 packages/react/src/NavList/NavList.module.css create mode 100644 packages/styled-react/src/components/NavList.tsx diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx index 49944d74fe2..911e8ead7f6 100644 --- a/packages/react/src/ActionList/Item.tsx +++ b/packages/react/src/ActionList/Item.tsx @@ -2,7 +2,6 @@ import React from 'react' import {useId} from '../hooks/useId' import {useSlots} from '../hooks/useSlots' -import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {ActionListContainerContext} from './ActionListContainerContext' import {Description} from './Description' import {GroupContext} from './Group' @@ -17,6 +16,7 @@ import VisuallyHidden from '../_VisuallyHidden' import classes from './ActionList.module.css' import {clsx} from 'clsx' import {BoxWithFallback} from '../internal/components/BoxWithFallback' +import {fixedForwardRef} from '../utils/modern-polymorphic' type ActionListSubItemProps = { children?: React.ReactNode @@ -28,285 +28,298 @@ export const SubItem: React.FC = ({children}) => { SubItem.displayName = 'ActionList.SubItem' -const ButtonItemContainerNoBox = React.forwardRef(({children, style, ...props}, forwardedRef) => { - return ( - - ) -}) as PolymorphicForwardRefComponent - -const DivItemContainerNoBox = React.forwardRef(({children, ...props}, forwardedRef) => { - return ( -
} {...props}> - {children} -
- ) -}) as PolymorphicForwardRefComponent - -export const Item = React.forwardRef( - ( - { - variant = 'default', - size = 'medium', - disabled = false, - inactiveText, - selected = undefined, - active = false, - onSelect: onSelectUser, - sx: sxProp, - id, - role, - loading, - _PrivateItemWrapper, - className, - groupId: _groupId, - renderItem: _renderItem, - handleAddItem: _handleAddItem, - ...props - }, - forwardedRef, - ): JSX.Element => { - const baseSlots = { - leadingVisual: LeadingVisual, - trailingVisual: TrailingVisual, - trailingAction: TrailingAction, - subItem: SubItem, - } - - const [partialSlots, childrenWithoutSlots] = useSlots(props.children, {...baseSlots, description: Description}) - - const slots = {description: undefined, ...partialSlots} - - const {container, afterSelect, selectionAttribute, defaultTrailingVisual} = - React.useContext(ActionListContainerContext) - - // Be sure to avoid rendering the container unless there is a default - const wrappedDefaultTrailingVisual = defaultTrailingVisual ? ( - {defaultTrailingVisual} - ) : null - const trailingVisual = slots.trailingVisual ?? wrappedDefaultTrailingVisual - - const {role: listRole, selectionVariant: listSelectionVariant} = React.useContext(ListContext) - const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext) - const inactive = Boolean(inactiveText) - // TODO change `menuContext` check to ```listRole !== undefined && ['menu', 'listbox'].includes(listRole)``` - // once we have a better way to handle existing usage in dotcom that incorrectly use ActionList.TrailingAction - const menuContext = container === 'ActionMenu' || container === 'SelectPanel' || container === 'FilteredActionList' - // TODO: when we change `menuContext` to check `listRole` instead of `container` - const showInactiveIndicator = inactive && !(listRole !== undefined && ['menu', 'listbox'].includes(listRole)) - - const onSelect = React.useCallback( - ( - event: React.MouseEvent | React.KeyboardEvent, - // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type - afterSelect?: Function, - ) => { - if (typeof onSelectUser === 'function') onSelectUser(event) - if (event.defaultPrevented) return - if (typeof afterSelect === 'function') afterSelect(event) - }, - [onSelectUser], +const ButtonItemContainerNoBox = React.forwardRef>( + ({children, style, ...props}, forwardedRef) => { + return ( + ) + }, +) - const selectionVariant: ActionListProps['selectionVariant'] = groupSelectionVariant - ? groupSelectionVariant - : listSelectionVariant - - /** Infer item role based on the container */ - let inferredItemRole: ActionListItemProps['role'] - if (container === 'ActionMenu') { - if (selectionVariant === 'single') inferredItemRole = 'menuitemradio' - else if (selectionVariant === 'multiple') inferredItemRole = 'menuitemcheckbox' - else inferredItemRole = 'menuitem' - } else if (listRole === 'listbox') { - if (selectionVariant !== undefined && !role) inferredItemRole = 'option' - } - - const itemRole = role || inferredItemRole - - if (slots.trailingAction) { - invariant( - !menuContext, - `ActionList.TrailingAction can not be used within a list with an ARIA role of "menu" or "listbox".`, - ) - } - - /** Infer the proper selection attribute based on the item's role */ - let inferredSelectionAttribute: 'aria-selected' | 'aria-checked' | undefined - if (itemRole === 'menuitemradio' || itemRole === 'menuitemcheckbox') inferredSelectionAttribute = 'aria-checked' - else if (itemRole === 'option') inferredSelectionAttribute = 'aria-selected' - - const itemSelectionAttribute = selectionAttribute || inferredSelectionAttribute - // Ensures ActionList.Item retains list item semantics if a valid ARIA role is applied, or if item is inactive - const listItemSemantics = - role === 'option' || role === 'menuitem' || role === 'menuitemradio' || role === 'menuitemcheckbox' - - const listRoleTypes = ['listbox', 'menu', 'list'] - const listSemantics = (listRole && listRoleTypes.includes(listRole)) || inactive || listItemSemantics - const buttonSemantics = !listSemantics && !_PrivateItemWrapper - - const clickHandler = React.useCallback( - (event: React.MouseEvent) => { - if (disabled || inactive || loading) return - onSelect(event, afterSelect) - }, - [onSelect, disabled, inactive, afterSelect, loading], +const DivItemContainerNoBox = React.forwardRef>( + ({children, ...props}, forwardedRef) => { + return ( +
} {...props}> + {children} +
) + }, +) + +const UnwrappedItem = ( + { + variant = 'default', + size = 'medium', + disabled = false, + inactiveText, + selected = undefined, + active = false, + onSelect: onSelectUser, + sx: sxProp, + id, + role, + loading, + _PrivateItemWrapper, + className, + groupId: _groupId, + renderItem: _renderItem, + handleAddItem: _handleAddItem, + ...props + }: ActionListItemProps, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + forwardedRef: React.Ref, +): JSX.Element => { + const baseSlots = { + leadingVisual: LeadingVisual, + trailingVisual: TrailingVisual, + trailingAction: TrailingAction, + subItem: SubItem, + } + + const [partialSlots, childrenWithoutSlots] = useSlots(props.children, {...baseSlots, description: Description}) + + const slots = {description: undefined, ...partialSlots} + + const {container, afterSelect, selectionAttribute, defaultTrailingVisual} = + React.useContext(ActionListContainerContext) + + // Be sure to avoid rendering the container unless there is a default + const wrappedDefaultTrailingVisual = defaultTrailingVisual ? ( + {defaultTrailingVisual} + ) : null + const trailingVisual = slots.trailingVisual ?? wrappedDefaultTrailingVisual + + const {role: listRole, selectionVariant: listSelectionVariant} = React.useContext(ListContext) + const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext) + const inactive = Boolean(inactiveText) + // TODO change `menuContext` check to ```listRole !== undefined && ['menu', 'listbox'].includes(listRole)``` + // once we have a better way to handle existing usage in dotcom that incorrectly use ActionList.TrailingAction + const menuContext = container === 'ActionMenu' || container === 'SelectPanel' || container === 'FilteredActionList' + // TODO: when we change `menuContext` to check `listRole` instead of `container` + const showInactiveIndicator = inactive && !(listRole !== undefined && ['menu', 'listbox'].includes(listRole)) + + const onSelect = React.useCallback( + ( + event: React.MouseEvent | React.KeyboardEvent, + // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type + afterSelect?: Function, + ) => { + if (typeof onSelectUser === 'function') onSelectUser(event) + if (event.defaultPrevented) return + if (typeof afterSelect === 'function') afterSelect(event) + }, + [onSelectUser], + ) - const keyPressHandler = React.useCallback( - (event: React.KeyboardEvent) => { - if (disabled || inactive || loading) return - if ([' ', 'Enter'].includes(event.key)) { - if (event.key === ' ') { - event.preventDefault() // prevent scrolling on Space - // immediately reset defaultPrevented once its job is done - // so as to not disturb the functions that use that event after this - event.defaultPrevented = false - } - onSelect(event, afterSelect) - } - }, - [onSelect, disabled, loading, inactive, afterSelect], + const selectionVariant: ActionListProps['selectionVariant'] = groupSelectionVariant + ? groupSelectionVariant + : listSelectionVariant + + /** Infer item role based on the container */ + let inferredItemRole: ActionListItemProps['role'] + if (container === 'ActionMenu') { + if (selectionVariant === 'single') inferredItemRole = 'menuitemradio' + else if (selectionVariant === 'multiple') inferredItemRole = 'menuitemcheckbox' + else inferredItemRole = 'menuitem' + } else if (listRole === 'listbox') { + if (selectionVariant !== undefined && !role) inferredItemRole = 'option' + } + + const itemRole = role || inferredItemRole + + if (slots.trailingAction) { + invariant( + !menuContext, + `ActionList.TrailingAction can not be used within a list with an ARIA role of "menu" or "listbox".`, ) + } + + /** Infer the proper selection attribute based on the item's role */ + let inferredSelectionAttribute: 'aria-selected' | 'aria-checked' | undefined + if (itemRole === 'menuitemradio' || itemRole === 'menuitemcheckbox') inferredSelectionAttribute = 'aria-checked' + else if (itemRole === 'option') inferredSelectionAttribute = 'aria-selected' + + const itemSelectionAttribute = selectionAttribute || inferredSelectionAttribute + // Ensures ActionList.Item retains list item semantics if a valid ARIA role is applied, or if item is inactive + const listItemSemantics = + role === 'option' || role === 'menuitem' || role === 'menuitemradio' || role === 'menuitemcheckbox' + + const listRoleTypes = ['listbox', 'menu', 'list'] + const listSemantics = (listRole && listRoleTypes.includes(listRole)) || inactive || listItemSemantics + const buttonSemantics = !listSemantics && !_PrivateItemWrapper + + const clickHandler = React.useCallback( + (event: React.MouseEvent) => { + if (disabled || inactive || loading) return + onSelect(event, afterSelect) + }, + [onSelect, disabled, inactive, afterSelect, loading], + ) - const itemId = useId(id) - const labelId = `${itemId}--label` - const inlineDescriptionId = `${itemId}--inline-description` - const blockDescriptionId = `${itemId}--block-description` - const trailingVisualId = `${itemId}--trailing-visual` - const inactiveWarningId = inactive && !showInactiveIndicator ? `${itemId}--warning-message` : undefined - - const DefaultItemWrapper = listSemantics ? DivItemContainerNoBox : ButtonItemContainerNoBox - - const ItemWrapper = _PrivateItemWrapper || DefaultItemWrapper - - // only apply aria-selected and aria-checked to selectable items - const selectableRoles = ['menuitemradio', 'menuitemcheckbox', 'option'] - const includeSelectionAttribute = itemSelectionAttribute && itemRole && selectableRoles.includes(itemRole) - - let focusable - - if (showInactiveIndicator) { - focusable = true - } - - // Extract the variant prop value from the description slot component - const descriptionVariant = slots.description?.props.variant ?? 'inline' - - const menuItemProps = { - onClick: clickHandler, - onKeyPress: !buttonSemantics ? keyPressHandler : undefined, - 'aria-disabled': disabled ? true : undefined, - 'data-inactive': inactive ? true : undefined, - 'data-loading': loading && !inactive ? true : undefined, - tabIndex: focusable ? undefined : 0, - 'aria-labelledby': `${labelId} ${slots.trailingVisual ? trailingVisualId : ''} ${ - slots.description && descriptionVariant === 'inline' ? inlineDescriptionId : '' - }`, - 'aria-describedby': - [ - slots.description && descriptionVariant === 'block' ? blockDescriptionId : undefined, - inactiveWarningId ?? undefined, - ] - .filter(String) - .join(' ') - .trim() || undefined, - ...(includeSelectionAttribute && {[itemSelectionAttribute]: selected}), - role: itemRole, - id: itemId, - } - - const containerProps = _PrivateItemWrapper - ? {role: itemRole ? 'none' : undefined, ...props} - : // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - (listSemantics && {...menuItemProps, ...props, ref: forwardedRef}) || {} - - const wrapperProps = _PrivateItemWrapper - ? menuItemProps - : !listSemantics && { - ...menuItemProps, - ...props, - ref: forwardedRef, + const keyPressHandler = React.useCallback( + (event: React.KeyboardEvent) => { + if (disabled || inactive || loading) return + if ([' ', 'Enter'].includes(event.key)) { + if (event.key === ' ') { + event.preventDefault() // prevent scrolling on Space + // immediately reset defaultPrevented once its job is done + // so as to not disturb the functions that use that event after this + event.defaultPrevented = false } + onSelect(event, afterSelect) + } + }, + [onSelect, disabled, loading, inactive, afterSelect], + ) - return ( - + - - - - + + + + {slots.leadingVisual} + + + + + {childrenWithoutSlots} + {/* Loading message needs to be in here so it is read with the label */} + {/* If the item is inactive, we do not simultaneously announce that it is loading */} + {loading === true && !inactive && Loading} + + {slots.description} + - {slots.leadingVisual} + {trailingVisual} - - - - {childrenWithoutSlots} - {/* Loading message needs to be in here so it is read with the label */} - {/* If the item is inactive, we do not simultaneously announce that it is loading */} - {loading === true && !inactive && Loading} + + { + // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel), + // render the inactive warning message directly in the item. + !showInactiveIndicator && inactiveText ? ( + + {inactiveText} - {slots.description} - - - {trailingVisual} - - - { - // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel), - // render the inactive warning message directly in the item. - !showInactiveIndicator && inactiveText ? ( - - {inactiveText} - - ) : null - } - - - {!inactive && !loading && !menuContext && Boolean(slots.trailingAction) && slots.trailingAction} - {slots.subItem} - - - ) - }, -) as PolymorphicForwardRefComponent<'li', ActionListItemProps> + ) : null + } + + + {!inactive && !loading && !menuContext && Boolean(slots.trailingAction) && slots.trailingAction} + {slots.subItem} + + + ) +} + +const Item = fixedForwardRef(UnwrappedItem) + +Object.assign(Item, {displayName: 'ActionList.Item'}) -Item.displayName = 'ActionList.Item' +export {Item} diff --git a/packages/react/src/ActionList/List.tsx b/packages/react/src/ActionList/List.tsx index f8448ba0055..97ead828fea 100644 --- a/packages/react/src/ActionList/List.tsx +++ b/packages/react/src/ActionList/List.tsx @@ -1,5 +1,5 @@ import React from 'react' -import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' +import {fixedForwardRef} from '../utils/modern-polymorphic' import {ActionListContainerContext} from './ActionListContainerContext' import {useSlots} from '../hooks/useSlots' import {Heading} from './Heading' @@ -11,68 +11,80 @@ import {clsx} from 'clsx' import classes from './ActionList.module.css' import {BoxWithFallback} from '../internal/components/BoxWithFallback' -export const List = React.forwardRef( - ( - {variant = 'inset', selectionVariant, showDividers = false, role, disableFocusZone = false, className, ...props}, - forwardedRef, - ): JSX.Element => { - const [slots, childrenWithoutSlots] = useSlots(props.children, { - heading: Heading, - }) +const UnwrappedList = ( + props: ActionListProps, + forwardedRef: React.Ref, +): JSX.Element => { + const { + as: Component = 'ul', + variant = 'inset', + selectionVariant, + showDividers = false, + role, + disableFocusZone = false, + className, + ...restProps + } = props + const [slots, childrenWithoutSlots] = useSlots(restProps.children, { + heading: Heading, + }) - const headingId = useId() + const headingId = useId() - /** if list is inside a Menu, it will get a role from the Menu */ - const { - listRole: listRoleFromContainer, - listLabelledBy, - selectionVariant: containerSelectionVariant, // TODO: Remove after DropdownMenu2 deprecation - enableFocusZone: enableFocusZoneFromContainer, - container, - } = React.useContext(ActionListContainerContext) + /** if list is inside a Menu, it will get a role from the Menu */ + const { + listRole: listRoleFromContainer, + listLabelledBy, + selectionVariant: containerSelectionVariant, // TODO: Remove after DropdownMenu2 deprecation + enableFocusZone: enableFocusZoneFromContainer, + container, + } = React.useContext(ActionListContainerContext) - const ariaLabelledBy = slots.heading ? (slots.heading.props.id ?? headingId) : listLabelledBy - const listRole = role || listRoleFromContainer - const listRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) + const ariaLabelledBy = slots.heading ? (slots.heading.props.id ?? headingId) : listLabelledBy + const listRole = role || listRoleFromContainer + const listRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) - let enableFocusZone = false - if (enableFocusZoneFromContainer !== undefined) enableFocusZone = enableFocusZoneFromContainer - else if (listRole && !disableFocusZone) enableFocusZone = ['menu', 'menubar', 'listbox'].includes(listRole) + let enableFocusZone = false + if (enableFocusZoneFromContainer !== undefined) enableFocusZone = enableFocusZoneFromContainer + else if (listRole && !disableFocusZone) enableFocusZone = ['menu', 'menubar', 'listbox'].includes(listRole) - useFocusZone({ - disabled: !enableFocusZone, - containerRef: listRef, - bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | FocusKeys.PageUpDown, - focusOutBehavior: - listRole === 'menu' || container === 'SelectPanel' || container === 'FilteredActionList' ? 'wrap' : undefined, - }) + useFocusZone({ + disabled: !enableFocusZone, + containerRef: listRef, + bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | FocusKeys.PageUpDown, + focusOutBehavior: + listRole === 'menu' || container === 'SelectPanel' || container === 'FilteredActionList' ? 'wrap' : undefined, + }) - return ( - + {slots.heading} + - {slots.heading} - - {childrenWithoutSlots} - - - ) - }, -) as PolymorphicForwardRefComponent<'ul', ActionListProps> + {childrenWithoutSlots} + + + ) +} -List.displayName = 'ActionList' +const List = fixedForwardRef(UnwrappedList) + +Object.assign(List, {displayName: 'ActionList'}) + +export {List} diff --git a/packages/react/src/ActionList/shared.ts b/packages/react/src/ActionList/shared.ts index a4563ee4f7b..6cd74a2f151 100644 --- a/packages/react/src/ActionList/shared.ts +++ b/packages/react/src/ActionList/shared.ts @@ -1,63 +1,70 @@ import React from 'react' import type {SxProp} from '../sx' import type {AriaRole} from '../utils/types' +import type {PolymorphicProps} from '../utils/modern-polymorphic' -export type ActionListItemProps = { - /** - * Primary content for an Item - */ - children?: React.ReactNode - /** - * Callback that will trigger both on click selection and keyboard selection. - * This is not called for disabled or inactive items. - */ - onSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void - /** - * Is the `Item` is currently selected? - */ - selected?: boolean - /** - * Indicate whether the item is active. There should never be more than one active item. - */ - active?: boolean - /** - * Style variations associated with various `Item` types. - * - * - `"default"` - An action `Item`. - * - `"danger"` - A destructive action `Item`. - */ - variant?: 'default' | 'danger' - size?: 'medium' | 'large' - /** - * Items that are disabled can not be clicked, selected, or navigated through. - */ - disabled?: boolean - /** - * The ARIA role describing the function of `Item` component. `option` is a common value. - */ - role?: AriaRole - /** - * id to attach to the root element of the Item - */ - id?: string - /** - * Text describing why the item is inactive. This may be used when an item's usual functionality - * is unavailable due to a system error such as a database outage. - */ - inactiveText?: string - /** - * Whether the item is loading - */ - loading?: boolean - /** - * Private API for use internally only. Used by LinkItem to wrap contents in an anchor - */ - _PrivateItemWrapper?: React.FC> - className?: string - groupId?: string - renderItem?: (item: React.FC>) => React.ReactNode - handleAddItem?: (item: React.FC>) => void -} & SxProp +export type ActionListItemProps = + // need to omit `onSelect` from native HTML
  • attributes to avoid collision + Omit, 'onSelect'> & { + /** + * Primary content for an Item + */ + children?: React.ReactNode + /** + * Callback that will trigger both on click selection and keyboard selection. + * This is not called for disabled or inactive items. + */ + onSelect?: (event: React.MouseEvent | React.KeyboardEvent | React.SyntheticEvent) => void + /** + * Is the `Item` is currently selected? + */ + selected?: boolean + /** + * Indicate whether the item is active. There should never be more than one active item. + */ + active?: boolean + /** + * Style variations associated with various `Item` types. + * + * - `"default"` - An action `Item`. + * - `"danger"` - A destructive action `Item`. + */ + variant?: 'default' | 'danger' + size?: 'medium' | 'large' + /** + * Items that are disabled can not be clicked, selected, or navigated through. + */ + disabled?: boolean + /** + * The ARIA role describing the function of `Item` component. `option` is a common value. + */ + role?: AriaRole + /** + * id to attach to the root element of the Item + */ + id?: string + /** + * Text describing why the item is inactive. This may be used when an item's usual functionality + * is unavailable due to a system error such as a database outage. + */ + inactiveText?: string + /** + * Whether the item is loading + */ + loading?: boolean + /** + * Private API for use internally only. Used by LinkItem to wrap contents in an anchor + */ + _PrivateItemWrapper?: React.FC> + className?: string + groupId?: string + renderItem?: (item: React.FC>) => React.ReactNode + handleAddItem?: (item: React.FC>) => void + /** + * @deprecated `as` prop has no effect on `ActionList.Item`, only `ActionList.LinkItem` + */ + as?: As + } & SxProp type MenuItemProps = { onClick?: (event: React.MouseEvent) => void @@ -70,7 +77,7 @@ type MenuItemProps = { className?: string } -export type ItemContext = Pick & { +export type ItemContext = Pick, 'variant' | 'disabled' | 'size'> & { inlineDescriptionId?: string blockDescriptionId?: string trailingVisualId?: string @@ -80,8 +87,8 @@ export type ItemContext = Pick({}) export const getVariantStyles = ( - variant: ActionListItemProps['variant'], - disabled: ActionListItemProps['disabled'], + variant: ActionListItemProps['variant'], + disabled: ActionListItemProps['disabled'], inactive?: boolean, ) => { if (disabled) { @@ -120,32 +127,39 @@ export const getVariantStyles = ( export const TEXT_ROW_HEIGHT = '20px' // custom value off the scale -export type ActionListProps = React.PropsWithChildren<{ - /** - * `inset` children are offset (vertically and horizontally) from `List`’s edges, `full` children are flush (vertically and horizontally) with `List` edges - */ - variant?: 'inset' | 'horizontal-inset' | 'full' - /** - * Whether multiple Items or a single Item can be selected. - */ - selectionVariant?: 'single' | 'radio' | 'multiple' - /** - * Display a divider above each `Item` in this `List` when it does not follow a `Header` or `Divider`. - */ - showDividers?: boolean - /** - * The ARIA role describing the function of `List` component. `listbox` or `menu` are a common values. - */ - role?: AriaRole - /** - * Disables the focus zone for the list if applicable. Focus zone is enabled by default for `menu` and `listbox` roles, or components such as `ActionMenu` and `SelectPanel`. - */ - disableFocusZone?: boolean - className?: string -}> & +export type ActionListProps = PolymorphicProps< + As, + 'ul', + React.PropsWithChildren<{ + /** + * `inset` children are offset (vertically and horizontally) from `List`’s edges, `full` children are flush (vertically and horizontally) with `List` edges + */ + variant?: 'inset' | 'horizontal-inset' | 'full' + /** + * Whether multiple Items or a single Item can be selected. + */ + selectionVariant?: 'single' | 'radio' | 'multiple' + /** + * Display a divider above each `Item` in this `List` when it does not follow a `Header` or `Divider`. + */ + showDividers?: boolean + /** + * The ARIA role describing the function of `List` component. `listbox` or `menu` are a common values. + */ + role?: AriaRole + /** + * Disables the focus zone for the list if applicable. Focus zone is enabled by default for `menu` and `listbox` roles, or components such as `ActionMenu` and `SelectPanel`. + */ + disableFocusZone?: boolean + className?: string + }> +> & SxProp -type ContextProps = Pick & { +type ContextProps = Pick< + ActionListProps, + 'variant' | 'selectionVariant' | 'showDividers' | 'role' +> & { headingId?: string } diff --git a/packages/react/src/NavList/NavList.docs.json b/packages/react/src/NavList/NavList.docs.json index e3315ab91ab..d1800f44558 100644 --- a/packages/react/src/NavList/NavList.docs.json +++ b/packages/react/src/NavList/NavList.docs.json @@ -26,11 +26,6 @@ "name": "as", "type": "React.ElementType", "defaultValue": "\"nav\"" - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ], "subcomponents": [ @@ -68,11 +63,6 @@ "name": "as", "type": "React.ElementType", "defaultValue": "\"a\"" - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ], "passthrough": { @@ -83,11 +73,6 @@ { "name": "NavList.LeadingVisual", "props": [ - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true - }, { "name": "ref", "type": "React.RefObject" @@ -97,11 +82,6 @@ { "name": "NavList.TrailingVisual", "props": [ - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true - }, { "name": "ref", "type": "React.RefObject" @@ -111,11 +91,6 @@ { "name": "NavList.SubNav", "props": [ - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true - }, { "name": "ref", "type": "React.RefObject" @@ -137,11 +112,6 @@ "type": "string", "description": "The text that gets rendered as the group's heading. Alternatively, you can pass the `NavList.GroupHeading` component as a child of `NavList.Group`.\nIf both `title` and `NavList.GroupHeading` are passed, `NavList.GroupHeading` will be rendered as the heading." }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true - }, { "name": "ref", "type": "React.RefObject" @@ -172,11 +142,6 @@ "description": "", "defaultValue": "" }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true - }, { "name": "ref", "type": "React.RefObject" @@ -186,11 +151,6 @@ { "name": "NavList.Divider", "props": [ - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true - }, { "name": "ref", "type": "React.RefObject" @@ -264,4 +224,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/packages/react/src/NavList/NavList.module.css b/packages/react/src/NavList/NavList.module.css new file mode 100644 index 00000000000..9908361e112 --- /dev/null +++ b/packages/react/src/NavList/NavList.module.css @@ -0,0 +1,8 @@ +.GroupHeading > a { + color: var(--fgColor-default); + text-decoration: inherit; +} + +.GroupHeading > a:hover { + text-decoration: underline; +} diff --git a/packages/react/src/NavList/NavList.tsx b/packages/react/src/NavList/NavList.tsx index 360558fded8..fa75e61834e 100644 --- a/packages/react/src/NavList/NavList.tsx +++ b/packages/react/src/NavList/NavList.tsx @@ -1,6 +1,7 @@ import {ChevronDownIcon, PlusIcon, type Icon} from '@primer/octicons-react' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import React, {isValidElement} from 'react' +import {clsx} from 'clsx' import type { ActionListTrailingActionProps, ActionListDividerProps, @@ -11,27 +12,22 @@ import type { import {ActionList} from '../ActionList' import {SubItem} from '../ActionList/Item' import {ActionListContainerContext} from '../ActionList/ActionListContainerContext' -import Box from '../Box' -import type {SxProp} from '../sx' -import {merge} from '../sx' -import {defaultSxProp} from '../utils/defaultSxProp' import {useId} from '../hooks/useId' import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' import classes from '../ActionList/ActionList.module.css' +import navListClasses from './NavList.module.css' import {flushSync} from 'react-dom' -import {BoxWithFallback} from '../internal/components/BoxWithFallback' // ---------------------------------------------------------------------------- // NavList export type NavListProps = { children: React.ReactNode -} & SxProp & - React.ComponentProps<'nav'> +} & React.ComponentProps<'nav'> const Root = React.forwardRef(({children, ...props}, ref) => { return ( - + ) }) @@ -54,10 +50,10 @@ export type NavListItemProps = { href?: string 'aria-current'?: 'page' | 'step' | 'location' | 'date' | 'time' | 'true' | 'false' | boolean inactiveText?: string -} & SxProp +} const Item = React.forwardRef( - ({'aria-current': ariaCurrent, children, defaultOpen, sx: sxProp = defaultSxProp, ...props}, ref) => { + ({'aria-current': ariaCurrent, children, defaultOpen, ...props}, ref) => { const {depth} = React.useContext(SubNavContext) // Get SubNav from children @@ -79,7 +75,6 @@ const Item = React.forwardRef( subNav={subNav} depth={depth} defaultOpen={defaultOpen} - sx={sxProp} style={{'--subitem-depth': depth} as React.CSSProperties} > {childrenWithoutSubNavOrTrailingAction} @@ -112,7 +107,7 @@ type ItemWithSubNavProps = { depth: number defaultOpen?: boolean style: React.CSSProperties -} & SxProp +} const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: string; isOpen: boolean}>({ buttonId: '', @@ -120,14 +115,7 @@ const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: s isOpen: false, }) -function ItemWithSubNav({ - children, - subNav, - depth: _depth, - defaultOpen, - style, - sx: sxProp = defaultSxProp, -}: ItemWithSubNavProps) { +function ItemWithSubNav({children, subNav, depth: _depth, defaultOpen, style}: ItemWithSubNavProps) { const buttonId = useId() const subNavId = useId() const [isOpen, setIsOpen] = React.useState((defaultOpen || null) ?? false) @@ -147,28 +135,6 @@ function ItemWithSubNav({ } }, [subNav, buttonId]) - if (sxProp !== defaultSxProp) { - return ( - - setIsOpen(open => !open)} - style={style} - sx={sxProp} - > - {children} - {/* What happens if the user provides a TrailingVisual? */} - - - - {React.cloneElement(subNav as React.ReactElement, {ref: subNavRef})} - - - ) - } return ( setIsOpen(open => !open)} + onSelect={() => setIsOpen((open: boolean) => !open)} style={style} > {children} @@ -195,12 +161,12 @@ function ItemWithSubNav({ export type NavListSubNavProps = { children: React.ReactNode -} & SxProp +} const SubNavContext = React.createContext<{depth: number}>({depth: 0}) // NOTE: SubNav must be a direct child of an Item -const SubNav = React.forwardRef(({children, sx: sxProp = defaultSxProp}: NavListSubNavProps, forwardedRef) => { +const SubNav = React.forwardRef(({children}, forwardedRef) => { const {buttonId, subNavId} = React.useContext(ItemWithSubNavContext) const {depth} = React.useContext(SubNavContext) if (!buttonId || !subNavId) { @@ -215,22 +181,6 @@ const SubNav = React.forwardRef(({children, sx: sxProp = defaultSxProp}: NavList return null } - if (sxProp !== defaultSxProp) { - return ( - - - {children} - - - ) - } return (
      @@ -283,18 +233,9 @@ TrailingAction.displayName = 'NavList.TrailingAction' export type NavListGroupProps = React.HTMLAttributes & { children: React.ReactNode title?: string -} & SxProp +} -const defaultSx = {} -const Group: React.FC = ({title, children, sx: sxProp = defaultSx, ...props}) => { - if (sxProp !== defaultSx) { - return ( - - {title ? {title} : null} - {children} - - ) - } +const Group: React.FC = ({title, children, ...props}) => { return ( <> @@ -419,20 +360,11 @@ export type NavListGroupHeadingProps = ActionListGroupHeadingProps * This is an alternative to the `title` prop on `NavList.Group`. * It was primarily added to allow links in group headings. */ -const GroupHeading: React.FC = ({as = 'h3', sx: sxProp = defaultSxProp, ...rest}) => { +const GroupHeading: React.FC = ({as = 'h3', className, ...rest}) => { return ( ( - { - '> a {': { - color: 'var(--fgColor-default)', - textDecoration: 'inherit', - ':hover': {textDecoration: 'underline'}, - }, - }, - sxProp, - )} + className={clsx(navListClasses.GroupHeading, className)} data-component="NavList.GroupHeading" headingWrapElement="li" {...rest} diff --git a/packages/styled-react/src/components/NavList.tsx b/packages/styled-react/src/components/NavList.tsx new file mode 100644 index 00000000000..a1ee428016f --- /dev/null +++ b/packages/styled-react/src/components/NavList.tsx @@ -0,0 +1,73 @@ +import {NavList as PrimerNavList, Box} from '@primer/react' +import type { + NavListProps as PrimerNavListProps, + NavListItemProps as PrimerNavListItemProps, + NavListSubNavProps as PrimerNavListSubNavProps, + NavListDividerProps as PrimerNavListDividerProps, + NavListGroupProps as PrimerNavListGroupProps, + SxProp, +} from '@primer/react' +import {forwardRef, type ComponentProps, type PropsWithChildren} from 'react' + +type NavListProps = PropsWithChildren & SxProp + +const NavListImpl = forwardRef(function NavList(props, ref) { + return +}) + +type NavListItemProps = PropsWithChildren & SxProp + +const NavListItem = forwardRef(function NavListItem(props, ref) { + // @ts-expect-error - PrimerNavList.Item is not recognized as a valid component type + return +}) + +type NavListSubNavProps = PropsWithChildren & SxProp + +const NavListSubNav = forwardRef(function NavListSubNav(props, ref) { + // @ts-expect-error - PrimerNavList.SubNav is not recognized as a valid component type + return +}) + +type NavListDividerProps = PropsWithChildren & SxProp + +const NavListDivider = forwardRef(function NavListDivider(props, ref) { + // @ts-expect-error - PrimerNavList.Divider is not recognized as a valid component type + return +}) + +type NavListGroupProps = PropsWithChildren & SxProp + +const NavListGroup = forwardRef(function NavListGroup(props, ref) { + // @ts-expect-error - PrimerNavList.Group is not recognized as a valid component type + return +}) + +const NavList = Object.assign(NavListImpl, { + // Wrapped components that need sx support added back in + Item: NavListItem, + SubNav: NavListSubNav, + Divider: NavListDivider, + Group: NavListGroup, + + // Re-exporting others directly + // TODO: try to remove typecasts to work around "non-portable types" TS error + LeadingVisual: PrimerNavList.LeadingVisual as React.FC< + React.PropsWithChildren & SxProp> + >, + TrailingVisual: PrimerNavList.TrailingVisual as React.FC< + React.PropsWithChildren & SxProp> + >, + TrailingAction: PrimerNavList.TrailingAction as React.FC< + React.PropsWithChildren & SxProp> + >, + GroupHeading: PrimerNavList.GroupHeading as React.FC< + React.PropsWithChildren & SxProp> + >, + GroupExpand: PrimerNavList.GroupExpand as React.FC< + React.PropsWithChildren & SxProp> + >, +}) + +export {NavList} +export type {NavListProps} From 5f9d7aacb8727d16abbc1fb5a39ec7a6ad7706df Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 17 Sep 2025 11:17:15 -0400 Subject: [PATCH 02/16] adds NavList to react-styled --- packages/styled-react/src/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/styled-react/src/index.tsx b/packages/styled-react/src/index.tsx index 66763c5389a..83ee14e5c25 100644 --- a/packages/styled-react/src/index.tsx +++ b/packages/styled-react/src/index.tsx @@ -28,6 +28,7 @@ import type { SpaceProps, TypographyProps, } from 'styled-system' +import {NavList} from './components/NavList' type StyledProps = SxProp & SpaceProps & @@ -90,7 +91,7 @@ const ToggleSwitch = forwardRef(function T return }) -export {SegmentedControl, StateLabel, SubNav, ToggleSwitch} +export {NavList, SegmentedControl, StateLabel, SubNav, ToggleSwitch} export { ActionList, @@ -113,7 +114,6 @@ export { Label, Link, LinkButton, - NavList, Overlay, PageHeader, PageLayout, From 89645c6b55955fd0be17dfe87782dcc4894c3cbf Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 17 Sep 2025 11:24:32 -0400 Subject: [PATCH 03/16] brings over AutocompleteMenu changes --- .../react/src/Autocomplete/AutocompleteMenu.tsx | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/react/src/Autocomplete/AutocompleteMenu.tsx b/packages/react/src/Autocomplete/AutocompleteMenu.tsx index bc00c27eb64..d2f19b6d6f7 100644 --- a/packages/react/src/Autocomplete/AutocompleteMenu.tsx +++ b/packages/react/src/Autocomplete/AutocompleteMenu.tsx @@ -6,7 +6,7 @@ import type {ScrollIntoViewOptions} from '@primer/behaviors' import type {ActionListItemProps} from '../ActionList' import {ActionList} from '../ActionList' import {useFocusZone} from '../hooks/useFocusZone' -import type {ComponentProps, MandateProps} from '../utils/types' +import type {ComponentProps, MandateProps, AriaRole} from '../utils/types' import Spinner from '../Spinner' import {useId} from '../hooks/useId' import {AutocompleteContext} from './AutocompleteContext' @@ -365,19 +365,25 @@ function AutocompleteMenu(props: AutocompleteMe text, leadingVisual: LeadingVisual, trailingVisual: TrailingVisual, - // @ts-expect-error this is defined in the items above but is - // missing in TS key, + role, ...itemProps } = item return ( - onAction(item)} {...itemProps} id={id} data-id={id}> + onAction(item)} + {...itemProps} + id={id} + data-id={id} + role={role as AriaRole} + > {LeadingVisual && ( {isElement(LeadingVisual) ? LeadingVisual : } )} - {children ?? text} + {(children ?? text) as React.ReactNode} {TrailingVisual && ( {isElement(TrailingVisual) ? TrailingVisual : } From 70cdd560eb741801becb03fcad0a46a6850f7558 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 17 Sep 2025 11:25:49 -0400 Subject: [PATCH 04/16] exports NavListGroupHeadingProps --- packages/react/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 9578768eee8..6050a300264 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -117,6 +117,7 @@ export type { NavListLeadingVisualProps, NavListTrailingVisualProps, NavListDividerProps, + NavListGroupHeadingProps, } from './NavList' export {default as Overlay} from './Overlay' export type {OverlayProps} from './Overlay' From f9225f2b314e679fd417191f6049e727f6237cf6 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 17 Sep 2025 11:29:01 -0400 Subject: [PATCH 05/16] adds modern-polymorphic --- .../react/src/utils/modern-polymorphic.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 packages/react/src/utils/modern-polymorphic.ts diff --git a/packages/react/src/utils/modern-polymorphic.ts b/packages/react/src/utils/modern-polymorphic.ts new file mode 100644 index 00000000000..5f83844ac10 --- /dev/null +++ b/packages/react/src/utils/modern-polymorphic.ts @@ -0,0 +1,35 @@ +// Mostly taken from https://github.com/total-typescript/react-typescript-tutorial/blob/main/src/08-advanced-patterns/72-as-prop-with-forward-ref.solution.tsx + +import {forwardRef} from 'react' +import type {ComponentPropsWithRef, ElementType} from 'react' + +/** + * Distributive Omit utility type that works correctly with union types + */ +type DistributiveOmit = T extends unknown ? Omit : never + +/** + * Fixed version of forwardRef that provides better type inference for polymorphic components + */ +// TODO: figure out how to change this type so we can set displayName +// like this: `ComponentName.displayName = 'DisplayName' instead of using workarounds +type FixedForwardRef = = Record>( + render: (props: P, ref: React.Ref) => React.ReactNode, +) => (props: P & React.RefAttributes) => React.ReactNode + +/** + * Cast forwardRef to the fixed version with better type inference + */ +export const fixedForwardRef = forwardRef as FixedForwardRef + +/** + * Simplified polymorphic props type that handles the common pattern of + * `DistributiveOmit, 'as'>` + */ +export type PolymorphicProps< + TAs extends ElementType, + TDefaultElement extends ElementType = 'div', + Props extends Record = Record, +> = DistributiveOmit & Props, 'as'> & { + as?: TAs +} From 9bb640be1021c19d0cc847d9e958f19d75b5e519 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 17 Sep 2025 11:31:54 -0400 Subject: [PATCH 06/16] adds changeset --- .changeset/cool-clubs-think.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/cool-clubs-think.md diff --git a/.changeset/cool-clubs-think.md b/.changeset/cool-clubs-think.md new file mode 100644 index 00000000000..f025ef30476 --- /dev/null +++ b/.changeset/cool-clubs-think.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +Removes Box usage and sx prop from NavList and ActionList From 3e0774d57497325b29d0cfc42f1c7a23fc5481d7 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 17 Sep 2025 11:43:35 -0400 Subject: [PATCH 07/16] undo dumb ActionList.Item onSelect prop type --- packages/react/src/ActionList/shared.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/ActionList/shared.ts b/packages/react/src/ActionList/shared.ts index 6cd74a2f151..b19e2e08208 100644 --- a/packages/react/src/ActionList/shared.ts +++ b/packages/react/src/ActionList/shared.ts @@ -14,7 +14,7 @@ export type ActionListItemProps = * Callback that will trigger both on click selection and keyboard selection. * This is not called for disabled or inactive items. */ - onSelect?: (event: React.MouseEvent | React.KeyboardEvent | React.SyntheticEvent) => void + onSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void /** * Is the `Item` is currently selected? */ From 1fe97f3bb90809506e98b2b6bb267be44040bf34 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 17 Sep 2025 18:03:21 -0400 Subject: [PATCH 08/16] adds styled-react tests for NavList --- .../__tests__/primer-react.browser.test.tsx | 57 +++++++++++++++++-- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/packages/styled-react/src/__tests__/primer-react.browser.test.tsx b/packages/styled-react/src/__tests__/primer-react.browser.test.tsx index 2b061a08546..41272166bb0 100644 --- a/packages/styled-react/src/__tests__/primer-react.browser.test.tsx +++ b/packages/styled-react/src/__tests__/primer-react.browser.test.tsx @@ -209,7 +209,8 @@ describe('@primer/react', () => { expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') }) - test.todo('NavList.Item supports `sx` prop', () => { + // TODO: figure out why `sx` isn't working here + test('NavList.Item supports `sx` prop', () => { render( @@ -217,12 +218,36 @@ describe('@primer/react', () => { , ) - expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') + + const itemAnchorEl = screen.getByTestId('component') + const itemLiEl = itemAnchorEl.closest('li') + expect(itemLiEl).not.toBeNull() + expect(window.getComputedStyle(itemLiEl!).backgroundColor).toBe('rgb(255, 0, 0)') }) test('NavList.Group supports `sx` prop', () => { - const {container} = render(test) - expect(window.getComputedStyle(container.firstElementChild!).backgroundColor).toBe('rgb(255, 0, 0)') + render( + + + item + + , + ) + expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') + }) + + test('NavList.GroupHeading supports `sx` prop', () => { + render( + + + + test + + item + + , + ) + expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') }) test('NavList.LeadingVisual supports `sx` prop', () => { @@ -230,6 +255,26 @@ describe('@primer/react', () => { expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') }) + // TODO: figure out why `sx` isn't working here + test('NavList.SubNav supports `sx` prop', () => { + render( + + + Parent item + + subitem + + + , + ) + + // Select the NavList.SubNav element by finding the "subitem" text and traversing up to the nearest
        + const itemAnchorEl = screen.getByTestId('component') + const subNavElement = itemAnchorEl.closest('ul') + expect(subNavElement).not.toBeNull() + expect(window.getComputedStyle(subNavElement!).backgroundColor).toBe('rgb(255, 0, 0)') + }) + test('Overlay supports `sx` prop', () => { const ref = createRef() render( @@ -359,12 +404,12 @@ describe('@primer/react', () => { expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') }) - test('SubNav supports `sx` prop', () => { + test.skip('SubNav supports `sx` prop', () => { render() expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') }) - test('SubNav.Link supports `sx` prop', () => { + test.skip('SubNav.Link supports `sx` prop', () => { render() expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') }) From 9d2c8298ff66d7951a32986ac22596a209db5d84 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 17 Sep 2025 18:45:30 -0400 Subject: [PATCH 09/16] pull SegmentedControl changes from mp/rm-box-and-sx-from-components --- packages/react/src/SegmentedControl/SegmentedControl.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/SegmentedControl/SegmentedControl.tsx b/packages/react/src/SegmentedControl/SegmentedControl.tsx index f0e3ce38fd0..a706f41a0ef 100644 --- a/packages/react/src/SegmentedControl/SegmentedControl.tsx +++ b/packages/react/src/SegmentedControl/SegmentedControl.tsx @@ -132,7 +132,7 @@ const Root: React.FC> = ({ { + onSelect={event => { isUncontrolled && setSelectedIndexInternalState(index) onChange && onChange(index) child.props.onClick && child.props.onClick(event as React.MouseEvent) From 3ab2bc94713f542dd2e42c916e950bbf3d216435 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 18 Sep 2025 08:18:15 -0400 Subject: [PATCH 10/16] updates styled-react NavList ports based on latest github-ui usage --- .../__snapshots__/exports.test.ts.snap | 1 + .../__tests__/primer-react.browser.test.tsx | 21 ------------ .../styled-react/src/components/NavList.tsx | 32 +++++-------------- packages/styled-react/src/sx.ts | 8 +++++ 4 files changed, 17 insertions(+), 45 deletions(-) create mode 100644 packages/styled-react/src/sx.ts diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index d34f9dce59c..02f93e26e52 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -94,6 +94,7 @@ exports[`@primer/react > should not update exports without a semver change 1`] = "merge", "NavList", "type NavListDividerProps", + "type NavListGroupHeadingProps", "type NavListGroupProps", "type NavListItemProps", "type NavListLeadingVisualProps", diff --git a/packages/styled-react/src/__tests__/primer-react.browser.test.tsx b/packages/styled-react/src/__tests__/primer-react.browser.test.tsx index 41272166bb0..54e133eb0de 100644 --- a/packages/styled-react/src/__tests__/primer-react.browser.test.tsx +++ b/packages/styled-react/src/__tests__/primer-react.browser.test.tsx @@ -209,7 +209,6 @@ describe('@primer/react', () => { expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') }) - // TODO: figure out why `sx` isn't working here test('NavList.Item supports `sx` prop', () => { render( @@ -255,26 +254,6 @@ describe('@primer/react', () => { expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)') }) - // TODO: figure out why `sx` isn't working here - test('NavList.SubNav supports `sx` prop', () => { - render( - - - Parent item - - subitem - - - , - ) - - // Select the NavList.SubNav element by finding the "subitem" text and traversing up to the nearest
          - const itemAnchorEl = screen.getByTestId('component') - const subNavElement = itemAnchorEl.closest('ul') - expect(subNavElement).not.toBeNull() - expect(window.getComputedStyle(subNavElement!).backgroundColor).toBe('rgb(255, 0, 0)') - }) - test('Overlay supports `sx` prop', () => { const ref = createRef() render( diff --git a/packages/styled-react/src/components/NavList.tsx b/packages/styled-react/src/components/NavList.tsx index a1ee428016f..6c29c24b05a 100644 --- a/packages/styled-react/src/components/NavList.tsx +++ b/packages/styled-react/src/components/NavList.tsx @@ -2,12 +2,10 @@ import {NavList as PrimerNavList, Box} from '@primer/react' import type { NavListProps as PrimerNavListProps, NavListItemProps as PrimerNavListItemProps, - NavListSubNavProps as PrimerNavListSubNavProps, - NavListDividerProps as PrimerNavListDividerProps, NavListGroupProps as PrimerNavListGroupProps, - SxProp, } from '@primer/react' import {forwardRef, type ComponentProps, type PropsWithChildren} from 'react' +import {type SxProp} from '../sx' type NavListProps = PropsWithChildren & SxProp @@ -22,20 +20,6 @@ const NavListItem = forwardRef(function Nav return }) -type NavListSubNavProps = PropsWithChildren & SxProp - -const NavListSubNav = forwardRef(function NavListSubNav(props, ref) { - // @ts-expect-error - PrimerNavList.SubNav is not recognized as a valid component type - return -}) - -type NavListDividerProps = PropsWithChildren & SxProp - -const NavListDivider = forwardRef(function NavListDivider(props, ref) { - // @ts-expect-error - PrimerNavList.Divider is not recognized as a valid component type - return -}) - type NavListGroupProps = PropsWithChildren & SxProp const NavListGroup = forwardRef(function NavListGroup(props, ref) { @@ -46,26 +30,26 @@ const NavListGroup = forwardRef(function NavLi const NavList = Object.assign(NavListImpl, { // Wrapped components that need sx support added back in Item: NavListItem, - SubNav: NavListSubNav, - Divider: NavListDivider, Group: NavListGroup, // Re-exporting others directly // TODO: try to remove typecasts to work around "non-portable types" TS error + SubNav: PrimerNavList.SubNav as React.FC>>, + Divider: PrimerNavList.Divider as React.FC>>, LeadingVisual: PrimerNavList.LeadingVisual as React.FC< - React.PropsWithChildren & SxProp> + React.PropsWithChildren> >, TrailingVisual: PrimerNavList.TrailingVisual as React.FC< - React.PropsWithChildren & SxProp> + React.PropsWithChildren> >, TrailingAction: PrimerNavList.TrailingAction as React.FC< - React.PropsWithChildren & SxProp> + React.PropsWithChildren> >, GroupHeading: PrimerNavList.GroupHeading as React.FC< - React.PropsWithChildren & SxProp> + React.PropsWithChildren> >, GroupExpand: PrimerNavList.GroupExpand as React.FC< - React.PropsWithChildren & SxProp> + React.PropsWithChildren> >, }) diff --git a/packages/styled-react/src/sx.ts b/packages/styled-react/src/sx.ts new file mode 100644 index 00000000000..e3ff0277f10 --- /dev/null +++ b/packages/styled-react/src/sx.ts @@ -0,0 +1,8 @@ +import css from '@styled-system/css' +import type {SxProp} from '@primer/react' + +export const sx = (props: SxProp) => { + return css(props.sx) +} + +export type {SxProp} From 04d49fd03f01ed41f1d2f85bb52c846ce9c4f593 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 18 Sep 2025 11:07:52 -0400 Subject: [PATCH 11/16] Update packages/react/src/ActionList/Item.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- packages/react/src/ActionList/Item.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx index 911e8ead7f6..f44cb983ca3 100644 --- a/packages/react/src/ActionList/Item.tsx +++ b/packages/react/src/ActionList/Item.tsx @@ -262,7 +262,7 @@ const UnwrappedItem = ( {...wrapperProps} className={classes.ActionListContent} data-size={size} - // @ts-ignore this can be a `button` or `li`, so the type of ref will be different + // @ts-expect-error: ItemWrapper is polymorphic and the ref type depends on the rendered element ('button' or 'li') ref={forwardedRef} > From d2ed78e929ba9a576010689ecb6972aec55c2a6d Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 18 Sep 2025 12:47:23 -0400 Subject: [PATCH 12/16] Update packages/react/src/NavList/NavList.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- packages/react/src/NavList/NavList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/NavList/NavList.tsx b/packages/react/src/NavList/NavList.tsx index fa75e61834e..282bdae1e75 100644 --- a/packages/react/src/NavList/NavList.tsx +++ b/packages/react/src/NavList/NavList.tsx @@ -142,7 +142,7 @@ function ItemWithSubNav({children, subNav, depth: _depth, defaultOpen, style}: I aria-expanded={isOpen} aria-controls={subNavId} active={!isOpen && containsCurrentItem} - onSelect={() => setIsOpen((open: boolean) => !open)} + onSelect={() => setIsOpen(open => !open)} style={style} > {children} From 10b57524cd86738a8c94aa0dc532d6f1bf264a0b Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 18 Sep 2025 12:48:24 -0400 Subject: [PATCH 13/16] adjust Copilot's bad TS directive change --- packages/react/src/ActionList/Item.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx index f44cb983ca3..65408b623b4 100644 --- a/packages/react/src/ActionList/Item.tsx +++ b/packages/react/src/ActionList/Item.tsx @@ -262,7 +262,7 @@ const UnwrappedItem = ( {...wrapperProps} className={classes.ActionListContent} data-size={size} - // @ts-expect-error: ItemWrapper is polymorphic and the ref type depends on the rendered element ('button' or 'li') + // @ts-ignore: ItemWrapper is polymorphic and the ref type depends on the rendered element ('button' or 'li') ref={forwardedRef} > From 0308db2ddd1be7d3a7fab82b2958043862a73d22 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 24 Sep 2025 13:14:31 -0400 Subject: [PATCH 14/16] adds primer-styled NavList export back to exports --- packages/styled-react/src/components/NavList.tsx | 3 +-- packages/styled-react/src/index.tsx | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/styled-react/src/components/NavList.tsx b/packages/styled-react/src/components/NavList.tsx index 6c29c24b05a..2519af4870e 100644 --- a/packages/styled-react/src/components/NavList.tsx +++ b/packages/styled-react/src/components/NavList.tsx @@ -53,5 +53,4 @@ const NavList = Object.assign(NavListImpl, { >, }) -export {NavList} -export type {NavListProps} +export {NavList, type NavListProps} diff --git a/packages/styled-react/src/index.tsx b/packages/styled-react/src/index.tsx index 8be52072298..b39c87aa5df 100644 --- a/packages/styled-react/src/index.tsx +++ b/packages/styled-react/src/index.tsx @@ -14,7 +14,6 @@ export {Heading} from '@primer/react' export {IconButton} from '@primer/react' export {Label} from '@primer/react' export {Link} from '@primer/react' -export {NavList} from '@primer/react' export {Overlay} from '@primer/react' export {PageLayout} from '@primer/react' export {ProgressBar} from '@primer/react' @@ -40,6 +39,7 @@ export {CounterLabel, type CounterLabelProps} from './components/CounterLabel' export {Flash} from './components/Flash' export {Header, type HeaderProps} from './components/Header' export {LinkButton, type LinkButtonProps} from './components/LinkButton' +export {NavList, type NavListProps} from './components/NavList' export { PageHeader, type PageHeaderProps, From b2b9e9aeda88ff16d887efb779bbd54e21c79579 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 24 Sep 2025 15:26:25 -0400 Subject: [PATCH 15/16] refactor modern-polymorphic to be give more accurate prop types. e.g.: the 'event' type on event handler props like onClick --- packages/react/src/utils/modern-polymorphic.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/utils/modern-polymorphic.ts b/packages/react/src/utils/modern-polymorphic.ts index 5f83844ac10..d29e9e14f24 100644 --- a/packages/react/src/utils/modern-polymorphic.ts +++ b/packages/react/src/utils/modern-polymorphic.ts @@ -13,7 +13,7 @@ type DistributiveOmit = T extends unknown ? Omi */ // TODO: figure out how to change this type so we can set displayName // like this: `ComponentName.displayName = 'DisplayName' instead of using workarounds -type FixedForwardRef = = Record>( +type FixedForwardRef = ( render: (props: P, ref: React.Ref) => React.ReactNode, ) => (props: P & React.RefAttributes) => React.ReactNode @@ -29,7 +29,7 @@ export const fixedForwardRef = forwardRef as FixedForwardRef export type PolymorphicProps< TAs extends ElementType, TDefaultElement extends ElementType = 'div', - Props extends Record = Record, + Props = object, > = DistributiveOmit & Props, 'as'> & { as?: TAs } From 38b3e93d02aed0ba248ad5dc322834349f76e607 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 24 Sep 2025 19:23:21 -0400 Subject: [PATCH 16/16] more explicitly excludes native onSelect type from ActionList.Item type --- packages/react/src/ActionList/shared.ts | 128 ++++++++++++------------ 1 file changed, 66 insertions(+), 62 deletions(-) diff --git a/packages/react/src/ActionList/shared.ts b/packages/react/src/ActionList/shared.ts index b19e2e08208..fe3d937c5f8 100644 --- a/packages/react/src/ActionList/shared.ts +++ b/packages/react/src/ActionList/shared.ts @@ -3,68 +3,72 @@ import type {SxProp} from '../sx' import type {AriaRole} from '../utils/types' import type {PolymorphicProps} from '../utils/modern-polymorphic' -export type ActionListItemProps = - // need to omit `onSelect` from native HTML
        • attributes to avoid collision - Omit, 'onSelect'> & { - /** - * Primary content for an Item - */ - children?: React.ReactNode - /** - * Callback that will trigger both on click selection and keyboard selection. - * This is not called for disabled or inactive items. - */ - onSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void - /** - * Is the `Item` is currently selected? - */ - selected?: boolean - /** - * Indicate whether the item is active. There should never be more than one active item. - */ - active?: boolean - /** - * Style variations associated with various `Item` types. - * - * - `"default"` - An action `Item`. - * - `"danger"` - A destructive action `Item`. - */ - variant?: 'default' | 'danger' - size?: 'medium' | 'large' - /** - * Items that are disabled can not be clicked, selected, or navigated through. - */ - disabled?: boolean - /** - * The ARIA role describing the function of `Item` component. `option` is a common value. - */ - role?: AriaRole - /** - * id to attach to the root element of the Item - */ - id?: string - /** - * Text describing why the item is inactive. This may be used when an item's usual functionality - * is unavailable due to a system error such as a database outage. - */ - inactiveText?: string - /** - * Whether the item is loading - */ - loading?: boolean - /** - * Private API for use internally only. Used by LinkItem to wrap contents in an anchor - */ - _PrivateItemWrapper?: React.FC> - className?: string - groupId?: string - renderItem?: (item: React.FC>) => React.ReactNode - handleAddItem?: (item: React.FC>) => void - /** - * @deprecated `as` prop has no effect on `ActionList.Item`, only `ActionList.LinkItem` - */ - as?: As - } & SxProp +// need to explicitly omit `onSelect` from native HTML
        • attributes to avoid collision +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type ExcludeSelectEventHandler = T extends any ? Omit : never + +export type ActionListItemProps = ExcludeSelectEventHandler< + PolymorphicProps +> & { + /** + * Primary content for an Item + */ + children?: React.ReactNode + /** + * Callback that will trigger both on click selection and keyboard selection. + * This is not called for disabled or inactive items. + */ + onSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void + /** + * Is the `Item` is currently selected? + */ + selected?: boolean + /** + * Indicate whether the item is active. There should never be more than one active item. + */ + active?: boolean + /** + * Style variations associated with various `Item` types. + * + * - `"default"` - An action `Item`. + * - `"danger"` - A destructive action `Item`. + */ + variant?: 'default' | 'danger' + size?: 'medium' | 'large' + /** + * Items that are disabled can not be clicked, selected, or navigated through. + */ + disabled?: boolean + /** + * The ARIA role describing the function of `Item` component. `option` is a common value. + */ + role?: AriaRole + /** + * id to attach to the root element of the Item + */ + id?: string + /** + * Text describing why the item is inactive. This may be used when an item's usual functionality + * is unavailable due to a system error such as a database outage. + */ + inactiveText?: string + /** + * Whether the item is loading + */ + loading?: boolean + /** + * Private API for use internally only. Used by LinkItem to wrap contents in an anchor + */ + _PrivateItemWrapper?: React.FC> + className?: string + groupId?: string + renderItem?: (item: React.FC>) => React.ReactNode + handleAddItem?: (item: React.FC>) => void + /** + * @deprecated `as` prop has no effect on `ActionList.Item`, only `ActionList.LinkItem` + */ + as?: As +} & SxProp type MenuItemProps = { onClick?: (event: React.MouseEvent) => void