Skip to content

fix(ColorPicker): add screen reader support for color announcements#3346

Open
rivka-ungar wants to merge 6 commits intomasterfrom
fix/color-picker-voiceover-accessibility
Open

fix(ColorPicker): add screen reader support for color announcements#3346
rivka-ungar wants to merge 6 commits intomasterfrom
fix/color-picker-voiceover-accessibility

Conversation

@rivka-ungar
Copy link
Copy Markdown
Contributor

@rivka-ungar rivka-ungar commented Apr 9, 2026

User description

Summary

  • Added aria-activedescendant to the color grid so VoiceOver announces the active color during keyboard navigation
  • Changed aria-label from raw color values (done-green) to formatted names (Done Green)
  • Changed ARIA roles from grid/gridcell to listbox/option for proper color picker semantics
  • Added aria-selected to each color option for selection state announcements
  • Made Clickable inside each color item role="presentation" to avoid double screen reader announcements
  • Added unique ID generation per grid instance (following the List component pattern) to support multiple ColorPickers on one page

Monday Task

https://monday.monday.com/boards/3532714909/pulses/11709234599

Test plan

  • Unit tests pass (10/10)
  • Lint passes (0 errors)
  • Build passes
  • Verify VoiceOver announces color names when navigating with arrow keys
  • Verify VoiceOver announces selection state (selected / not selected)
  • Verify multiple ColorPickers on the same page don't have conflicting IDs

🤖 Generated with Claude Code


PR Type

Bug fix


Description

  • Enhanced ColorPicker screen reader support with proper ARIA attributes

    • Added aria-activedescendant for active color announcements
    • Changed roles from grid/gridcell to listbox/option for semantic accuracy
    • Added aria-selected to indicate selection state
  • Implemented formatted color names in aria-labels (e.g., "Done Green" instead of "done-green")

  • Added unique ID generation per grid instance to support multiple ColorPickers

  • Set role="presentation" on Clickable to prevent duplicate screen reader announcements

  • Updated all tests to use formatted color names matching new aria-labels


Diagram Walkthrough

flowchart LR
  A["ColorPicker Grid"] -->|"aria-activedescendant"| B["Active Color Item"]
  A -->|"role=listbox"| C["Semantic Container"]
  D["Color Item"] -->|"role=option"| E["Proper ARIA Role"]
  D -->|"aria-selected"| F["Selection State"]
  D -->|"formatted aria-label"| G["Screen Reader Text"]
  H["Clickable"] -->|"role=presentation"| I["No Duplicate Announcements"]
  J["Grid ID Generator"] -->|"unique IDs"| K["Multiple Instances Support"]
Loading

File Walkthrough

Relevant files
Enhancement
ColorPickerColorsGrid.tsx
Add ARIA attributes and ID generation to grid                       

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx

  • Added unique ID generation per grid instance using
    useColorPickerGridId hook
  • Changed grid role from grid to listbox for proper semantics
  • Added aria-activedescendant attribute to announce active color
  • Passed formatted color aria-labels and item IDs to
    ColorPickerItemComponent
  • Implemented index-based item IDs to avoid invalid DOM IDs from special
    color strings
+28/-2   
ColorPickerItemComponent.tsx
Add ARIA attributes and presentation role to items             

packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx

  • Changed list item role from implicit to explicit option role
  • Added aria-selected attribute to announce selection state
  • Added id prop for unique item identification
  • Added colorAriaLabel prop for formatted color names
  • Set Clickable role="presentation" to prevent nested role announcements
  • Updated aria-label on Clickable to use formatted color names
+11/-1   
Tests
ColorPicker.test.tsx
Update tests to use formatted color names                               

packages/core/src/components/ColorPicker/tests/ColorPicker.test.tsx

  • Added formatColorName utility function to convert color strings to
    title case
  • Updated all getByLabelText calls to use formatted color names
  • Updated all queryByLabelText calls to use formatted color names
  • Changed test expectations to match new aria-label format
+12/-10 
ColorPicker.interactions.ts
Update interaction tests with formatted names                       

packages/docs/src/pages/components/ColorPicker/ColorPicker.interactions.ts

  • Added formatColorName utility function for title case conversion
  • Updated clickOnColor function to use formatted color names in label
    lookup
  • Ensures Storybook interaction tests match new aria-label format
+5/-1     

VoiceOver was unable to announce color names when navigating the color picker.
Added aria-activedescendant, formatted aria-labels, and proper listbox/option
roles so screen readers announce color names, selection state, and position.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rivka-ungar rivka-ungar requested a review from a team as a code owner April 9, 2026 12:45
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 9, 2026

Code Review by Qodo

🐞 Bugs (2)   📘 Rule violations (1)   📎 Requirement gaps (0)   🎨 UX Issues (0)
🐞\ ≡ Correctness (1) ⚙ Maintainability (1) ⭐ New (1)
📘\ ⚙ Maintainability (1)

Grey Divider


Action required

1. Unlabeled active option🐞
Description
ColorPickerColorsGrid sets aria-activedescendant to an element id on the <li role="option">, but the
human-readable aria-label is applied to a nested <Clickable role="presentation"> instead. As a
result, the active descendant option itself has no accessible name, so screen readers may not
announce the active color during keyboard navigation.
Code

packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[R121-127]

    <li
+          id={id}
+          role="option"
+          aria-selected={isSelected}
      className={cx(styles.itemWrapper, {
        [styles.selectedColor]: isSelected,
        [styles.active]: isActive,
Evidence
The listbox uses aria-activedescendant to reference the active option by id, but the referenced
option element (<li role="option" id=...>) does not carry an aria-label/text; the label is instead
placed on a child Clickable marked role="presentation". In contrast, other listbox/option patterns
in the codebase put selection semantics and the accessible name on the option element itself (via
its own content).

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-151]
packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[120-137]
packages/core/src/components/ListItem/ListItem.tsx[125-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`aria-activedescendant` points to the `<li role="option">`, but the option has no accessible name because `aria-label` is currently on a nested `<Clickable role="presentation">`. This prevents reliable screen reader announcement of the active color.
### Issue Context
For the active-descendant listbox pattern, the element referenced by `aria-activedescendant` should be the one that exposes the accessible name.
### Fix Focus Areas
- packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[120-157]
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[133-160]
### Suggested fix
- Add `aria-label={colorAriaLabel}` to the `<li role="option">`.
- Remove `aria-label` from the nested `Clickable` (and keep it presentational/hidden, e.g. `aria-hidden`, if you still need to avoid double announcements).
- Consider moving the click handler to the `<li>` (or otherwise ensure activating the `role="option"` element triggers selection) to align behavior with the option semantics and avoid relying on a presentational child for interaction.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. New ARIA states untested📘
Description
The PR adds key accessibility attributes (aria-activedescendant on the listbox and aria-selected
on options) but the updated tests do not assert these behaviors/attributes. This weakens confidence
that screen reader announcements and selection-state semantics work as intended.
Code

packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[R121-135]

+        {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events -- keyboard handling is managed by the parent grid's useGridKeyboardNavigation */}
<li
+          id={id}
+          role="option"
+          aria-label={colorAriaLabel}
+          aria-selected={isSelected}
  className={cx(styles.itemWrapper, {
    [styles.selectedColor]: isSelected,
    [styles.active]: isActive,
    [styles.circle]: colorShape === "circle"
  })}
  data-testid={dataTestId || getTestId(ComponentDefaultTestId.COLOR_PICKER_ITEM, color)}
+          onClick={onClick}
+          onMouseDown={e => e.preventDefault()} // this is for quill to not lose the selection
>
Evidence
Compliance ID 6 requires tests to verify actual behavior and relevant accessibility attributes. The
component changes introduce aria-activedescendant and aria-selected, but the modified tests only
query by aria-label and assert callbacks/tooltip rendering, without asserting these new ARIA
attributes or keyboard-navigation announcement behavior.

CLAUDE.md
packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-142]
packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[121-135]
packages/core/src/components/ColorPicker/tests/ColorPicker.test.tsx[20-42]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New accessibility behavior was added (`role=listbox/option`, `aria-activedescendant`, `aria-selected`), but tests do not assert these attributes/behaviors.
## Issue Context
The PR’s goal is screen reader support during keyboard navigation and selection-state announcements; without assertions, regressions are likely.
## Fix Focus Areas
- packages/core/src/components/ColorPicker/__tests__/ColorPicker.test.tsx[20-42]
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-142]
- packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[121-135]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Invalid option IDs🐞
Description
ColorPickerColorsGrid builds option id values by concatenating the raw color string, which can
include spaces/characters (e.g. CSS rgb(0, 0, 0)) and produce invalid HTML IDs. This can break
aria-activedescendant targeting and prevent VoiceOver from announcing the active option during
keyboard navigation.
Code

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[R129-132]

+    const gridId = useColorPickerGridId(id);
+    const getColorItemId = (color: ColorPickerValueOnly) => (gridId ? `${gridId}-item-${color}` : undefined);
+    const activeDescendantId = activeIndex >= 0 ? getColorItemId(colorsToRender[activeIndex]) : undefined;
+
Evidence
aria-activedescendant relies on valid, unique option element IDs, but IDs are derived from color
directly; the component explicitly supports arbitrary CSS color strings, which commonly contain
spaces (e.g. rgb(0, 0, 0)).

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-132]
packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[140-151]
packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[89-91]
packages/core/src/components/ColorPicker/ColorPicker.types.ts[7-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getColorItemId()` builds DOM IDs from the raw `color` string (e.g. `...-item-rgb(0, 0, 0)`), which can contain spaces/special chars and become invalid, breaking `aria-activedescendant`.
### Issue Context
ColorPicker supports arbitrary CSS color strings (not only monday tokens), so this can occur in real usage when `forceUseRawColorList` is used with values like `rgb(...)`.
### Fix Focus Areas
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-160]
### Suggested approach
- Generate option IDs using a stable, safe suffix (preferably the `index`):
- `const getColorItemId = (index: number) => gridId ? `${gridId}-item-${index}` : undefined;`
- Pass `id={getColorItemId(index)}` into `ColorPickerItemComponent`
- Compute `activeDescendantId` using `activeIndex` directly: `activeIndex >= 0 ? getColorItemId(activeIndex) : undefined`
- Keep `key={color}` as-is to avoid React key churn.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. Grid id hook type mismatch 🐞
Description
useColorPickerGridId declares id as a required string, but VibeComponentProps.id is
optional, so callers naturally pass string | undefined. This weakens type-safety and makes the
hook signature misleading for future reuse/refactors.
Code

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[R13-21]

+let colorPickerGridIdCounter = 0;
+const generateColorPickerGridId = () => `color-picker-grid-${colorPickerGridIdCounter++}`;
+
+const useColorPickerGridId = (id: string) => {
+  const [gridId, setGridId] = useState<string>();
+  useIsomorphicLayoutEffect(() => {
+    setGridId(id || generateColorPickerGridId());
+  }, [id]);
+  return gridId;
Evidence
VibeComponentProps defines id?: string, and ColorPickerColorsGridProps extends it, so id can
be undefined. The new hook currently types the parameter as string, even though the
implementation explicitly supports falsy/undefined via id || generateColorPickerGridId().

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[13-22]
packages/core/src/types/VibeComponentProps.ts[1-14]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`useColorPickerGridId` accepts `id: string`, but the prop is optional (`id?: string`). The code handles undefined at runtime, but the type signature doesn’t reflect that.

### Issue Context
This is primarily a type-safety / maintainability concern: future refactors may rely on the incorrect signature.

### Fix Focus Areas
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[16-22]
- packages/core/src/types/VibeComponentProps.ts[1-14]

### What to change
- Update the hook signature to `const useColorPickerGridId = (id?: string) => { ... }`.
- (Optional improvement) Initialize state from `id` when provided to avoid an extra render:
 - e.g., `useState(() => id)` and only generate in the layout effect when `id` is not provided.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. ColorPickerItemComponentProps defined in .tsx 📘
Description
This PR updates ColorPickerItemComponentProps by adding colorAriaLabel, but the props interface
is declared in ColorPickerItemComponent.tsx instead of a dedicated *.types.ts file. This breaks
the repo typing standard and makes component API changes harder to track and reuse consistently.
Code

packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[R63-66]

+  /**
+   * Human-readable label for the color, used for screen reader announcements.
+   */
+  colorAriaLabel: string;
Evidence
Compliance requires component prop types to live in *.types.ts and extend VibeComponentProps.
The PR modifies the in-file props interface by adding colorAriaLabel within the .tsx
implementation file.

CLAUDE.md
packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[63-66]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ColorPickerItemComponentProps` was modified in the component implementation file (`.tsx`) by adding `colorAriaLabel`, but compliance requires component prop types to be defined in a dedicated `*.types.ts` file.
## Issue Context
Keeping prop types in `*.types.ts` standardizes APIs and supports shared typing across the library.
## Fix Focus Areas
- packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[63-86]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Missing multiselect listbox flag 🐞
Description
ColorPickerColorsGrid uses role="listbox" but never sets aria-multiselectable, even though
ColorPicker supports multi-selection via isMultiselect. In multiselect mode, screen readers may
interpret the listbox as single-select and announce selection state inconsistently.
Code

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[R134-142]

+      <ul
+        className={styles.colorsGrid}
+        ref={ref}
+        tabIndex={0}
+        id={id}
+        data-testid={dataTestId}
+        role="listbox"
+        aria-activedescendant={activeDescendantId}
+      >
Evidence
The component has explicit multiselect behavior (toggling items in an array) but the listbox element
does not declare it supports multiple selection.

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[134-142]
packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerContent.tsx[145-163]
packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerContent.tsx[172-189]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The grid is now a `role="listbox"`, but it doesn't declare multi-select capability via `aria-multiselectable`, even though ColorPicker supports multi-selection.
### Issue Context
`ColorPickerContent` toggles multiple colors when `isMultiselect` is enabled, so assistive tech should be told the listbox supports multiple selection.
### Fix Focus Areas
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerContent.tsx[172-189]
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[133-142]
### Suggested approach
- Thread `isMultiselect` down into `ColorPickerColorsGrid` (add to `ColorPickerColorsGridProps`, pass from `ColorPickerContent`).
- Set `aria-multiselectable={isMultiselect || undefined}` on the `<ul role="listbox">` so it is only present when needed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Grey Divider

Previous review results

Review updated until commit 649fdac

Results up to commit N/A


🐞 Bugs (1)  
📘 Rule violations (1)  
📎 Requirement gaps (0)  
🎨 UX Issues (0)

🐞\ ≡ Correctness (1)
📘\ ⚙ Maintainability (1)

Grey Divider


Action required

1. Unlabeled active option🐞
Description
ColorPickerColorsGrid sets aria-activedescendant to an element id on the <li role="option">, but the
human-readable aria-label is applied to a nested <Clickable role="presentation"> instead. As a
result, the active descendant option itself has no accessible name, so screen readers may not
announce the active color during keyboard navigation.
Code

packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[R121-127]

     <li
+          id={id}
+          role="option"
+          aria-selected={isSelected}
       className={cx(styles.itemWrapper, {
         [styles.selectedColor]: isSelected,
         [styles.active]: isActive,
Evidence
The listbox uses aria-activedescendant to reference the active option by id, but the referenced
option element (<li role="option" id=...>) does not carry an aria-label/text; the label is instead
placed on a child Clickable marked role="presentation". In contrast, other listbox/option patterns
in the codebase put selection semantics and the accessible name on the option element itself (via
its own content).

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-151]
packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[120-137]
packages/core/src/components/ListItem/ListItem.tsx[125-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`aria-activedescendant` points to the `<li role="option">`, but the option has no accessible name because `aria-label` is currently on a nested `<Clickable role="presentation">`. This prevents reliable screen reader announcement of the active color.
### Issue Context
For the active-descendant listbox pattern, the element referenced by `aria-activedescendant` should be the one that exposes the accessible name.
### Fix Focus Areas
- packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[120-157]
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[133-160]
### Suggested fix
- Add `aria-label={colorAriaLabel}` to the `<li role="option">`.
- Remove `aria-label` from the nested `Clickable` (and keep it presentational/hidden, e.g. `aria-hidden`, if you still need to avoid double announcements).
- Consider moving the click handler to the `<li>` (or otherwise ensure activating the `role="option"` element triggers selection) to align behavior with the option semantics and avoid relying on a presentational child for interaction.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. New ARIA states untested📘
Description
The PR adds key accessibility attributes (aria-activedescendant on the listbox and aria-selected
on options) but the updated tests do not assert these behaviors/attributes. This weakens confidence
that screen reader announcements and selection-state semantics work as intended.
Code

packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[R121-135]

+        {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events -- keyboard handling is managed by the parent grid's useGridKeyboardNavigation */}
 <li
+          id={id}
+          role="option"
+          aria-label={colorAriaLabel}
+          aria-selected={isSelected}
   className={cx(styles.itemWrapper, {
     [styles.selectedColor]: isSelected,
     [styles.active]: isActive,
     [styles.circle]: colorShape === "circle"
   })}
   data-testid={dataTestId || getTestId(ComponentDefaultTestId.COLOR_PICKER_ITEM, color)}
+          onClick={onClick}
+          onMouseDown={e => e.preventDefault()} // this is for quill to not lose the selection
 >
Evidence
Compliance ID 6 requires tests to verify actual behavior and relevant accessibility attributes. The
component changes introduce aria-activedescendant and aria-selected, but the modified tests only
query by aria-label and assert callbacks/tooltip rendering, without asserting these new ARIA
attributes or keyboard-navigation announcement behavior.

CLAUDE.md
packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-142]
packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[121-135]
packages/core/src/components/ColorPicker/tests/ColorPicker.test.tsx[20-42]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New accessibility behavior was added (`role=listbox/option`, `aria-activedescendant`, `aria-selected`), but tests do not assert these attributes/behaviors.
## Issue Context
The PR’s goal is screen reader support during keyboard navigation and selection-state announcements; without assertions, regressions are likely.
## Fix Focus Areas
- packages/core/src/components/ColorPicker/__tests__/ColorPicker.test.tsx[20-42]
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-142]
- packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[121-135]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Invalid option IDs🐞
Description
ColorPickerColorsGrid builds option id values by concatenating the raw color string, which can
include spaces/characters (e.g. CSS rgb(0, 0, 0)) and produce invalid HTML IDs. This can break
aria-activedescendant targeting and prevent VoiceOver from announcing the active option during
keyboard navigation.
Code

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[R129-132]

+    const gridId = useColorPickerGridId(id);
+    const getColorItemId = (color: ColorPickerValueOnly) => (gridId ? `${gridId}-item-${color}` : undefined);
+    const activeDescendantId = activeIndex >= 0 ? getColorItemId(colorsToRender[activeIndex]) : undefined;
+
Evidence
aria-activedescendant relies on valid, unique option element IDs, but IDs are derived from color
directly; the component explicitly supports arbitrary CSS color strings, which commonly contain
spaces (e.g. rgb(0, 0, 0)).

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-132]
packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[140-151]
packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[89-91]
packages/core/src/components/ColorPicker/ColorPicker.types.ts[7-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getColorItemId()` builds DOM IDs from the raw `color` string (e.g. `...-item-rgb(0, 0, 0)`), which can contain spaces/special chars and become invalid, breaking `aria-activedescendant`.
### Issue Context
ColorPicker supports arbitrary CSS color strings (not only monday tokens), so this can occur in real usage when `forceUseRawColorList` is used with values like `rgb(...)`.
### Fix Focus Areas
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[129-160]
### Suggested approach
- Generate option IDs using a stable, safe suffix (preferably the `index`):
- `const getColorItemId = (index: number) => gridId ? `${gridId}-item-${index}` : undefined;`
- Pass `id={getColorItemId(index)}` into `ColorPickerItemComponent`
- Compute `activeDescendantId` using `activeIndex` directly: `activeIndex >= 0 ? getColorItemId(activeIndex) : undefined`
- Keep `key={color}` as-is to avoid React key churn.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. ColorPickerItemComponentProps defined in .tsx 📘
Description
This PR updates ColorPickerItemComponentProps by adding colorAriaLabel, but the props interface
is declared in ColorPickerItemComponent.tsx instead of a dedicated *.types.ts file. This breaks
the repo typing standard and makes component API changes harder to track and reuse consistently.
Code

packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[R63-66]

+  /**
+   * Human-readable label for the color, used for screen reader announcements.
+   */
+  colorAriaLabel: string;
Evidence
Compliance requires component prop types to live in *.types.ts and extend VibeComponentProps.
The PR modifies the in-file props interface by adding colorAriaLabel within the .tsx
implementation file.

CLAUDE.md
packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[63-66]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ColorPickerItemComponentProps` was modified in the component implementation file (`.tsx`) by adding `colorAriaLabel`, but compliance requires component prop types to be defined in a dedicated `*.types.ts` file.
## Issue Context
Keeping prop types in `*.types.ts` standardizes APIs and supports shared typing across the library.
## Fix Focus Areas
- packages/core/src/components/ColorPicker/components/ColorPickerItemComponent/ColorPickerItemComponent.tsx[63-86]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Missing multiselect listbox flag 🐞
Description
ColorPickerColorsGrid uses role="listbox" but never sets aria-multiselectable, even though
ColorPicker supports multi-selection via isMultiselect. In multiselect mode, screen readers may
interpret the listbox as single-select and announce selection state inconsistently.
Code

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[R134-142]

+      <ul
+        className={styles.colorsGrid}
+        ref={ref}
+        tabIndex={0}
+        id={id}
+        data-testid={dataTestId}
+        role="listbox"
+        aria-activedescendant={activeDescendantId}
+      >
Evidence
The component has explicit multiselect behavior (toggling items in an array) but the listbox element
does not declare it supports multiple selection.

packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[134-142]
packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerContent.tsx[145-163]
packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerContent.tsx[172-189]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The grid is now a `role="listbox"`, but it doesn't declare multi-select capability via `aria-multiselectable`, even though ColorPicker supports multi-selection.
### Issue Context
`ColorPickerContent` toggles multiple colors when `isMultiselect` is enabled, so assistive tech should be told the listbox supports multiple selection.
### Fix Focus Areas
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerContent.tsx[172-189]
- packages/core/src/components/ColorPicker/components/ColorPickerContent/ColorPickerColorsGrid.tsx[133-142]
### Suggested approach
- Thread `isMultiselect` down into `ColorPickerColorsGrid` (add to `ColorPickerColorsGridProps`, pass from `ColorPickerContent`).
- Set `aria-multiselectable={isMultiselect || undefined}` on the `<ul role="listbox">` so it is only present when needed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Grey Divider

Qodo Logo

…pecial color strings

Color values like `rgb(0, 0, 0)` contain characters invalid in HTML IDs,
breaking aria-activedescendant. Use the color index instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 9, 2026

Code Review by Qodo

Grey Divider

New Review Started

This review has been superseded by a new analysis

Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

…avioral changes

Move aria-label back to Clickable (just with formatted name), keep onClick
and onMouseDown on Clickable as before. The <li> only gets additive a11y
attributes (role, id, aria-selected) with no behavioral changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 9, 2026

Code Review by Qodo

Grey Divider

New Review Started

This review has been superseded by a new analysis

Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

📦 Bundle Size Analysis

✅ No bundle size changes detected.

Unchanged Components
Component Base PR Diff
@vibe/button 17.3KB 17.27KB -37B 🟢
@vibe/clickable 5.97KB 5.96KB -8B 🟢
@vibe/dialog 52.17KB 52.13KB -40B 🟢
@vibe/icon-button 66.07KB 66.08KB +5B 🔺
@vibe/icon 12.92KB 12.93KB +6B 🔺
@vibe/layer 2.96KB 2.96KB 0B ➖
@vibe/layout 9.83KB 9.83KB 0B ➖
@vibe/loader 5.64KB 5.66KB +19B 🔺
@vibe/tooltip 61.31KB 61.31KB -3B 🟢
@vibe/typography 63.41KB 63.48KB +73B 🔺
Accordion 6.3KB 6.31KB +8B 🔺
AccordionItem 66.47KB 66.44KB -24B 🟢
AlertBanner 70.81KB 70.74KB -64B 🟢
AlertBannerButton 18.76KB 18.79KB +30B 🔺
AlertBannerLink 15.23KB 15.21KB -16B 🟢
AlertBannerText 63.91KB 63.92KB +7B 🔺
AttentionBox 74.28KB 74.29KB +13B 🔺
Avatar 66.68KB 66.73KB +52B 🔺
AvatarGroup 93.25KB 93.2KB -54B 🟢
Badge 43.2KB 43.18KB -12B 🟢
BreadcrumbItem 64.66KB 64.61KB -54B 🟢
BreadcrumbMenu 68.58KB 68.57KB -7B 🟢
BreadcrumbMenuItem 77.07KB 76.99KB -82B 🟢
BreadcrumbsBar 5.66KB 5.68KB +16B 🔺
ButtonGroup 68.23KB 68.27KB +43B 🔺
Checkbox 66.84KB 66.87KB +33B 🔺
Chips 75.02KB 74.93KB -88B 🟢
ColorPicker 74.3KB 74.41KB +108B 🔺
ColorPickerContent 73.57KB 73.8KB +240B 🔺
Combobox 84.01KB 84KB -7B 🟢
Counter 42.2KB 42.23KB +36B 🔺
DatePicker 112.39KB 112.27KB -124B 🟢
Divider 5.45KB 5.46KB +8B 🔺
Dropdown 95.23KB 95.08KB -149B 🟢
EditableHeading 66.43KB 66.41KB -20B 🟢
EditableText 66.34KB 66.31KB -29B 🟢
EmptyState 70.35KB 70.3KB -57B 🟢
ExpandCollapse 66.18KB 66.17KB -17B 🟢
FormattedNumber 5.81KB 5.82KB +11B 🔺
GridKeyboardNavigationContext 4.65KB 4.65KB -4B 🟢
HiddenText 5.41KB 5.39KB -18B 🟢
Info 72.01KB 71.98KB -33B 🟢
Label 68.57KB 68.63KB +54B 🔺
Link 14.9KB 14.89KB -14B 🟢
List 72.85KB 72.84KB -7B 🟢
ListItem 65.53KB 65.48KB -59B 🟢
ListItemAvatar 66.93KB 66.9KB -29B 🟢
ListItemIcon 14KB 13.97KB -31B 🟢
ListTitle 64.92KB 64.96KB +46B 🔺
Menu 8.67KB 8.63KB -41B 🟢
MenuDivider 5.55KB 5.59KB +43B 🔺
MenuGridItem 7.16KB 7.19KB +36B 🔺
MenuItem 76.96KB 76.8KB -163B 🟢
MenuItemButton 70.09KB 69.99KB -100B 🟢
MenuTitle 65.32KB 65.3KB -19B 🟢
MenuButton 66.07KB 66.09KB +18B 🔺
Modal 79.05KB 79.02KB -32B 🟢
ModalContent 4.72KB 4.71KB -1B 🟢
ModalHeader 65.85KB 65.84KB -10B 🟢
ModalMedia 7.51KB 7.49KB -18B 🟢
ModalFooter 67.65KB 67.64KB -11B 🟢
ModalFooterWizard 68.65KB 68.54KB -119B 🟢
ModalBasicLayout 8.92KB 8.91KB -10B 🟢
ModalMediaLayout 8.06KB 8.08KB +17B 🔺
ModalSideBySideLayout 6.3KB 6.31KB +8B 🔺
MultiStepIndicator 52.97KB 52.93KB -40B 🟢
NumberField 72.77KB 72.78KB +10B 🔺
ProgressBar 7.36KB 7.34KB -15B 🟢
RadioButton 65.87KB 65.88KB +10B 🔺
Search 70.59KB 70.57KB -21B 🟢
Skeleton 6KB 5.98KB -15B 🟢
Slider 73.91KB 73.85KB -59B 🟢
SplitButton 66.42KB 66.56KB +141B 🔺
SplitButtonMenu 8.79KB 8.8KB +16B 🔺
Steps 71.28KB 71.32KB +36B 🔺
Table 7.25KB 7.28KB +31B 🔺
TableBody 66.73KB 66.69KB -47B 🟢
TableCell 65.23KB 65.2KB -33B 🟢
TableContainer 5.31KB 5.31KB +8B 🔺
TableHeader 5.65KB 5.66KB +10B 🔺
TableHeaderCell 72.19KB 72.18KB -11B 🟢
TableRow 5.56KB 5.56KB 0B ➖
TableRowMenu 68.79KB 68.84KB +55B 🔺
TableVirtualizedBody 71.44KB 71.35KB -86B 🟢
Tab 63.93KB 63.96KB +24B 🔺
TabList 8.86KB 8.86KB -6B 🟢
TabPanel 5.32KB 5.29KB -34B 🟢
TabPanels 5.87KB 5.84KB -35B 🟢
TabsContext 5.48KB 5.51KB +29B 🔺
TextArea 66.29KB 66.37KB +79B 🔺
TextField 69.38KB 69.43KB +52B 🔺
TextWithHighlight 64.34KB 64.25KB -101B 🟢
ThemeProvider 4.36KB 4.36KB -1B 🟢
Tipseen 71.12KB 71.11KB -14B 🟢
TipseenContent 71.58KB 71.65KB +76B 🔺
TipseenMedia 71.29KB 71.29KB 0B ➖
TipseenWizard 73.78KB 73.86KB +81B 🔺
Toast 73.97KB 73.97KB -5B 🟢
ToastButton 18.64KB 18.6KB -34B 🟢
ToastLink 15.08KB 15.07KB -8B 🟢
Toggle 66.59KB 66.61KB +14B 🔺
TransitionView 5.45KB 5.43KB -18B 🟢
VirtualizedGrid 12.57KB 12.53KB -36B 🟢
VirtualizedList 12.31KB 12.25KB -57B 🟢
List (Next) 8.15KB 8.17KB +17B 🔺
ListItem (Next) 69.9KB 69.92KB +19B 🔺
ListTitle (Next) 65.31KB 65.29KB -16B 🟢

📊 Summary:

  • Total Base Size: 4.75MB
  • Total PR Size: 4.75MB
  • Total Difference: 665B

…" VoiceOver announcement

The default role=button on Clickable inside role=option caused VoiceOver
to announce "group". Setting role=presentation removes the nested
interactive semantics without changing click behavior.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 9, 2026

Persistent review updated to latest commit a9d3248

The aria-label now uses formatted names (e.g. "Bright Green" instead of
"bright-green"), so the Storybook interaction tests need to match.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 9, 2026

Persistent review updated to latest commit fe40661

import { type ColorStyle } from "../../../../types";

let colorPickerGridIdCounter = 0;
const generateColorPickerGridId = () => `color-picker-grid-${colorPickerGridIdCounter++}`;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will 2 pickers in the same page have same id?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The counter is global, same as we have with list

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 9, 2026

Persistent review updated to latest commit 649fdac

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

A new prerelease version of this PR has been published! 🎉
To use this prerelease version, install the needed packages in your project:

@vibe/base@4.0.1-alpha-649fd.0
@vibe/button@4.0.1-alpha-649fd.0
@vibe/clickable@4.0.1-alpha-649fd.0
@vibe/dialog@4.0.1-alpha-649fd.0
@vibe/icon@4.0.1-alpha-649fd.0
@vibe/icon-button@4.0.1-alpha-649fd.0
@vibe/layer@4.0.1-alpha-649fd.0
@vibe/layout@4.0.1-alpha-649fd.0
@vibe/loader@4.0.1-alpha-649fd.0
@vibe/tooltip@4.0.1-alpha-649fd.0
@vibe/typography@4.0.1-alpha-649fd.0
@vibe/core@4.0.1-alpha-649fd.0
@vibe/docs@4.0.1-alpha-649fd.0
@vibe/hooks@4.0.1-alpha-649fd.0
@vibe/shared@4.0.1-alpha-649fd.0

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

A new prerelease version of this PR has been published! 🎉
To use this prerelease version, install the needed packages in your project:

@vibe/base@4.0.1-alpha-649fd.0
@vibe/button@4.0.1-alpha-649fd.0
@vibe/clickable@4.0.1-alpha-649fd.0
@vibe/dialog@4.0.1-alpha-649fd.0
@vibe/icon@4.0.1-alpha-649fd.0
@vibe/icon-button@4.0.1-alpha-649fd.0
@vibe/layer@4.0.1-alpha-649fd.0
@vibe/layout@4.0.1-alpha-649fd.0
@vibe/loader@4.0.1-alpha-649fd.0
@vibe/tooltip@4.0.1-alpha-649fd.0
@vibe/typography@4.0.1-alpha-649fd.0
@vibe/core@4.0.1-alpha-649fd.0
@vibe/docs@4.0.1-alpha-649fd.0
@vibe/hooks@4.0.1-alpha-649fd.0
@vibe/shared@4.0.1-alpha-649fd.0

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.

2 participants