Skip to content

Forward a shared set of button props across the *Button components - #438

Merged
czarandy merged 1 commit into
mainfrom
fix/chat-send-button-rest-props
Aug 6, 2026
Merged

Forward a shared set of button props across the *Button components#438
czarandy merged 1 commit into
mainfrom
fix/chat-send-button-rest-props

Conversation

@czarandy

@czarandy czarandy commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Fixes #371.

ChatSendButton had a closed props interface with no passthrough, so nothing beyond className/style/ref/data-testid reached the rendered Button. ChatScrollButton had the opposite problem — it extended ComponentPropsWithoutRef<'div'> and spread arbitrary DOM props onto its wrapper.

Rather than opening either one up to all element props, this adds a curated set and applies it to every *Button component so they behave consistently with the regular Button.

ButtonPassthroughProps

A Pick of ButtonProps covering identity, description, and interaction:

export type ButtonPassthroughProps = Pick<
  ButtonProps,
  | 'aria-controls'
  | 'aria-describedby'
  | 'aria-details'
  | 'aria-expanded'
  | 'aria-haspopup'
  | 'aria-keyshortcuts'
  | 'aria-labelledby'
  | 'form'
  | 'id'
  | 'onFocus'
  | 'onKeyDown'
>;

Deriving from ButtonProps (the idiom CopyButton already used) keeps the components tied to Button's curated API — a prop is only forwardable if Button documents it.

Changes

  • Button — gains the two members of that set it was missing: id and onFocus. ActionElement already spread unlisted props onto both the <button> and link branches, so it only needed the onFocus type.
  • ChatSendButton — extends ButtonPassthroughProps, forwarded ahead of the contract props it sets itself (icon, label, onClick, variant).
  • ChatScrollButton — narrowed from ComponentPropsWithoutRef<'div'> to the same set. It now forwards to the inner Button rather than the wrapper div, since that is the interactive element these props describe. className/style/data-testid/ref still target the wrapper as before.
  • CopyButton, ToggleButton, SplitButton — same set added. SplitButton forwards to the primary action button, not the menu toggle.

Note on title

title was on the issue's list but is deliberately not included. Button already has a tooltip prop that renders the styled Tooltip, and icon-only buttons get one by default — so a native title would show a second browser tooltip on top of it. Happy to add it if you'd rather have the native attribute available.

Tests

A regression test per component asserting the props land on the rendered button, plus a Button test covering id/onFocus on both the button and link renderings. The aria-describedby assertions use stringContaining because Tooltip appends its own description id alongside the consumer's — verified the consumer's value survives rather than being replaced.

Full suite passes (3185 tests), lint/typecheck/format clean.

https://claude.ai/code/session_01XFkkvUQfynGRedEYDNxKoY

ChatSendButton was a closed props interface with no passthrough, so
consumers could not attach `aria-describedby`, `aria-keyshortcuts`,
`id`, `form`, or focus/keyboard handlers to it. ChatScrollButton had
the opposite problem: it extended `ComponentPropsWithoutRef<'div'>`
and spread arbitrary DOM props onto its wrapper.

Introduce `ButtonPassthroughProps`, a curated `Pick` of `ButtonProps`
covering identity, description, and interaction, and extend every
button-like component with it so they all forward the same set to the
button they render. Button itself gains the two members it was missing
(`id`, `onFocus`); ActionElement already spread unlisted props, so it
only needed the `onFocus` type.

ChatScrollButton now forwards to the inner Button rather than the
wrapper div, since that is the interactive element the props describe.

Fixes #371

Claude-Session: https://claude.ai/code/session_01XFkkvUQfynGRedEYDNxKoY
@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
silver-ui-site Ready Ready Preview Aug 6, 2026 7:54am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
silver-ui Ignored Ignored Aug 6, 2026 7:54am

@czarandy
czarandy merged commit 616c131 into main Aug 6, 2026
4 checks passed
@czarandy
czarandy deleted the fix/chat-send-button-rest-props branch August 6, 2026 14:44
czarandy added a commit that referenced this pull request Aug 6, 2026
The eight Chat components outside the button pair still extended
ComponentPropsWithoutRef and spread `...rest` onto their root, so they
accepted the whole DOM prop surface — including props that fight their
own contract (`role`, `aria-live`, `data-sender`) with no indication
which one wins.

Introduce `ChatPassthroughProps` — id plus the three aria naming and
description attributes — and extend each component with it, mirroring
`ButtonPassthroughProps` from #438. ChatComposerInput is a textarea, so
it adds the text-input props a chat composer actually needs on top:
autoComplete, enterKeyHint, maxLength, name, onBlur, onFocus, onPaste,
and onKeyDown (which the component already consumed but only declared
via the wide extension).

Two behavior fixes fall out of the narrowing:

- ChatMessage spread `...rest` before its generated `aria-label` /
  `aria-labelledby`, so a consumer label was silently dropped. The
  consumer's value now wins and falls back to the generated one.
- ChatMessageList hardcoded `aria-live="polite"` with no way to opt
  out; it is now a prop defaulting to `polite`.

Fixes #439

Claude-Session: https://claude.ai/code/session_01XFkkvUQfynGRedEYDNxKoY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ChatSendButton doesn't forward rest props

1 participant