From 263c25cf47f78402a8842818a0b8b0bd21df5edc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:09:02 +0000 Subject: [PATCH 01/39] Document prop merging conventions Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com> --- contributor-docs/style.md | 53 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/contributor-docs/style.md b/contributor-docs/style.md index c9dabb0db1a..8d71f6905d4 100644 --- a/contributor-docs/style.md +++ b/contributor-docs/style.md @@ -68,6 +68,7 @@ row before the line. - [Utilities](#utilities) - [Props](#props) - [Prefer applying component rest parameters to the root element rendered by a component](#prefer-applying-component-rest-parameters-to-the-root-element-rendered-by-a-component) + - [Merge shared props intentionally](#merge-shared-props-intentionally) - [Prefer authoring callback prop types with arguments that can be extended](#prefer-authoring-callback-prop-types-with-arguments-that-can-be-extended) - [Hooks](#hooks) - [Prefer authoring hooks that accept a `ref` instead of returning a `ref` to apply](#prefer-authoring-hooks-that-accept-a-ref-instead-of-returning-a-ref-to-apply) @@ -299,6 +300,58 @@ function Example({children, ...rest}: Props) { +#### Merge shared props intentionally + +When a component and a consumer can both provide a prop, define how those values +are merged instead of allowing the order of prop spreads to decide accidentally. +Use the following conventions: + +- Event handlers run the component's handler first, then the consumer's handler + if the event has not been prevented. +- Class names are merged with `clsx`. +- Style objects apply the component's styles first and the consumer's styles + second so that the consumer can override them. +- Other attributes use the consumer's value. If an attribute must be controlled + by the component, do not expose it as a prop, or use types that only offer it + in the scenarios where the consumer can control it. + +For example: + +```tsx +type Props = React.ComponentPropsWithoutRef<'button'> + +function Example({className, onClick, style, ...rest}: Props) { + const handleClick = (event: React.MouseEvent) => { + performInternalAction(event) + + if (!event.defaultPrevented) { + onClick?.(event) + } + } + + return ( + ) From 1e0c5a4997acb7daee3987eca42fd6cc71c5654a Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 11 Aug 2026 16:55:25 -0500 Subject: [PATCH 30/39] Preserve ActionList prop behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9541ca7f-0281-4223-a839-39d75393d922 --- .changeset/calm-action-lists-merge.md | 5 ----- packages/react/src/ActionList/Group.tsx | 3 ++- .../src/ActionList/GroupHeadingTrailingAction.tsx | 4 ++-- packages/react/src/ActionList/Heading.tsx | 14 +++++++++----- packages/react/src/ActionList/Item.test.tsx | 10 ++++++++++ packages/react/src/ActionList/Item.tsx | 4 ++-- 6 files changed, 25 insertions(+), 15 deletions(-) delete mode 100644 .changeset/calm-action-lists-merge.md diff --git a/.changeset/calm-action-lists-merge.md b/.changeset/calm-action-lists-merge.md deleted file mode 100644 index ac258866b05..00000000000 --- a/.changeset/calm-action-lists-merge.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@primer/react': patch ---- - -ActionList: Preserve component behavior and styling when merging consumer props diff --git a/packages/react/src/ActionList/Group.tsx b/packages/react/src/ActionList/Group.tsx index e508e7893f1..7f18e8a9d76 100644 --- a/packages/react/src/ActionList/Group.tsx +++ b/packages/react/src/ActionList/Group.tsx @@ -70,6 +70,7 @@ export const Group: FCWithSlotMarker { @@ -96,7 +97,7 @@ export const Group: FCWithSlotMarker ( ), ) as PolymorphicForwardRefComponent<'button' | 'a', ActionListGroupHeadingTrailingActionProps> & { diff --git a/packages/react/src/ActionList/Heading.tsx b/packages/react/src/ActionList/Heading.tsx index 3c5920a00bf..3bb1ec3ec26 100644 --- a/packages/react/src/ActionList/Heading.tsx +++ b/packages/react/src/ActionList/Heading.tsx @@ -20,7 +20,7 @@ export type ActionListHeadingProps = { style?: React.CSSProperties } -export const Heading = forwardRef(({as, size, children, visuallyHidden = false, ...props}, forwardedRef) => { +export const Heading = forwardRef(({as, size, children, visuallyHidden = false, className, ...props}, forwardedRef) => { const innerRef = React.useRef(null) const mergedRef = useMergedRefs(forwardedRef, innerRef) @@ -35,6 +35,7 @@ export const Heading = forwardRef(({as, size, children, visuallyHidden = false, return ( {children} diff --git a/packages/react/src/ActionList/Item.test.tsx b/packages/react/src/ActionList/Item.test.tsx index 21d3556e2c9..5a2835e7b8d 100644 --- a/packages/react/src/ActionList/Item.test.tsx +++ b/packages/react/src/ActionList/Item.test.tsx @@ -214,6 +214,16 @@ describe('ActionList.Item', () => { const listItems = container.querySelectorAll('li') expect(listItems.length).toBe(2) }) + it('allows consumers to override the button type', async () => { + const props = {type: 'submit'} as const + const {container} = HTMLRender( + + Item 1 + , + ) + + expect(container.querySelector('button')).toHaveAttribute('type', 'submit') + }) it('should render ActionList.Item as li when item has proper aria role', async () => { const {container} = HTMLRender( diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx index 88dce805b95..441f120363e 100644 --- a/packages/react/src/ActionList/Item.tsx +++ b/packages/react/src/ActionList/Item.tsx @@ -51,10 +51,10 @@ export const SubItem: React.FC = ({children}) => { SubItem.displayName = 'ActionList.SubItem' -const ButtonItemContainer = React.forwardRef>( +const ButtonItemContainer = React.forwardRef>( ({children, ...props}, forwardedRef) => { return ( - ) From 28b19cabd8d2dfa4e9ec87718025b212ef5a0c97 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 11 Aug 2026 16:42:42 -0500 Subject: [PATCH 31/39] Migrate Button roots to mergeProps Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9541ca7f-0281-4223-a839-39d75393d922 --- .changeset/bright-buttons-merge.md | 5 +++ packages/react/src/Button/Button.tsx | 3 +- packages/react/src/Button/IconButton.tsx | 41 ++++++++++++++---------- packages/react/src/Button/LinkButton.tsx | 3 +- 4 files changed, 33 insertions(+), 19 deletions(-) create mode 100644 .changeset/bright-buttons-merge.md diff --git a/.changeset/bright-buttons-merge.md b/.changeset/bright-buttons-merge.md new file mode 100644 index 00000000000..cfef16dd2f3 --- /dev/null +++ b/.changeset/bright-buttons-merge.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Button: Preserve component behavior and styling when merging consumer props diff --git a/packages/react/src/Button/Button.tsx b/packages/react/src/Button/Button.tsx index 30f64bd9d67..367ccbe00a1 100644 --- a/packages/react/src/Button/Button.tsx +++ b/packages/react/src/Button/Button.tsx @@ -2,10 +2,11 @@ import {forwardRef, type JSX} from 'react' import type {ButtonProps} from './types' import {ButtonBase} from './ButtonBase' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' +import {mergeProps} from '../utils/mergeProps' const ButtonComponent = forwardRef(({children, ...props}, forwardedRef): JSX.Element => { return ( - + {children} ) diff --git a/packages/react/src/Button/IconButton.tsx b/packages/react/src/Button/IconButton.tsx index b48d54475a5..753bbaa0788 100644 --- a/packages/react/src/Button/IconButton.tsx +++ b/packages/react/src/Button/IconButton.tsx @@ -6,7 +6,7 @@ import {Tooltip} from '../TooltipV2/Tooltip' import {TooltipContext} from '../TooltipV2/TooltipContext' import {TooltipContext as TooltipContextV1} from '../Tooltip/TooltipContext' import classes from './ButtonBase.module.css' -import {clsx} from 'clsx' +import {mergeProps} from '../utils/mergeProps' const IconButton = forwardRef( ( @@ -20,7 +20,6 @@ const IconButton = forwardRef( unsafeDisableTooltip = false, keyshortcuts, keybindingHint, - className, ...props }, forwardedRef, @@ -43,13 +42,17 @@ const IconButton = forwardRef( if (withoutTooltip) { return ( @@ -66,14 +69,18 @@ const IconButton = forwardRef( _privateDisableTooltip={hasActivePopup} > ) diff --git a/packages/react/src/Button/LinkButton.tsx b/packages/react/src/Button/LinkButton.tsx index a617581fa8b..7244e1ea4c7 100644 --- a/packages/react/src/Button/LinkButton.tsx +++ b/packages/react/src/Button/LinkButton.tsx @@ -2,12 +2,13 @@ import {forwardRef, type JSX} from 'react' import type {LinkButtonProps as BaseLinkButtonProps, ButtonProps} from './types' import {ButtonBase} from './ButtonBase' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' +import {mergeProps} from '../utils/mergeProps' export type LinkButtonProps = BaseLinkButtonProps & ButtonProps const LinkButton = forwardRef(({children, as: Component = 'a', ...props}, forwardedRef): JSX.Element => { return ( - + {children} ) From 2a7e15c9400d933f7b7392a0e96d4ed1ac09981b Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 11 Aug 2026 16:56:58 -0500 Subject: [PATCH 32/39] Preserve Button prop behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9541ca7f-0281-4223-a839-39d75393d922 --- .changeset/bright-buttons-merge.md | 5 ----- packages/react/src/Button/Button.tsx | 2 +- packages/react/src/Button/IconButton.tsx | 10 ++++++---- packages/react/src/Button/LinkButton.tsx | 2 +- 4 files changed, 8 insertions(+), 11 deletions(-) delete mode 100644 .changeset/bright-buttons-merge.md diff --git a/.changeset/bright-buttons-merge.md b/.changeset/bright-buttons-merge.md deleted file mode 100644 index cfef16dd2f3..00000000000 --- a/.changeset/bright-buttons-merge.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@primer/react': patch ---- - -Button: Preserve component behavior and styling when merging consumer props diff --git a/packages/react/src/Button/Button.tsx b/packages/react/src/Button/Button.tsx index 367ccbe00a1..080d67fd329 100644 --- a/packages/react/src/Button/Button.tsx +++ b/packages/react/src/Button/Button.tsx @@ -6,7 +6,7 @@ import {mergeProps} from '../utils/mergeProps' const ButtonComponent = forwardRef(({children, ...props}, forwardedRef): JSX.Element => { return ( - + {children} ) diff --git a/packages/react/src/Button/IconButton.tsx b/packages/react/src/Button/IconButton.tsx index 753bbaa0788..617e751044f 100644 --- a/packages/react/src/Button/IconButton.tsx +++ b/packages/react/src/Button/IconButton.tsx @@ -7,6 +7,7 @@ import {TooltipContext} from '../TooltipV2/TooltipContext' import {TooltipContext as TooltipContextV1} from '../Tooltip/TooltipContext' import classes from './ButtonBase.module.css' import {mergeProps} from '../utils/mergeProps' +import {clsx} from 'clsx' const IconButton = forwardRef( ( @@ -20,6 +21,7 @@ const IconButton = forwardRef( unsafeDisableTooltip = false, keyshortcuts, keybindingHint, + className, ...props }, forwardedRef, @@ -42,10 +44,12 @@ const IconButton = forwardRef( if (withoutTooltip) { return ( ) } else { @@ -72,7 +74,7 @@ const IconButton = forwardRef( {...mergeProps( { icon: Icon, - className: classes.IconButton, + className: clsx(classes.IconButton, className), 'data-component': 'IconButton', type: 'button', 'aria-keyshortcuts': keyshortcuts ?? undefined, diff --git a/packages/react/src/Button/LinkButton.tsx b/packages/react/src/Button/LinkButton.tsx index 7244e1ea4c7..b15aed99ca2 100644 --- a/packages/react/src/Button/LinkButton.tsx +++ b/packages/react/src/Button/LinkButton.tsx @@ -8,7 +8,7 @@ export type LinkButtonProps = BaseLinkButtonProps & ButtonProps const LinkButton = forwardRef(({children, as: Component = 'a', ...props}, forwardedRef): JSX.Element => { return ( - + {children} ) From 5b3f3d2795aa291d8172c84dbdd187fcab5fad64 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 12 Aug 2026 09:29:20 -0500 Subject: [PATCH 33/39] Migrate Text root to mergeProps Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84a0d8ca-13d3-4cb2-ab24-18fa2d9a8a14 --- packages/react/src/Text/Text.tsx | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/react/src/Text/Text.tsx b/packages/react/src/Text/Text.tsx index 7bc199f10b8..35c9fea9557 100644 --- a/packages/react/src/Text/Text.tsx +++ b/packages/react/src/Text/Text.tsx @@ -3,6 +3,7 @@ import type React from 'react' import {type ForwardedRef} from 'react' import classes from './Text.module.css' import {fixedForwardRef, type PolymorphicProps} from '../utils/modern-polymorphic' +import {mergeProps} from '../utils/mergeProps' export type TextProps = PolymorphicProps< As, @@ -21,13 +22,17 @@ function Text(props: TextProps, ref: Forwarded return ( ) } From 30a75da791ceb02a9b470353fa027b331cb8b0b3 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 13 Aug 2026 16:20:06 -0500 Subject: [PATCH 34/39] Update TextInputWithTokens snapshot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 565aa678-84c7-41f4-9281-17b47f3885ff --- .../__snapshots__/TextInputWithTokens.test.tsx.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/TextInputWithTokens/__snapshots__/TextInputWithTokens.test.tsx.snap b/packages/react/src/TextInputWithTokens/__snapshots__/TextInputWithTokens.test.tsx.snap index e281566982b..44c031c6f53 100644 --- a/packages/react/src/TextInputWithTokens/__snapshots__/TextInputWithTokens.test.tsx.snap +++ b/packages/react/src/TextInputWithTokens/__snapshots__/TextInputWithTokens.test.tsx.snap @@ -1088,7 +1088,7 @@ exports[`TextInputWithTokens > renders a truncated set of tokens 1`] = ` + @@ -1217,7 +1217,7 @@ exports[`TextInputWithTokens > renders a truncated set of tokens 1`] = ` + From b9b6140774f3346737f9bb0701a2c2f4556797ec Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 12 Aug 2026 09:35:59 -0500 Subject: [PATCH 35/39] Migrate Link root to mergeProps Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84a0d8ca-13d3-4cb2-ab24-18fa2d9a8a14 --- packages/react/src/Link/Link.tsx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/react/src/Link/Link.tsx b/packages/react/src/Link/Link.tsx index ca0b1734417..6c53350e161 100644 --- a/packages/react/src/Link/Link.tsx +++ b/packages/react/src/Link/Link.tsx @@ -5,6 +5,7 @@ import {useMergedRefs} from '../hooks' import classes from './Link.module.css' import type {ComponentProps} from '../utils/types' import {type PolymorphicProps, fixedForwardRef} from '../utils/modern-polymorphic' +import {mergeProps} from '../utils/mergeProps' type StyledLinkProps = { as?: As @@ -41,13 +42,17 @@ export const UnwrappedLink = ( return ( ) } From b89a6f1acb7939cfbf3680002993f1ede6ba6eef Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 12 Aug 2026 10:19:55 -0500 Subject: [PATCH 36/39] Migrate navigation roots to mergeProps Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84a0d8ca-13d3-4cb2-ab24-18fa2d9a8a14 --- ...underline-panels-controlled-panel-props.md | 5 + packages/react/src/ActionBar/ActionBar.tsx | 104 +++++----- .../react/src/Breadcrumbs/Breadcrumbs.tsx | 13 +- packages/react/src/NavList/NavList.tsx | 81 +++++--- packages/react/src/PageHeader/PageHeader.tsx | 183 ++++++++++++------ packages/react/src/Pagehead/Pagehead.tsx | 13 +- packages/react/src/SideNav.tsx | 11 +- .../src/SplitPageLayout/SplitPageLayout.tsx | 114 ++++++++--- packages/react/src/SubNav/SubNav.tsx | 29 ++- packages/react/src/TabNav/TabNav.tsx | 29 ++- .../deprecated/UnderlineNav/UnderlineNav.tsx | 19 +- .../UnderlinePanels/UnderlinePanels.tsx | 37 +++- .../UnderlinePanels.types.test.tsx | 19 ++ .../components/UnderlineTabbedInterface.tsx | 8 +- 14 files changed, 473 insertions(+), 192 deletions(-) create mode 100644 .changeset/underline-panels-controlled-panel-props.md create mode 100644 packages/react/src/experimental/UnderlinePanels/UnderlinePanels.types.test.tsx diff --git a/.changeset/underline-panels-controlled-panel-props.md b/.changeset/underline-panels-controlled-panel-props.md new file mode 100644 index 00000000000..c52f1dceee0 --- /dev/null +++ b/.changeset/underline-panels-controlled-panel-props.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +UnderlinePanels: Remove unsupported panel attributes that are controlled by the component. diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 4e5324b65fd..4ba42585c0b 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -13,6 +13,7 @@ import {useMergedRefs} from '../hooks' import {createDescendantRegistry} from '../utils/descendant-registry' import {OverflowObserverProvider} from '../internal/components/OverflowObserverProvider' import {useIsClipped} from '../internal/hooks/useOverflowObserver' +import {mergeProps} from '../utils/mergeProps' type ChildProps = | { @@ -332,12 +333,11 @@ function useActionBarItem(ref: React.RefObject, registryProp } export const ActionBarIconButton = forwardRef( - ({disabled, onClick, ...props}: ActionBarIconButtonProps, forwardedRef) => { + ({disabled, onClick, className, ...props}: ActionBarIconButtonProps, forwardedRef) => { const ref = useRef(null) const mergedRef = useMergedRefs(forwardedRef, ref) const {size} = React.useContext(ActionBarContext) - const {['aria-label']: ariaLabel, icon} = props const {dataOverflowingAttr} = useActionBarItem( @@ -364,11 +364,16 @@ export const ActionBarIconButton = forwardRef( return ( @@ -376,50 +381,57 @@ export const ActionBarIconButton = forwardRef( }, ) -export const ActionBarButton = forwardRef(({disabled, onClick, ...props}: ActionBarButtonProps, forwardedRef) => { - const ref = useRef(null) - const mergedRef = useMergedRefs(forwardedRef, ref) - - const {size} = React.useContext(ActionBarContext) +export const ActionBarButton = forwardRef( + ({disabled, onClick, className, children, leadingVisual, ...props}: ActionBarButtonProps, forwardedRef) => { + const ref = useRef(null) + const mergedRef = useMergedRefs(forwardedRef, ref) - const {children, leadingVisual} = props + const {size} = React.useContext(ActionBarContext) - const {dataOverflowingAttr} = useActionBarItem( - ref, - useMemo( - (): ChildProps => ({ - type: 'action', - label: children, - // Only forward the leading visual to the overflow menu when it is a component - // that can be rendered as an icon (e.g. an octicon), matching ActionBar.IconButton. - icon: typeof leadingVisual === 'function' ? (leadingVisual as ActionBarIconButtonProps['icon']) : undefined, - disabled: !!disabled, - onClick: onClick as MouseEventHandler, - }), - [children, leadingVisual, disabled, onClick], - ), - ) + const {dataOverflowingAttr} = useActionBarItem( + ref, + useMemo( + (): ChildProps => ({ + type: 'action', + label: children, + // Only forward the leading visual to the overflow menu when it is a component + // that can be rendered as an icon (e.g. an octicon), matching ActionBar.IconButton. + icon: typeof leadingVisual === 'function' ? (leadingVisual as ActionBarIconButtonProps['icon']) : undefined, + disabled: !!disabled, + onClick: onClick as MouseEventHandler, + }), + [children, leadingVisual, disabled, onClick], + ), + ) - const clickHandler = useCallback( - (event: React.MouseEvent) => { - if (disabled) return - onClick?.(event) - }, - [disabled, onClick], - ) + const clickHandler = useCallback( + (event: React.MouseEvent) => { + if (disabled) return + onClick?.(event) + }, + [disabled, onClick], + ) - return ( -