From 33e0dc4c332c263139c609949660a2a78b1938c2 Mon Sep 17 00:00:00 2001 From: Fred Visser <1458528+fredvisser@users.noreply.github.com> Date: Tue, 25 Nov 2025 09:41:44 -0600 Subject: [PATCH 1/8] initial commit --- packages/nimble-components/src/chip/index.ts | 176 +++++++++++- .../src/chip/specs/README.md | 10 +- .../src/chip/specs/selection-toggle.md | 117 ++++++++ packages/nimble-components/src/chip/styles.ts | 164 ++++++++--- .../nimble-components/src/chip/template.ts | 60 ++-- .../src/chip/testing/chip.pageobject.ts | 11 + .../src/chip/tests/chip.spec.ts | 261 +++++++++++++++++- packages/nimble-components/src/chip/types.ts | 7 + packages/storybook/src/nimble/chip/chip.mdx | 12 +- .../storybook/src/nimble/chip/chip.stories.ts | 35 ++- 10 files changed, 769 insertions(+), 84 deletions(-) create mode 100644 packages/nimble-components/src/chip/specs/selection-toggle.md diff --git a/packages/nimble-components/src/chip/index.ts b/packages/nimble-components/src/chip/index.ts index e0a189c452..7429af5c92 100644 --- a/packages/nimble-components/src/chip/index.ts +++ b/packages/nimble-components/src/chip/index.ts @@ -1,4 +1,5 @@ import { attr, nullableNumberConverter, observable } from '@ni/fast-element'; +import { keyEnter, keyEscape, keySpace } from '@ni/fast-web-utilities'; import { applyMixins, DesignSystem, @@ -10,7 +11,7 @@ import { } from '@ni/fast-foundation'; import { styles } from './styles'; import { template } from './template'; -import { ChipAppearance } from './types'; +import { ChipAppearance, ChipSelectionMode } from './types'; import { slotTextContent } from '../utilities/models/slot-text-content'; import { itemRemoveLabel } from '../label-provider/core/label-tokens'; @@ -19,6 +20,10 @@ declare global { 'nimble-chip': Chip; } } +export { + ChipSelectionMode, + type ChipSelectionMode as ChipSelectionModeType +} from './types'; export type ChipOptions = FoundationElementDefinition & StartOptions & @@ -34,12 +39,29 @@ export class Chip extends FoundationElement { @attr({ mode: 'boolean' }) public disabled = false; + @attr({ attribute: 'selection-mode' }) + public selectionMode: ChipSelectionMode; + + @attr({ mode: 'boolean' }) + public selected = false; + @attr() public appearance: ChipAppearance = ChipAppearance.outline; @attr({ attribute: 'tabindex', converter: nullableNumberConverter }) public override tabIndex!: number; + /** + * Indicates whether the remove button is currently in a mousedown state. + * Used to prevent the chip's active styling from showing when the remove button is being clicked. + * + * @internal + * @remarks + * This attribute is automatically managed by handleRemoveMousedown and should not be set directly. + */ + @attr({ attribute: 'remove-button-active', mode: 'boolean' }) + public removeButtonActive = false; + /** @internal */ public readonly content?: HTMLElement[]; @@ -60,22 +82,164 @@ export class Chip extends FoundationElement { /** @internal */ public contentSlot!: HTMLSlotElement; + private managingTabIndex = false; + private suppressTabIndexChanged = false; + private mouseUpHandler: EventListener | null = null; + + public override connectedCallback(): void { + super.connectedCallback(); + this.updateManagedTabIndex(); + } + + public override disconnectedCallback(): void { + super.disconnectedCallback(); + if (this.mouseUpHandler) { + document.removeEventListener('mouseup', this.mouseUpHandler); + this.mouseUpHandler = null; + } + } + /** @internal */ - public handleRemoveClick(): void { + public clickHandler(_e: MouseEvent): boolean { + if (this.disabled) { + return false; + } + + if (this.selectionMode === ChipSelectionMode.single) { + this.selected = !this.selected; + this.$emit('selected-change'); + return false; + } + return true; + } + + /** @internal */ + public keyupHandler(e: KeyboardEvent): boolean { + if (this.disabled) { + return false; + } + switch (e.key) { + case keySpace: + case keyEnter: + if (this.selectionMode === ChipSelectionMode.single) { + this.selected = !this.selected; + this.$emit('selected-change'); + } + return true; + case keyEscape: + if ( + this.removable + && this.selectionMode === ChipSelectionMode.single + ) { + this.$emit('remove'); + return false; + } + return true; + default: + return true; + } + } + + /** @internal */ + public handleRemoveClick(event: MouseEvent): void { + event.stopPropagation(); if (this.removable) { this.$emit('remove'); } } + + /** + * Handles mousedown events on the remove button. + * Sets removeButtonActive to true and registers a document-level mouseup handler to clear it. + * + * @internal + * @remarks + * The mouseup listener is added to document to ensure it fires even if the mouse moves + * outside the chip before being released. The listener is cleaned up in disconnectedCallback + * to prevent memory leaks. + */ + public handleRemoveMousedown(event: MouseEvent): void { + event.stopPropagation(); + this.removeButtonActive = true; + + // Clean up any existing listener first + if (this.mouseUpHandler) { + document.removeEventListener('mouseup', this.mouseUpHandler); + } + + this.mouseUpHandler = (): void => { + this.removeButtonActive = false; + if (this.mouseUpHandler) { + document.removeEventListener('mouseup', this.mouseUpHandler); + this.mouseUpHandler = null; + } + }; + document.addEventListener('mouseup', this.mouseUpHandler); + } + + /** @internal */ + public handleRemoveKeyup(event: KeyboardEvent): void { + event.stopPropagation(); + } + + protected selectionModeChanged( + _oldValue: ChipSelectionMode | undefined, + _newValue: ChipSelectionMode | undefined + ): void { + this.updateManagedTabIndex(); + } + + protected disabledChanged(_oldValue: boolean, _newValue: boolean): void { + this.updateManagedTabIndex(); + } + + protected tabIndexChanged(): void { + if (this.suppressTabIndexChanged) { + this.suppressTabIndexChanged = false; + return; + } + + this.managingTabIndex = false; + } + + private updateManagedTabIndex(): void { + if (!this.$fastController?.isConnected) { + return; + } + + const shouldManage = this.selectionMode === ChipSelectionMode.single && !this.disabled; + + if (shouldManage) { + if (!this.hasAttribute('tabindex')) { + this.setManagedTabIndex(0); + } + } else { + this.removeManagedTabIndex(); + } + } + + private setManagedTabIndex(value: number): void { + this.managingTabIndex = true; + this.suppressTabIndexChanged = true; + this.tabIndex = value; + } + + private removeManagedTabIndex(): void { + if (!this.managingTabIndex) { + return; + } + + this.managingTabIndex = false; + this.suppressTabIndexChanged = true; + this.removeAttribute('tabindex'); + } } applyMixins(Chip, StartEnd); const nimbleChip = Chip.compose({ baseName: 'chip', template, - styles, - shadowOptions: { - delegatesFocus: true - } + styles }); DesignSystem.getOrCreate().withPrefix('nimble').register(nimbleChip()); diff --git a/packages/nimble-components/src/chip/specs/README.md b/packages/nimble-components/src/chip/specs/README.md index 7a5a48abd4..91b4b1b9c7 100644 --- a/packages/nimble-components/src/chip/specs/README.md +++ b/packages/nimble-components/src/chip/specs/README.md @@ -145,12 +145,18 @@ We will provide styling for the `disabled` attribute state. _Consider the accessibility of the component, including:_ - _Keyboard Navigation and Focus_ - - when the chip component is removable, the remove button will be focusable, otherwise it will not receive focus (following the `nimble-banner` pattern). + - When the chip is selectable (`selection-mode="single"`) and removable: + - The chip itself is focusable and receives keyboard events + - Space/Enter toggles the selected state + - Escape removes the chip (emits `remove` event) + - The remove button is **not** focusable (`tabindex="-1"`) to avoid nested interactive controls (violates [WCAG 4.1.2](https://dequeuniversity.com/rules/axe/4.11/nested-interactive)) + - When the chip is removable but not selectable (`selection-mode="none"`): + - The remove button is focusable and can be activated with Space or Enter - _Form Input_ - N/A - _Use with Assistive Technology_ + - When selectable, the chip has `role="button"` and `aria-pressed` to indicate its toggle state - a `chip`'s accessible name comes from the element's contents by default - - no ARIA `role` seems necessary to define for the chip, as it isn't interactive itself (only the remove button is which has a `role`). The only valid role seemed to be [`status`](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/status_role), but that also didn't seem helpful from an accessibility perspective, particularly since it mainly relates to providing helpful information when the content changes (which we don't expect). - the remove button will have its content set to provide a label provider token for "Remove". - title will not be set, which aligns with decisions for the filterable select clear button and the banner - ideally this would include the contents of the chip itself (so a screen reader would announce "Remove ") but differing word order between diff --git a/packages/nimble-components/src/chip/specs/selection-toggle.md b/packages/nimble-components/src/chip/specs/selection-toggle.md new file mode 100644 index 0000000000..79e7459e2e --- /dev/null +++ b/packages/nimble-components/src/chip/specs/selection-toggle.md @@ -0,0 +1,117 @@ +# Chip Selection Toggle + +## Problem Statement + +The `nimble-chip` component currently lacks a built-in mechanism to represent a selected or toggled state. This feature is required to support use cases where chips act as filter toggles or selectable items in a list. This design proposes adding a selection state to the `nimble-chip`. + +## Links To Relevant Work Items and Reference Material + +* [Nimble Chip Component](../index.ts) +* [Figma Design - Chip Interactive States](https://www.figma.com/design/PO9mFOu5BCl8aJvFchEeuN/Nimble_Components?node-id=2227-78839&m=dev) +* [FAST Toggle Button](https://explore.fast.design/components/toggle-button) +* [FAST Checkbox](https://explore.fast.design/components/checkbox) +* [ARIA: button role](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role) (specifically `aria-pressed`) +* [WCAG 4.1.2: Name, Role, Value](https://www.w3.org/WAI/WCAG21/Understanding/name-role-value.html) (nested interactive controls) + +## Implementation / Design + +### API Proposal + +The chip uses the `selection-mode` attribute to control whether the chip is selectable. + +* **Attribute:** `selection-mode` (enum: `none` (default: `undefined`) | `single` (`'single'`)) + * `none`: The chip is not selectable. Does not have `role="button"` or `aria-pressed`. User-supplied `tabindex` is forwarded to the remove button if removable. + * `single`: The chip can be toggled on/off via click or Space/Enter keys. Has `role="button"` and `aria-pressed`. Automatically receives `tabindex="0"` unless user provides a different value. +* **Attribute:** `selected` (boolean, default: `false`) + * Indicates the current selection state when `selection-mode="single"`. + * When `true`, the chip displays with `fillSelectedColor` background. +* **Event:** `selected-change` + * Emitted when the user toggles the chip state via click or keyboard (Space/Enter). + * Only emitted when `selection-mode="single"`. +* **Event:** `remove` + * Emitted when the user activates the remove button (click) or presses Escape key (when selectable and removable). + +### Keyboard Interaction + +* **Space/Enter:** Toggles `selected` state (when `selection-mode="single"`). +* **Escape:** Removes the chip (when `selection-mode="single"`, `removable`, and not `disabled`). + * This provides keyboard access to the remove functionality without requiring the remove button to be focusable, which would violate WCAG 4.1.2 (nested interactive controls). + +### Accessibility + +* **Role:** + * If `selection-mode="none"`: No explicit role (generic container). + * If `selection-mode="single"`: `role="button"` with `aria-pressed="true"` or `aria-pressed="false"`. +* **Tabindex Management:** + * When `selection-mode="single"`: The chip automatically manages its own `tabindex` (defaults to `0`). User-supplied values are preserved. + * When `selection-mode="none"`: The chip does not manage `tabindex`. User-supplied values are forwarded to the remove button if `removable`. +* **Nested Interactive Controls:** To comply with WCAG 4.1.2, when a chip is both selectable and removable, the remove button is set to `tabindex="-1"` (not keyboard-focusable) and the Escape key provides the keyboard mechanism for removal. + +### Visual Design + +Visual states follow the [Figma design specification](https://www.figma.com/design/PO9mFOu5BCl8aJvFchEeuN/Nimble_Components?node-id=2227-78839&m=dev). + +* **Default State:** + * Border: `rgba(actionRgbPartialColor, 0.3)` for selectable chips; `rgba(borderRgbPartialColor, 0.3)` for non-selectable, non-block appearance + * Background: transparent + * Cursor: pointer (when `selection-mode="single"`) +* **Selected State:** + * Background: `fillSelectedColor` + * Border: `rgba(actionRgbPartialColor, 0.3)` +* **Hover State** (selectable chips only): + * Border: `borderHoverColor` (2px green) + * Outline: 2px green (`borderHoverColor`) at -2px offset (creates 2px green outline appearance) +* **Focus-Visible State** (selectable chips only): + * 3-ring effect using layered box-shadows: + * Outer: 2px green border (`borderHoverColor`) + * Middle: 2px white ring (`applicationBackgroundColor` inset) + * Inner: 1px green ring (`borderHoverColor` inset) +* **Active State** (selectable chips only, during mousedown/click): + * Background: `rgba(fillSelectedRgbPartialColor, 0.3)` (30% opacity green) + * Border: `borderHoverColor` (green) + * Outline: 1px green at -1px offset + * Box-shadow: 1px white ring inset + * **Note:** Active styling is suppressed when the remove button is in mousedown state (via `remove-button-active` attribute) +* **Disabled State:** + * Text color: `bodyDisabledFontColor` + * Icons: 30% opacity + * No hover, focus, or active styling + * Remove button hidden + +### Implementation Details + +* **CSS Cascade Layers:** Styles are organized using `@layer base, hover, focusVisible, active, disabled, top` to ensure proper precedence of interactive states. +* **Tabindex Management:** + * The chip tracks whether it's managing tabindex via internal `managingTabIndex` flag. + * When `selection-mode` changes or the chip connects/disconnects, `updateManagedTabIndex()` is called. + * User-supplied tabindex values are preserved by detecting attribute changes that didn't originate from internal management. +* **Remove Button Active State:** + * The `remove-button-active` attribute is set during remove button mousedown to prevent chip active styling from appearing. + * A document-level mouseup listener clears this state, ensuring it works even if the mouse moves outside the chip. + * The listener is cleaned up in `disconnectedCallback()` to prevent memory leaks. + +## Alternative Implementations / Designs + +### Alternative 1: New Component +* Create a `nimble-toggle-chip` instead of modifying `nimble-chip`. + * **Pros:** Separation of concerns. `nimble-chip` stays simple (just for display/dismiss). + * **Cons:** Code duplication. Users might expect `nimble-chip` to handle this common case. + * **Decision:** Rejected. The `selection-mode` attribute provides a clean API for opt-in behavior without requiring a separate component. + +### Alternative 2: Focusable Remove Button (Initial Implementation) +* Make the remove button keyboard-focusable when the chip is selectable and removable. + * **Pros:** Direct keyboard access to remove button matches mouse interaction. + * **Cons:** Violates WCAG 4.1.2 (nested interactive controls - a button within a button). + * **Decision:** Rejected. Implemented Escape key pattern instead to maintain accessibility compliance. + +### Alternative 3: Use AbortController for Event Cleanup +* Use `AbortController` to manage the document mouseup listener instead of storing the handler. + * **Pros:** Modern JavaScript pattern, automatic cleanup. + * **Cons:** No precedent in Nimble codebase for this pattern. + * **Decision:** Rejected. Followed established Nimble pattern of storing handler and cleaning up in `disconnectedCallback()`. + +## Open Issues + +### Resolved +* ~~Does this affect the `remove` functionality? Can a chip be both selectable and removable?~~ + * **Resolution:** Yes, chips can be both selectable and removable. When both are true, the chip uses the Escape key for keyboard removal to avoid nested interactive controls (WCAG 4.1.2 violation). diff --git a/packages/nimble-components/src/chip/styles.ts b/packages/nimble-components/src/chip/styles.ts index ffb5f2464b..70ebe3baad 100644 --- a/packages/nimble-components/src/chip/styles.ts +++ b/packages/nimble-components/src/chip/styles.ts @@ -1,82 +1,162 @@ import { css } from '@ni/fast-element'; import { actionRgbPartialColor, + applicationBackgroundColor, bodyDisabledFontColor, bodyFont, bodyFontColor, + borderHoverColor, borderRgbPartialColor, borderWidth, controlHeight, + fillSelectedColor, + fillSelectedRgbPartialColor, iconColor, iconSize, mediumPadding, smallPadding } from '../theme-provider/design-tokens'; import { display } from '../utilities/style/display'; +import { focusVisible } from '../utilities/style/focus'; import { appearanceBehavior } from '../utilities/style/appearance'; import { ChipAppearance } from './types'; export const styles = css` ${display('inline-flex')} - :host { - height: ${controlHeight}; - width: fit-content; - max-width: 300px; - color: ${bodyFontColor}; - font: ${bodyFont}; - padding: 0 ${mediumPadding}; - gap: 4px; - background-color: transparent; - border: ${borderWidth} solid rgba(${actionRgbPartialColor}, 0.3); - border-radius: 4px; - justify-content: center; - align-items: center; - } + @layer base, hover, focusVisible, active, disabled, top; - :host([disabled]) { - cursor: default; - color: ${bodyDisabledFontColor}; - border-color: rgba(${borderRgbPartialColor}, 0.3); - } + @layer base { + :host { + height: ${controlHeight}; + width: fit-content; + max-width: 300px; + color: ${bodyFontColor}; + font: ${bodyFont}; + padding: 0 ${mediumPadding}; + gap: 4px; + background-color: transparent; + border: ${borderWidth} solid rgba(${actionRgbPartialColor}, 0.3); + border-radius: 4px; + justify-content: center; + align-items: center; + box-shadow: none; + outline: none; + outline-offset: 0; + } - :host([disabled]) slot[name='start']::slotted(*) { - opacity: 0.3; - ${iconColor.cssCustomProperty}: ${bodyFontColor}; - } + :host([selection-mode='single']) { + cursor: pointer; + } + + :host( + [selection-mode='single']:not([selected]):not([appearance='block']) + ) { + border-color: rgba(${borderRgbPartialColor}, 0.3); + } + + :host([selected]) { + background-color: ${fillSelectedColor}; + border-color: rgba(${actionRgbPartialColor}, 0.3); + } + + slot[name='start']::slotted(*) { + flex-shrink: 0; + } + + [part='start'] { + display: contents; + ${iconColor.cssCustomProperty}: ${bodyFontColor}; + } - slot[name='start']::slotted(*) { - flex-shrink: 0; + .content { + text-overflow: ellipsis; + overflow: hidden; + white-space: nowrap; + } + + .remove-button { + height: ${iconSize}; + width: ${iconSize}; + margin-right: calc(-1 * ${smallPadding}); + } + + [part='end'] { + display: none; + } } - [part='start'] { - display: contents; - ${iconColor.cssCustomProperty}: ${bodyFontColor}; + @layer hover { + :host([selection-mode='single']:hover:not([disabled])) { + border-color: ${borderHoverColor}; + outline: calc(${borderWidth} * 2) solid ${borderHoverColor}; + outline-offset: calc(-2 * ${borderWidth}); + box-shadow: none; + } } - .content { - text-overflow: ellipsis; - overflow: hidden; - white-space: nowrap; + @layer focusVisible { + :host([selection-mode='single']${focusVisible}:not([disabled])) { + border-color: ${borderHoverColor}; + outline: calc(${borderWidth} * 2) solid ${borderHoverColor}; + outline-offset: calc(-2 * ${borderWidth}); + box-shadow: + 0 0 0 calc(${borderWidth} * 2) ${applicationBackgroundColor} + inset, + 0 0 0 calc(${borderWidth} * 3) ${borderHoverColor} inset; + } } - .remove-button { - height: ${iconSize}; - width: ${iconSize}; - margin-right: calc(-1 * ${smallPadding}); + @layer active { + :host( + [selection-mode='single']:active:not([disabled]):not( + [remove-button-active] + ) + ) { + background-color: rgba(${fillSelectedRgbPartialColor}, 0.3); + border-color: ${borderHoverColor}; + outline: ${borderWidth} solid ${borderHoverColor}; + outline-offset: calc(-1 * ${borderWidth}); + box-shadow: 0 0 0 ${borderWidth} ${applicationBackgroundColor} inset; + } } - [part='end'] { - display: none; + @layer disabled { + :host([disabled]) { + cursor: default; + color: ${bodyDisabledFontColor}; + box-shadow: none; + } + + :host([disabled]:not([appearance='block'])) { + border-color: rgba(${borderRgbPartialColor}, 0.3); + background-color: transparent; + } + + :host([disabled][appearance='block']) { + border-color: transparent; + } + + :host([disabled]) slot[name='start']::slotted(*) { + opacity: 0.3; + ${iconColor.cssCustomProperty}: ${bodyFontColor}; + } } `.withBehaviors( appearanceBehavior( ChipAppearance.block, css` - :host, - :host([disabled]) { - background-color: rgba(${borderRgbPartialColor}, 0.1); - border-color: transparent; + @layer base { + :host { + background-color: rgba(${borderRgbPartialColor}, 0.1); + border-color: transparent; + } + } + + @layer disabled { + :host([disabled]) { + background-color: rgba(${borderRgbPartialColor}, 0.1); + } } ` ) diff --git a/packages/nimble-components/src/chip/template.ts b/packages/nimble-components/src/chip/template.ts index 4bb1683a04..f0803395e2 100644 --- a/packages/nimble-components/src/chip/template.ts +++ b/packages/nimble-components/src/chip/template.ts @@ -9,37 +9,47 @@ import { overflow } from '../utilities/directive/overflow'; import { buttonTag } from '../button'; import { iconTimesTag } from '../icons/times'; import { ButtonAppearance } from '../button/types'; +import { ChipSelectionMode } from './types'; // prettier-ignore export const template: FoundationElementTemplate< ViewTemplate, ChipOptions > = (context, definition) => html` - ${startSlotTemplate(context, definition)} - (x.hasOverflow && x.elementTextContent - ? x.elementTextContent - : null)} + `; diff --git a/packages/nimble-components/src/chip/testing/chip.pageobject.ts b/packages/nimble-components/src/chip/testing/chip.pageobject.ts index 5b0fd9528c..f1745838c2 100644 --- a/packages/nimble-components/src/chip/testing/chip.pageobject.ts +++ b/packages/nimble-components/src/chip/testing/chip.pageobject.ts @@ -35,6 +35,17 @@ export class ChipPageObject { throw new Error('Remove button not found'); } + public mousedownRemoveButton(): void { + const removeButton = this.getRemoveButton(); + if (removeButton) { + removeButton.dispatchEvent( + new MouseEvent('mousedown', { bubbles: true }) + ); + } else { + throw new Error('Remove button not found'); + } + } + private getRemoveButton(): Button | null { return ( this.chipElement.shadowRoot?.querySelector