Skip to content

@kondee/chip md3 - #5081

Open
k0ndee wants to merge 7 commits into
callstack:mainfrom
k0ndee:@kondee/chip-md3
Open

@kondee/chip md3#5081
k0ndee wants to merge 7 commits into
callstack:mainfrom
k0ndee:@kondee/chip-md3

Conversation

@k0ndee

@k0ndee k0ndee commented Aug 27, 2026

Copy link
Copy Markdown

Motivation

Refactors Chip toward the v6/MD3 API and implementation direction.

  • Extracts Chip-specific sizing, color, and typescale values into a dedicated tokens.ts file, following the pattern used by other recently modernized components (e.g. Badge).
  • Simplifies the internal layout: the ripple/state-layer is now rendered in its own layer behind the content instead of being nested inside it, and content spacing is derived from tokens instead of inline math.
  • Removes the press-time elevation animation (Animated.timing driven scale/elevation on press) in favor of a static elevation derived from the elevated and disabled props.
  • Reworks selected, outlined, disabled, icon/avatar, and close-icon color resolution (getChipColors in utils.ts, renamed from helpers.tsx) to better align with MD3 state and color roles, including a dedicated ripple color and an avatar selected-overlay color.
  • Adds a proper minimum touch target (48dp) via hitSlop, and moves the close icon into TouchableRipple for consistent ripple/state-layer feedback.
  • Removes the compact and showSelectedOverlay props, which are no longer part of the MD3-aligned API (the selected overlay is now handled automatically for avatars).
  • Changes the default mode from flat to outlined.

The example app has been updated to match the new API (e.g. dropping now-removed showSelectedOverlay usage) and to showcase MD3-oriented Chip states.

Related issue

Closes #4931

Related PRs

Based on the #5002

Test plan

  • Verified Chip examples in the example app.
  • Ran typecheck:
    yarn typecheck
    
  • Ran lint:
    yarn lint
    
  • Ran tests:
    yarn test --runInBand --watchman=false
    
  • Ran docs build:
    yarn docs build
    
Screenshot_1788266249

@k0ndee
k0ndee marked this pull request as ready for review August 31, 2026 12:22
Comment thread src/components/Chip/Chip.tsx Outdated
]}
>
{icon ? (
<View style={styles.rippleContent} />

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Try keeping the label inside TouchableRipple, or deriving aria-label from it - as a pointerEvents="none" sibling it never reaches the button, so getByRole('button', { name: 'Example Chip' }) resolves on main and returns null here. Worth a test to pin it.

Comment thread src/components/Chip/tokens.ts Outdated
selectedIconColor: 'onSecondaryContainer',
trailingIconColor: 'onSurfaceVariant',
selectedTrailingIconColor: 'onSecondaryContainer',
outlineColor: 'outline',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Was outline intentional here? main uses outlineVariant (helpers.tsx:48), and all four live chip namespaces resolve to outline-variant — the only token giving outline is md.comp.input-chip.outline.color, which is deprecated.

Comment thread src/components/Chip/Chip.tsx Outdated
theme={theme}
>
<Icon
source={hasTrailingIcon ? trailingIcon! : (closeIcon ?? 'close')}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Try dropping the ! - it typechecks without one, and @typescript-eslint/no-non-null-assertion is error-level on main (eslint.config.mjs:150).

Comment on lines +381 to +385
aria-label={
hasTrailingIcon
? trailingIconAccessibilityLabel
: closeIconAccessibilityLabel
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Worth giving trailingIconAccessibilityLabel a default, or requiring it alongside onTrailingIconPress - <Chip trailingIcon="menu-down" onTrailingIconPress={…}> currently renders a role="button" with no accessible name, where the close button defaults to 'Close'.

Comment thread src/components/Chip/Chip.tsx Outdated
const styles = StyleSheet.create({
container: {
borderWidth: StyleSheet.hairlineWidth,
height: ChipTokens.containerHeight,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Try minHeight here - with overflow: 'hidden' on the content (line 438) a fixed height clips the label at larger system font sizes. main reached the same 32dp via lineHeight: 20 plus marginVertical: 6, so it scaled.

-    height: ChipTokens.containerHeight,
+    minHeight: ChipTokens.containerHeight,

*/
const Chip = ({
mode = 'flat',
mode = 'outlined',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Worth adding a Chip section to migration.md - compact and showSelectedOverlay are gone and this default flip changes every existing <Chip>. The TextInput entry is a template, and the generated docs are already done.

disabledColor: 'onSurface',
} as const satisfies Record<string, ColorRole>;

export const ChipTokens = { ...sizes, ...colors };

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Worth settling this one before it lands: @satya164 asked for the constants to be exported directly on #5002, but Checkbox, Switch and FAB all use the wrapper. Which shape should Chip follow?

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.

refactor(chip): improve selection model and MD3 compliance

3 participants