Skip to content

Migrate initial components to mergeProps - #8256

Open
joshblack wants to merge 2 commits into
migrate/merge-props-migratorfrom
migrate/merge-props-batch-01
Open

Migrate initial components to mergeProps#8256
joshblack wants to merge 2 commits into
migrate/merge-props-migratorfrom
migrate/merge-props-batch-01

Conversation

@joshblack

@joshblack joshblack commented Jul 31, 2026

Copy link
Copy Markdown
Member

This PR contains the first bounded ADR-025 implementation batch. It migrates three reviewed component patterns without mixing changes into the tooling foundation or migration driver.

Changelog

New

Changed

  • Update VisuallyHidden to merge component and consumer class names through mergeProps.
  • Update TokenBase to run component keyboard behavior before consumer handlers while keeping refs separate.
  • Update Radio to preserve radio semantics and compose context and consumer change handlers.

Removed

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

These changes correct existing component behavior to match the intended contracts. They do not add a supported public capability, so this batch does not include release notes.

Testing & Reviewing

Please review class-name ordering, optional handler composition, ref handling, and the Radio type invariant. The migration report should no longer list the three assigned source files.

@changeset-bot

changeset-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 08170af

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added the staff Author is a staff member label Jul 31, 2026
@joshblack joshblack added the skip changeset This change does not need a changelog label Jul 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

@github-actions github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Jul 31, 2026
@github-actions
github-actions Bot temporarily deployed to storybook-preview-8256 July 31, 2026 18:47 Inactive
@joshblack
joshblack force-pushed the migrate/merge-props-batch-01 branch from f791ad9 to d6b13d8 Compare August 10, 2026 20:05
@joshblack
joshblack marked this pull request as ready for review August 10, 2026 20:13
@joshblack
joshblack requested a review from a team as a code owner August 10, 2026 20:13
@joshblack
joshblack requested review from liuliu-dev and a lite review from Copilot August 10, 2026 20:13
@github-actions
github-actions Bot temporarily deployed to storybook-preview-8256 August 10, 2026 20:16 Inactive

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements the first bounded ADR-025 batch by migrating three component patterns (VisuallyHidden, TokenBase, Radio) to use mergeProps, aiming to standardize className merging and event-handler composition semantics.

Changes:

  • VisuallyHidden: switch to mergeProps for composing component and consumer className.
  • TokenBase: move keyboard removal behavior into a component handler and compose it with consumer onKeyDown via mergeProps (with a new ordering test).
  • Radio: enforce the type="radio" invariant at the type level and compose RadioGroup context + consumer onChange via mergeProps (with new runtime and type-level tests).
Show a summary per file
File Description
packages/react/src/VisuallyHidden/VisuallyHidden.tsx Replaces clsx usage with mergeProps for className composition.
packages/react/src/Token/TokenBase.tsx Uses mergeProps to compose onKeyDown and className, ensuring component keyboard behavior runs before consumer handlers.
packages/react/src/Token/tests/Token.test.tsx Adds a test asserting removal behavior runs before consumer onKeyDown.
packages/react/src/Radio/Radio.types.test.tsx Adds a type-level invariant test preventing type override.
packages/react/src/Radio/Radio.tsx Uses mergeProps for prop composition and omits type from props to preserve radio semantics.
packages/react/src/Radio/Radio.test.tsx Adds a runtime assertion that rendered input has type="radio".

Review details

Tip

Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread packages/react/src/Radio/Radio.tsx Outdated
Comment on lines +72 to +77
{...mergeProps(
{
onChange: handleOnChange,
className: clsx(sharedClasses.Input, classes.Radio),
'aria-checked': checked ? ('true' as const) : ('false' as const),
},
@github-actions
github-actions Bot temporarily deployed to storybook-preview-8256 August 10, 2026 20:24 Inactive
@joshblack
joshblack force-pushed the migrate/merge-props-batch-01 branch from d6b13d8 to d828879 Compare August 10, 2026 21:10
@joshblack
joshblack force-pushed the migrate/merge-props-batch-01 branch from d6b13d8 to d828879 Compare August 10, 2026 21:10
@github-actions
github-actions Bot requested a deployment to storybook-preview-8256 August 10, 2026 21:14 Abandoned
Apply the validated class name, event handler, ref, and component-owned prop patterns in a dedicated implementation batch.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 00bfd295-6ed9-4119-8d63-103a0b3c63e8
@joshblack
joshblack force-pushed the migrate/merge-props-batch-01 branch from d828879 to 68c2e7c Compare August 10, 2026 21:19
@github-actions
github-actions Bot requested a deployment to storybook-preview-8256 August 10, 2026 21:23 Abandoned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm skip changeset This change does not need a changelog staff Author is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants