feat(core): add composeEventHandlers; forward consumer onClick in SegmentedControlItem#3863
Merged
Merged
Conversation
…mentedControlItem
SegmentedControlItem spread {...rest} onto the button but then set its own
onClick after it, silently dropping any consumer onClick. It now composes the
consumer handler with selection: the consumer's runs first and can call
preventDefault() to opt out.
Add composeEventHandlers(...handlers) — chains handlers in argument order and
stops when one prevents default. This is the shared home for the
consumer-first / respect-defaultPrevented composition that was copy-pasted
across Button, ClickableCard, Toolbar, and TabList.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
josephfarina
approved these changes
Jul 12, 2026
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR No new or modified components detected. Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related changes to how components handle event handlers passed through
...rest.1.
SegmentedControlItemsilently dropped a consumeronClick. It spreads{...rest}onto the<button>but then sets its ownonClick={handleClick}after the spread, so any consumer-suppliedonClickwas overwritten and never fired. This is the same class of bug as #3738 / #3852 (props that type-check viaBasePropsbut never reach the DOM), except here the prop reached the element and was then clobbered.2. Add
composeEventHandlers. When a component owns an interaction (a click that selects, a keydown that drives arrow navigation) and accepts a consumer handler for the same event, it must compose them, not clobber one. That logic — call handlers in order, stop if one callspreventDefault()— was copy-pasted acrossButton,ClickableCard,Toolbar, andTabListwith subtly different semantics. This adds one shared utility for it, alongsidemergeProps/mergeRefs.Change
packages/core/src/utils/composeEventHandlers.ts—composeEventHandlers(...handlers)returns a single handler that calls each in argument order and stops when one prevents default. Order is the precedence knob: consumer-first (the default here) lets a consumerpreventDefault()to opt out of built-in behavior; component-first runs built-in behavior unconditionally.SegmentedControlItem.tsx— destructureonClick: onClickPropout of...restand compose it with selection (composeEventHandlers(onClickProp, select)), consumer-first.utils/index.ts(flows to the package root via the existingexport * from './utils').Precedence policy
This PR encodes the per-category rule now documented in the API Conventions wiki:
className/style/xstylemergeProps)role, ownedaria-*, computedid){...rest})composeEventHandlers)data-testid, otherdata-*){...rest})Follow-up
Scoped intentionally small. A follow-up will migrate the existing hand-rolled composition sites (
Button,ClickableCard,SelectableCard,Toolbar,TabList,Lightbox,ResizeHandle, …) ontocomposeEventHandlersonce this lands.Test plan
composeEventHandlersunit tests (7): argument order, skipsundefined, same event to each, stops afterpreventDefault, consumer opt-out, runs when not prevented, all-undefined is safe.SegmentedControlItemtests (2): a consumeronClickfires alongside selection; a consumeronClickthat callspreventDefault()cancels selection.pnpm -F @astryxdesign/core typecheckclean; SegmentedControl suite (39) + utils suite green;pnpm -F @astryxdesign/core buildgreen;sync-exports --checkclean.@astryxdesign/corepatch changeset added.