Skip to content

fix(chip,filter-chip): forward HTML attributes onto root element #769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rohilsurana merged 3 commits into from
May 4, 2026
Merged

fix(chip,filter-chip): forward HTML attributes onto root element #769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fb06b9a
fix(chip,filter-chip): forward HTML attributes onto root element
paanSinghCoder May 4, 2026
e3fa200
fix(chip): standardize aria-label prop across components and tests
paanSinghCoder May 4, 2026
c14a51e
fix(chip): update Chip component to use standard aria-label and forwa…
paanSinghCoder May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions apps/www/src/components/playground/chip-examples.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function ChipExamples() {
<Chip
isDismissible
onDismiss={() => alert('dismissed')}
ariaLabel='Dismissible chip'
aria-label='Dismissible chip'
>
Dismissable Chip
</Chip>
Expand All @@ -44,7 +44,7 @@ export function ChipExamples() {
color='accent'
isDismissible
onDismiss={() => alert('dismissed')}
ariaLabel='Dismissible chip'
aria-label='Dismissible chip'
>
Dismissable Chip
</Chip>
Expand All @@ -53,7 +53,7 @@ export function ChipExamples() {
color='accent'
isDismissible
onDismiss={() => alert('dismissed')}
ariaLabel='Dismissible chip'
aria-label='Dismissible chip'
>
Dismissable Chip
</Chip>
Expand Down
6 changes: 3 additions & 3 deletions apps/www/src/content/docs/components/chip/demo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ export const dismissableDemo = {
type: 'code',
code: `
<Flex gap="large">
<Chip isDismissible onDismiss={() => alert('dismissed')} ariaLabel="Dismissible chip">Dismissable Chip</Chip>
<Chip variant="outline" color="accent" isDismissible onDismiss={() => alert('dismissed')} ariaLabel="Dismissible chip">Dismissable Chip</Chip>
<Chip variant="filled" color="accent" isDismissible onDismiss={() => alert('dismissed')} ariaLabel="Dismissible chip">Dismissable Chip</Chip>
<Chip isDismissible onDismiss={() => alert('dismissed')} aria-label="Dismissible chip">Dismissable Chip</Chip>
<Chip variant="outline" color="accent" isDismissible onDismiss={() => alert('dismissed')} aria-label="Dismissible chip">Dismissable Chip</Chip>
<Chip variant="filled" color="accent" isDismissible onDismiss={() => alert('dismissed')} aria-label="Dismissible chip">Dismissable Chip</Chip>
</Flex>`
};

Expand Down
2 changes: 1 addition & 1 deletion apps/www/src/content/docs/components/chip/props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ export interface ChipProps {
role?: string;

/** Custom accessibility label for the chip */
ariaLabel?: string;
'aria-label'?: string;
}
26 changes: 26 additions & 0 deletions docs/V1-migration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ This guide covers all breaking changes when upgrading from the last stable Radix
- [Avatar](#avatar)
- [Breadcrumb](#breadcrumb)
- [Button](#button)
- [Chip](#chip)
- [Checkbox](#checkbox)
- [New: `Checkbox.Group`](#new-checkboxgroup)
- [New Features](#new-features)
Expand Down Expand Up @@ -418,6 +419,31 @@ Unchanged: `size`, `radius`, `variant`, `color`, `fallback`, `src`, `alt`, `clas

---

### Chip

**`ariaLabel` prop removed** -- use the standard `aria-label` HTML attribute:

```tsx
// Before
<Chip ariaLabel="Dismissible chip" isDismissible onDismiss={...}>
Tag
</Chip>

// After
<Chip aria-label="Dismissible chip" isDismissible onDismiss={...}>
Tag
</Chip>
```

`Chip` now forwards all standard HTML attributes through `...props`, making the dedicated `ariaLabel` prop redundant. The auto-fallback to string children when no label is supplied is unchanged:

```tsx
// Still works — uses "Tag" as the accessibility label
<Chip>Tag</Chip>
```

---

### Checkbox

1. **Indeterminate API changed** -- `checked="indeterminate"` replaced by separate `indeterminate` boolean:
Expand Down
47 changes: 43 additions & 4 deletions packages/raystack/components/chip/__tests__/chip.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,18 +214,57 @@ describe('Chip', () => {
expect(chip).toHaveAttribute('aria-label', 'Test Label');
});

it('uses custom ariaLabel when provided', () => {
render(<Chip ariaLabel='Custom Label'>Test Chip</Chip>);
it('uses custom aria-label when provided', () => {
render(<Chip aria-label='Custom Label'>Test Chip</Chip>);

const chip = screen.getByRole('status');
expect(chip).toHaveAttribute('aria-label', 'Custom Label');
});

it('custom ariaLabel overrides string children', () => {
render(<Chip ariaLabel='Override Label'>String Child</Chip>);
it('custom aria-label overrides string children', () => {
render(<Chip aria-label='Override Label'>String Child</Chip>);

const chip = screen.getByRole('status');
expect(chip).toHaveAttribute('aria-label', 'Override Label');
});

it('accepts native aria-label attribute', () => {
render(<Chip aria-label='Native Label'>String Child</Chip>);

const chip = screen.getByRole('status');
expect(chip).toHaveAttribute('aria-label', 'Native Label');
});
Comment on lines +231 to +236
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Redundant test — identical assertion path to the test at Line 217.

"accepts native aria-label attribute" (lines 231-236) and "uses custom aria-label when provided" (lines 217-222) both render <Chip aria-label='...' /> and assert toHaveAttribute('aria-label', ...). They exercise the exact same code path; the only variation is the label/children string.

The "override" test at lines 224-229 is distinct (string child + explicit aria-label — checks precedence) and should be kept.

🗑️ Proposed removal 
-    it('accepts native aria-label attribute', () => {
-      render(<Chip aria-label='Native Label'>String Child</Chip>);
-
-      const chip = screen.getByRole('status');
-      expect(chip).toHaveAttribute('aria-label', 'Native Label');
-    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('accepts native aria-label attribute', () => {
render(<Chip aria-label='Native Label'>String Child</Chip>);
const chip = screen.getByRole('status');
expect(chip).toHaveAttribute('aria-label', 'Native Label');
});
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/chip/__tests__/chip.test.tsx` around lines 231 -
236, The test "accepts native aria-label attribute" is redundant because it
duplicates the assertion path of the existing "uses custom aria-label when
provided" test; remove the entire it block named "accepts native aria-label
attribute" from the chip.test.tsx file (the render(<Chip aria-label='...'>...);
+ expect(...toHaveAttribute('aria-label', ...)) block) and keep the distinct
"uses custom aria-label when provided" and the override test that checks
precedence (both referenced in the same test suite).

});

describe('Forwarded HTML attributes', () => {
it('forwards arbitrary HTML attributes onto the root span', () => {
render(
<Chip
id='my-chip'
data-testid='chip-root'
data-state='active'
title='Tooltip'
>
Test Chip
</Chip>
);

const chip = screen.getByTestId('chip-root');
expect(chip).toHaveAttribute('id', 'my-chip');
expect(chip).toHaveAttribute('data-state', 'active');
expect(chip).toHaveAttribute('title', 'Tooltip');
});

it('forwards mouse events from the spread props', () => {
const onMouseEnter = vi.fn();
render(
<Chip onMouseEnter={onMouseEnter} data-testid='chip-root'>
Test Chip
</Chip>
);

fireEvent.mouseEnter(screen.getByTestId('chip-root'));
expect(onMouseEnter).toHaveBeenCalledTimes(1);
});
});
});
32 changes: 14 additions & 18 deletions packages/raystack/components/chip/chip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client';

import { cva, type VariantProps } from 'class-variance-authority';
import { ReactNode } from 'react';
import { ComponentProps, ReactNode } from 'react';

import styles from './chip.module.css';

Expand All @@ -27,19 +27,15 @@ const chip = cva(styles.chip, {
}
});

type ChipProps = VariantProps<typeof chip> & {
trailingIcon?: ReactNode;
leadingIcon?: ReactNode;
isDismissible?: boolean;
children: ReactNode;
className?: string;
onDismiss?: () => void;
onClick?: () => void;
role?: string;
ariaLabel?: string;
disabled?: boolean;
'data-state'?: string;
};
type ChipProps = ComponentProps<'span'> &
VariantProps<typeof chip> & {
trailingIcon?: ReactNode;
leadingIcon?: ReactNode;
isDismissible?: boolean;
children: ReactNode;
onDismiss?: () => void;
disabled?: boolean;
};

export const Chip = ({
variant,
Expand All @@ -53,9 +49,9 @@ export const Chip = ({
onDismiss,
onClick,
role = 'status',
ariaLabel,
disabled,
'data-state': dataState
'aria-label': ariaLabel,
...props
}: ChipProps) => {
const handleDismiss = (e: React.MouseEvent) => {
e.stopPropagation();
Expand All @@ -64,14 +60,14 @@ export const Chip = ({

return (
<span
{...props}
className={chip({ variant, size, color, className })}
role={role}
aria-label={
ariaLabel || (typeof children === 'string' ? children : undefined)
ariaLabel ?? (typeof children === 'string' ? children : undefined)
}
onClick={disabled ? undefined : onClick}
data-disabled={disabled || undefined}
data-state={dataState}
>
{leadingIcon && (
<span
Expand Down
17 changes: 17 additions & 0 deletions packages/raystack/components/filter-chip/__tests__/filter-chip.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,21 @@ describe('FilterChip', () => {
expect(input).toHaveValue('initial value');
});
});

describe('Forwarded HTML attributes', () => {
it('forwards arbitrary HTML attributes onto the root div', () => {
render(
<FilterChip
label='Name'
id='my-filter'
data-testid='filter-root'
title='Tooltip'
/>
);

const root = screen.getByTestId('filter-root');
expect(root).toHaveAttribute('id', 'my-filter');
expect(root).toHaveAttribute('title', 'Tooltip');
});
});
});
9 changes: 4 additions & 5 deletions packages/raystack/components/filter-chip/filter-chip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { Cross1Icon } from '@radix-ui/react-icons';
import { cva, cx, VariantProps } from 'class-variance-authority';
import { ReactElement, ReactNode, useCallback, useState } from 'react';
import { ComponentProps, ReactElement, useCallback, useState } from 'react';
import {
FilterOperation,
FilterOperator,
Expand Down Expand Up @@ -32,13 +32,12 @@ const chip = cva(styles.chip, {
}
});

export interface FilterChipProps extends VariantProps<typeof chip> {
export interface FilterChipProps
extends ComponentProps<'div'>,
VariantProps<typeof chip> {
label: string;
value?: string;
onRemove?: () => void;
className?: string;
ref?: React.RefObject<HTMLDivElement>;
children?: ReactNode;
columnType?: FilterTypes;
options?: FilterSelectOption[];
onValueChange?: (value: any, operation: string) => void;
Expand Down
Loading