Skip to content

feat(fuselage): Add onDismiss and size props to Chip rendering a dedicated dismiss IconButton - #2145

Open
jeanfbrito wants to merge 13 commits into
mainfrom
feat/chip-dismiss-icon-button
Open

feat(fuselage): Add onDismiss and size props to Chip rendering a dedicated dismiss IconButton#2145
jeanfbrito wants to merge 13 commits into
mainfrom
feat/chip-dismiss-icon-button

Conversation

@jeanfbrito

@jeanfbrito jeanfbrito commented Aug 6, 2026

Copy link
Copy Markdown
Member

Proposed changes (including videos or screenshots)

Jira task: DSN-183 — Review chips (fuselage)

Before / After

Before — the whole chip is a <button> and any click dismisses; the ✕ is decorative, the label fails AA contrast:

Legacy chips before this PR

After — a dedicated, accessible dismiss IconButton (default and custom dismissLabel):

Dismissible chips after this PR

Sizesmedium (28px) and small (20px), with avatar (20px/16px) and leading-icon applications:

Chip sizes

Other trailing actions — the icon prop generalizes the trailing IconButton beyond dismiss:

Chips with custom trailing actions

Problem

Chip's dismiss action is currently triggered by clicking anywhere on the chip — the whole root element is a <button>, and the ✕ is just a decorative <Icon>. This makes the close affordance imprecise (any accidental click on the label or avatar removes the selection) and misleading for assistive technology, since the accessible name of the "close button" is the entire chip label.

Solution

This PR introduces an opt-in onDismiss prop that moves the dismiss trigger to a dedicated, real button:

  • New mode (onDismiss provided): the chip root becomes a non-interactive <span> and an internal IconButton (icon='cross') is rendered as the only dismiss trigger. The button:
    • carries an accessible name and a tooltip (title) via the new dismissLabel prop (default: "Dismiss");
    • is natively keyboard-operable (Tab + Enter/Space) and is the only focusable element of the chip;
    • calls preventDefault() on mousedown so it never steals focus from multiselect/autocomplete anchor inputs;
    • honors disabled.
    • onClick/onMouseDown, when passed alongside onDismiss, behave as ordinary handlers on the chip body — they no longer mean "dismiss".
    • spacing is handled by the chip's own CSS (gap + padding on .rcx-chip--dismissible) instead of a Margins wrapper. Per design, the chip has no top/bottom/right padding — the dismiss IconButton sits flush against those edges — and 4px inline-start padding. A text-only label gets 4px more, so its ~8px visual inset matches what the dismiss button's inner padding produces on the other side; a leading avatar or icon keeps the plain 4px.
    • a new size prop selects the chip's dimensions: medium (default) renders a 28px dismiss IconButton (28px chip) with a 20px default avatar, small a 20px one (20px chip) with a 16px default avatar.
    • a new leadingIcon prop renders an icon before the label as a flex sibling (vertically centered by the chip, sized by size), and a new icon prop customizes the trailing IconButton icon (default cross), so the same anatomy can express other actions — e.g. 'chevron-down' for a chip that opens a filters menu. Since size collides with the Box styling prop of the same name (consumed by withBoxStyling before reaching the component), the exported Chip extracts it and forwards it internally as chipSize.
  • Legacy mode (no onDismiss): the DOM is byte-for-byte identical to the current release — root <button class="rcx-box rcx-chip">, whole-chip click dismisses, decorative cross icon. This behavior is now documented as deprecated (prose in the component JSDoc and an @deprecated tag on renderDismissSymbol only, so the component itself is not flagged as deprecated at call sites).
// Before (still works, now deprecated behavior)
<Chip onClick={handleRemove}>Marie Rowe</Chip>

// After
<Chip onDismiss={handleRemove} dismissLabel='Remove Marie Rowe'>Marie Rowe</Chip>

Accessibility: label contrast fix

The chip label color moved from font(secondary-info) to button(on-secondary) — the foreground token that pairs with the chip's button-secondary background. This fixes the WCAG AA failure previously flagged by the a11y addon on every Chip story (4.39:1): now 12.71:1 on the light theme and 9.08:1 on the dark theme. Disabled states keep their dedicated tokens in both modes. Per design, the chip label also moved from the p2 font scale to p2m (same 14px/20px style, medium weight — from the typography tokens, no hardcoded font-weight). Since these tokens are shared, legacy chips get both updates too — the only visual changes to the legacy mode (its DOM remains untouched).

Fixes found while polishing the layout

Dropping the internal Margins wrapper exposed two ways a consumer's Margins reached the chip, both fixed here:

  • through the BoxTransforms context, which pushed a margin onto the inner Box-based IconButton and inflated the chip (28px → 36px inside MultiSelect). The chip now resets that context for its children, exactly as Box does, since its inner spacing is owned by gap.
  • through a className patched onto its direct children, which SelectedOptions was dropping when it destructured its props — losing the 4px spacing between MultiSelect chips.

Verified against main in the same viewport: chip height (28px), outer margin (4px) and wrapping behavior now match, with the chip only ~9px wider from the larger dismiss button.

Backward compatibility

This is intentionally a non-breaking minor release (changeset included). We audited all 11 direct Chip usages in the main Rocket.Chat repository before deciding on this design:

  • All of them pass onClick as the removal/selection handler → the legacy path keeps that contract untouched.
  • Existing snapshot tests in Rocket.Chat record <button class="rcx-chip"> as the chip root → legacy DOM is unchanged.
  • Tests using screen.getByRole('button') on legacy chips keep passing.
  • No consumer uses button-only HTML attributes (form, type, name), so the type surface remains compatible.

Internal consumers migrated

AutoComplete, MultiSelect/SelectedOptions, and PaginatedMultiSelect now use onDismiss, so every product built on these pickers gets the improved dismiss behavior automatically by upgrading fuselage — no code changes required downstream.

Notes on the migration:

  • AutoComplete: removal no longer relies on event.currentTarget.value (which would resolve to the inner IconButton after the change); a removeValue(value) helper is used instead. The public onRemove contract of renderSelected is untouched.
  • SelectedOptions: keeps its external prop surface (onMouseDown) and maps it internally to onDismiss. MultiSelect no longer passes tabIndex, so the dismiss IconButton is the only focusable element (review feedback), and the remaining props are forwarded so the surrounding Margins can still space the chips.
  • PaginatedMultiSelect: kept role='option'/tabIndex={-1} on the chips — they implement the ARIA listbox pattern (parent role='listbox') and are unrelated to dismiss focusability.

Storybook

All main stories now showcase the new onDismiss chip: Default, Dismissible (default and custom dismissLabel, captioned side by side), Sizes (medium/small, showing both avatar and leading-icon applications), With Thumb Url, With Thumb, With Icon (leadingIcon instead of an avatar), Other Actions (trailing icon beyond dismiss: dropdown and edit), and Disabled. The entire legacy (whole-chip dismiss) scope is isolated in a single deprecated Legacy Dismissible story showing every legacy possibility captioned: dismissible, with thumb, disabled, and no handler. Docs and argTypes describe the new props (including the dismissLabel tooltip) and flag the deprecated ones.

How it was verified

  • Unit tests: 54/54 passing across the affected suites (Chip, AutoComplete, MultiSelect, Options, PaginatedSelect/PaginatedMultiSelect). New Chip specs assert: body click does not dismiss, IconButton click dismisses exactly once, accessible name and tooltip (default and custom), disabled blocks the button, and onClick coexists with onDismiss without triggering dismissal.
  • Static checks: eslint (0 errors), stylelint, prettier, tsc --noEmit, and the d.ts build (tsc -p tsconfig.build.json --emitDeclarationOnly) all clean.
  • Manual verification in Storybook (live browser):
    • Clicking the chip body logs no action; clicking the ✕ logs dismiss — trigger confined to the button.
    • In MultiSelect, removing a selected option via its ✕ removes only that option and the anchor keeps focus (no dropdown flicker), confirming the mousedown-preventDefault guard works. The accessibility tree shows the dismiss IconButtons as the only interactive elements inside the chips.
    • Dismissible chip measured at 28px height with symmetric 4px padding and 4px gap — matching the legacy chip's proportions.
    • Label contrast measured at 9.08:1 (dark) in the live page and 12.71:1 (light) from the design tokens — the a11y-addon violation on Chip stories is fixed.
    • Legacy story DOM renders identically to the previous release.

Why a span root instead of nesting buttons

A <button> inside a <button> is invalid HTML and unreliably handled by browsers, so the dismissible variant's root must stop being a button. That structural change is exactly what's gated behind the opt-in onDismiss prop to keep existing consumers safe.

🤖 Generated with Claude Code

@changeset-bot

changeset-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: edec7bc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@rocket.chat/fuselage Minor

Not sure what this means? Click here to learn what changesets are.

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

Comment thread packages/fuselage/src/components/MultiSelect/SelectedOptions.tsx Outdated
Comment thread packages/fuselage/src/components/Chip/Chip.tsx Outdated
@ivans-netto
ivans-netto force-pushed the feat/chip-dismiss-icon-button branch from ab6d7fc to 6ec0482 Compare August 10, 2026 14:22
@ivans-netto ivans-netto changed the title feat(fuselage): add onDismiss prop to Chip rendering a dedicated dismiss IconButton feat(fuselage): Add onDismiss and size props to Chip rendering a dedicated dismiss IconButton Aug 10, 2026
ivans-netto added a commit that referenced this pull request Aug 10, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ivans-netto
ivans-netto requested a review from dougfabris August 10, 2026 19:39
ivans-netto and others added 2 commits August 10, 2026 16:41
… Chip label

The previous font token (secondary-info) rendered at 4.39:1 against the
chip's secondary background, failing WCAG AA; button(on-secondary) is the
matching foreground (12.71:1 light, 9.08:1 dark). The label also moves
from the p2 to the p2m font scale — same 14px/20px style at medium
weight, sourced from the typography tokens instead of a hardcoded
font-weight. Disabled states keep their dedicated tokens.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When onDismiss is provided, the chip root becomes a non-interactive
<span> and a dedicated, accessible IconButton is the only dismiss
trigger: dismissLabel sets its accessible name and tooltip, icon
(default 'cross') lets it express other actions (e.g. chevron-down),
and size selects the dimensions — medium (28px chip/button, 20px
avatar) or small (20px chip/button, 16px avatar). The dismissible chip
is spaced by its own CSS (gap + inline-start padding only, button flush
against the other edges). The size prop is forwarded internally as
chipSize because `size` is a Box styling prop that withBoxStyling would
otherwise consume.

Legacy mode (no onDismiss) renders byte-for-byte the same DOM as before
and is documented as deprecated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ivans-netto added a commit that referenced this pull request Aug 10, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ivans-netto
ivans-netto force-pushed the feat/chip-dismiss-icon-button branch from 696ec85 to c339d5b Compare August 10, 2026 19:43
ivans-netto and others added 5 commits August 10, 2026 16:46
Main stories showcase the onDismiss chip (Default, Dismissible with
default/custom label, Sizes with avatar and icon applications, thumbs,
OtherActions, Disabled), each variant captioned via a ChipVariants
helper following the Badge stories pattern. The whole legacy scope is
isolated in a single deprecated LegacyDismissible story.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AutoComplete, MultiSelect/SelectedOptions and PaginatedMultiSelect now
use the dedicated dismiss IconButton. AutoComplete removal no longer
relies on event.currentTarget.value; SelectedOptions keeps its external
onMouseDown contract, maps it to onDismiss, and stops forwarding
tabIndex so the IconButton is the only focusable element.
PaginatedMultiSelect keeps role='option'/tabIndex={-1} (ARIA listbox).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ivans-netto
ivans-netto force-pushed the feat/chip-dismiss-icon-button branch from c339d5b to 80db667 Compare August 10, 2026 19:46
ivans-netto and others added 6 commits August 10, 2026 17:03
Regenerated in the Playwright Docker container: Chip stories (label
color/weight, flush dismiss button, new Sizes/WithIcon/OtherActions/
LegacyDismissible stories) plus the AutoComplete and MultiSelect
stories that render chips.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Chip

`Margins` reaches children two ways, and the dismissible chip was caught
by both once it stopped wrapping its content in a `Margins` of its own:

- via the `BoxTransforms` context, which pushed a margin onto the inner
  `Box`-based IconButton and inflated the chip (28px -> 36px inside
  MultiSelect). The chip now resets that context for its children, the
  same way `Box` does, since its inner spacing is owned by `gap`.
- via a className patched onto its direct children, which
  `SelectedOptions` was dropping when it destructured its props — losing
  the spacing between MultiSelect chips. It forwards them again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
leadingIcon renders an icon before the label as a flex sibling, so the
chip centers it vertically and sizes it from `size` (20px on medium, 16px
on small). Icons passed inside `children` instead align on the text
baseline and always read slightly high.

A text-only label also gains a 4px inline-start margin, matching the
~8px visual inset the trailing IconButton's inner padding produces on the
other side; a leading avatar or icon keeps the plain 4px padding.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The inner IconButton no longer carries the margin class leaked from the
consumer's Margins, and the chip root carries its spacing class again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants