diff --git a/change/@ni-nimble-components-83f228b7-a387-4f1a-9bfe-2527beb62054.json b/change/@ni-nimble-components-83f228b7-a387-4f1a-9bfe-2527beb62054.json new file mode 100644 index 0000000000..62226a62f1 --- /dev/null +++ b/change/@ni-nimble-components-83f228b7-a387-4f1a-9bfe-2527beb62054.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Adds support for selectable nimble-chip behavior and selected state.", + "packageName": "@ni/nimble-components", + "email": "1458528+fredvisser@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/angular-workspace/example-client-app/src/app/customapp/chip-section.component.ts b/packages/angular-workspace/example-client-app/src/app/customapp/chip-section.component.ts index 9a6ba9ab5f..a7b706b865 100644 --- a/packages/angular-workspace/example-client-app/src/app/customapp/chip-section.component.ts +++ b/packages/angular-workspace/example-client-app/src/app/customapp/chip-section.component.ts @@ -7,6 +7,9 @@ import { Component } from '@angular/core'; Outline Chip Block Chip + + Selectable Chip + Removable Chip Disabled Chip @@ -14,6 +17,12 @@ import { Component } from '@angular/core'; standalone: false }) export class ChipSectionComponent { + public chipSelected = false; + + public onChipSelectedChange(): void { + this.chipSelected = !this.chipSelected; + } + public onChipRemove(): void { alert('Chip removed'); } diff --git a/packages/angular-workspace/nimble-angular/chip/nimble-chip.directive.ts b/packages/angular-workspace/nimble-angular/chip/nimble-chip.directive.ts index 9c16fd4efc..3ae2fa1da0 100644 --- a/packages/angular-workspace/nimble-angular/chip/nimble-chip.directive.ts +++ b/packages/angular-workspace/nimble-angular/chip/nimble-chip.directive.ts @@ -31,6 +31,22 @@ export class NimbleChipDirective { this.renderer.setProperty(this.elementRef.nativeElement, 'disabled', toBooleanProperty(value)); } + public get selectable(): boolean { + return this.elementRef.nativeElement.selectable; + } + + @Input() public set selectable(value: BooleanValueOrAttribute) { + this.renderer.setProperty(this.elementRef.nativeElement, 'selectable', toBooleanProperty(value)); + } + + public get selected(): boolean { + return this.elementRef.nativeElement.selected; + } + + @Input() public set selected(value: BooleanValueOrAttribute) { + this.renderer.setProperty(this.elementRef.nativeElement, 'selected', toBooleanProperty(value)); + } + public get appearance(): ChipAppearance { return this.elementRef.nativeElement.appearance; } diff --git a/packages/angular-workspace/nimble-angular/chip/tests/nimble-chip.directive.spec.ts b/packages/angular-workspace/nimble-angular/chip/tests/nimble-chip.directive.spec.ts index 940d3723d8..953dee10f4 100644 --- a/packages/angular-workspace/nimble-angular/chip/tests/nimble-chip.directive.spec.ts +++ b/packages/angular-workspace/nimble-angular/chip/tests/nimble-chip.directive.spec.ts @@ -54,6 +54,16 @@ describe('Nimble chip', () => { expect(nativeElement.disabled).toBeFalse(); }); + it('has expected defaults for selectable', () => { + expect(directive.selectable).toBeFalse(); + expect(nativeElement.selectable).toBeFalse(); + }); + + it('has expected defaults for selected', () => { + expect(directive.selected).toBeFalse(); + expect(nativeElement.selected).toBeFalse(); + }); + it('has expected defaults for appearance', () => { expect(directive.appearance).toBe(ChipAppearance.outline); expect(nativeElement.appearance).toBe(ChipAppearance.outline); @@ -66,6 +76,8 @@ describe('Nimble chip', () => { `, standalone: false @@ -100,6 +112,16 @@ describe('Nimble chip', () => { expect(nativeElement.disabled).toBeTrue(); }); + it('will use template string values for selectable', () => { + expect(directive.selectable).toBeTrue(); + expect(nativeElement.selectable).toBeTrue(); + }); + + it('will use template string values for selected', () => { + expect(directive.selected).toBeTrue(); + expect(nativeElement.selected).toBeTrue(); + }); + it('will use template string values for appearance', () => { expect(directive.appearance).toBe(ChipAppearance.block); expect(nativeElement.appearance).toBe(ChipAppearance.block); @@ -112,6 +134,8 @@ describe('Nimble chip', () => { `, @@ -122,6 +146,8 @@ describe('Nimble chip', () => { @ViewChild('chip', { read: ElementRef }) public elementRef: ElementRef; public removable = false; public disabled = false; + public selectable = false; + public selected = false; public appearance: ChipAppearance = ChipAppearance.outline; } @@ -162,6 +188,28 @@ describe('Nimble chip', () => { expect(nativeElement.disabled).toBeTrue(); }); + it('can be configured with property binding for selectable', () => { + expect(directive.selectable).toBeFalse(); + expect(nativeElement.selectable).toBeFalse(); + + fixture.componentInstance.selectable = true; + fixture.detectChanges(); + + expect(directive.selectable).toBeTrue(); + expect(nativeElement.selectable).toBeTrue(); + }); + + it('can be configured with property binding for selected', () => { + expect(directive.selected).toBeFalse(); + expect(nativeElement.selected).toBeFalse(); + + fixture.componentInstance.selected = true; + fixture.detectChanges(); + + expect(directive.selected).toBeTrue(); + expect(nativeElement.selected).toBeTrue(); + }); + it('can be configured with property binding for appearance', () => { expect(directive.appearance).toBe(ChipAppearance.outline); expect(nativeElement.appearance).toBe(ChipAppearance.outline); @@ -180,6 +228,8 @@ describe('Nimble chip', () => { `, @@ -190,6 +240,8 @@ describe('Nimble chip', () => { @ViewChild('chip', { read: ElementRef }) public elementRef: ElementRef; public removable: BooleanValueOrAttribute = null; public disabled: BooleanValueOrAttribute = null; + public selectable: BooleanValueOrAttribute = null; + public selected: BooleanValueOrAttribute = null; public appearance: ChipAppearance = ChipAppearance.outline; } @@ -230,6 +282,28 @@ describe('Nimble chip', () => { expect(nativeElement.disabled).toBeTrue(); }); + it('can be configured with attribute binding for selectable', () => { + expect(directive.selectable).toBeFalse(); + expect(nativeElement.selectable).toBeFalse(); + + fixture.componentInstance.selectable = ''; + fixture.detectChanges(); + + expect(directive.selectable).toBeTrue(); + expect(nativeElement.selectable).toBeTrue(); + }); + + it('can be configured with attribute binding for selected', () => { + expect(directive.selected).toBeFalse(); + expect(nativeElement.selected).toBeFalse(); + + fixture.componentInstance.selected = ''; + fixture.detectChanges(); + + expect(directive.selected).toBeTrue(); + expect(nativeElement.selected).toBeTrue(); + }); + it('can be configured with attribute binding for appearance', () => { expect(directive.appearance).toBe(ChipAppearance.outline); expect(nativeElement.appearance).toBe(ChipAppearance.outline); diff --git a/packages/blazor-workspace/CodeAnalysisDictionary.xml b/packages/blazor-workspace/CodeAnalysisDictionary.xml index 76b55dd8e0..a604646326 100644 --- a/packages/blazor-workspace/CodeAnalysisDictionary.xml +++ b/packages/blazor-workspace/CodeAnalysisDictionary.xml @@ -22,6 +22,7 @@ programmatically runtime rtl + selectable sortable spright tooltip diff --git a/packages/blazor-workspace/Examples/Demo.Shared/Pages/Sections/ChipSection.razor b/packages/blazor-workspace/Examples/Demo.Shared/Pages/Sections/ChipSection.razor index a8ceaf3a25..abcd4eb42c 100644 --- a/packages/blazor-workspace/Examples/Demo.Shared/Pages/Sections/ChipSection.razor +++ b/packages/blazor-workspace/Examples/Demo.Shared/Pages/Sections/ChipSection.razor @@ -4,6 +4,7 @@ Outline Chip Block Chip + Selectable Chip Removable Chip Disabled Chip diff --git a/packages/blazor-workspace/Examples/Demo.Shared/Pages/Sections/ChipSection.razor.cs b/packages/blazor-workspace/Examples/Demo.Shared/Pages/Sections/ChipSection.razor.cs index 3b4069eeee..6e9e0c97de 100644 --- a/packages/blazor-workspace/Examples/Demo.Shared/Pages/Sections/ChipSection.razor.cs +++ b/packages/blazor-workspace/Examples/Demo.Shared/Pages/Sections/ChipSection.razor.cs @@ -5,6 +5,8 @@ namespace Demo.Shared.Pages.Sections; public partial class ChipSection { + private bool _chipSelected; + [Inject] private IJSRuntime? JSRuntime { get; set; } diff --git a/packages/blazor-workspace/NimbleBlazor/Source/Chip/NimbleChip.razor b/packages/blazor-workspace/NimbleBlazor/Source/Chip/NimbleChip.razor index 918775be24..8ffd52d0fc 100644 --- a/packages/blazor-workspace/NimbleBlazor/Source/Chip/NimbleChip.razor +++ b/packages/blazor-workspace/NimbleBlazor/Source/Chip/NimbleChip.razor @@ -1,8 +1,11 @@ @namespace NimbleBlazor @ChildContent diff --git a/packages/blazor-workspace/NimbleBlazor/Source/Chip/NimbleChip.razor.cs b/packages/blazor-workspace/NimbleBlazor/Source/Chip/NimbleChip.razor.cs index e75a080a7e..1c79e8af9b 100644 --- a/packages/blazor-workspace/NimbleBlazor/Source/Chip/NimbleChip.razor.cs +++ b/packages/blazor-workspace/NimbleBlazor/Source/Chip/NimbleChip.razor.cs @@ -4,6 +4,12 @@ namespace NimbleBlazor; public partial class NimbleChip : ComponentBase { + [Parameter] + public bool? Selectable { get; set; } + + [Parameter] + public bool? Selected { get; set; } + [Parameter] public bool? Removable { get; set; } @@ -22,6 +28,12 @@ public partial class NimbleChip : ComponentBase [Parameter] public EventCallback ChipRemoved { get; set; } + /// + /// Gets or sets a callback that's invoked when the chip selected state changes. + /// + [Parameter] + public EventCallback SelectedChanged { get; set; } + /// /// Called when the chip remove button is activated. /// @@ -30,6 +42,15 @@ protected async void HandleRemove() await ChipRemoved.InvokeAsync(); } + /// + /// Called when the chip selected state changes. + /// + protected async void HandleSelectedChange() + { + Selected = !Selected.GetValueOrDefault(); + await SelectedChanged.InvokeAsync(Selected.Value); + } + [Parameter(CaptureUnmatchedValues = true)] public IDictionary? AdditionalAttributes { get; set; } } diff --git a/packages/blazor-workspace/NimbleBlazor/Source/Patterns/EventHandlers.cs b/packages/blazor-workspace/NimbleBlazor/Source/Patterns/EventHandlers.cs index ba04ff668c..c3cfdc97c3 100644 --- a/packages/blazor-workspace/NimbleBlazor/Source/Patterns/EventHandlers.cs +++ b/packages/blazor-workspace/NimbleBlazor/Source/Patterns/EventHandlers.cs @@ -82,6 +82,7 @@ public class WaferMapHoverDieChangedEventArgs : EventArgs [EventHandler("onnimbletabsactiveidchange", typeof(TabsChangeEventArgs), enableStopPropagation: true, enablePreventDefault: true)] [EventHandler("onnimblecheckedchange", typeof(CheckboxChangeEventArgs), enableStopPropagation: true, enablePreventDefault: true)] [EventHandler("onnimblechipremove", typeof(EventArgs), enableStopPropagation: true, enablePreventDefault: false)] +[EventHandler("onnimblechipselectedchange", typeof(EventArgs), enableStopPropagation: true, enablePreventDefault: false)] [EventHandler("onnimblemenubuttontoggle", typeof(MenuButtonToggleEventArgs), enableStopPropagation: true, enablePreventDefault: false)] [EventHandler("onnimblemenubuttonbeforetoggle", typeof(MenuButtonToggleEventArgs), enableStopPropagation: true, enablePreventDefault: false)] [EventHandler("onnimblebannertoggle", typeof(BannerToggleEventArgs), enableStopPropagation: true, enablePreventDefault: true)] diff --git a/packages/nimble-components/src/chip/index.ts b/packages/nimble-components/src/chip/index.ts index 366ef4c6b1..0ee4b93dc2 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, @@ -34,12 +35,29 @@ export class Chip extends FoundationElement { @attr({ mode: 'boolean' }) public disabled = false; + @attr({ mode: 'boolean' }) + public selectable = false; + + @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[]; @@ -57,25 +75,200 @@ export class Chip extends FoundationElement { return itemRemoveLabel.getValueFor(this); } + /** @internal */ + @observable + public removeButtonTabIndex: string | null = null; + /** @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.selectable) { + e.stopPropagation(); + 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: + if (this.selectable) { + e.stopPropagation(); + this.selected = !this.selected; + this.$emit('selected-change'); + } + return true; + default: + return true; + } + } + + /** @internal */ + public keydownHandler(e: KeyboardEvent): boolean { + if (this.disabled) { + return false; + } + switch (e.key) { + case keySpace: + if (this.selectable) { + return false; + } + return true; + case keyEnter: + if (this.selectable) { + e.stopPropagation(); + this.selected = !this.selected; + this.$emit('selected-change'); + return false; + } + return true; + case keyEscape: + if (this.removable) { + e.stopPropagation(); + 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 selectableChanged(_oldValue: boolean, _newValue: boolean): 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; + this.updateRemoveButtonTabIndex(); + } + + private updateManagedTabIndex(): void { + if (!this.$fastController?.isConnected) { + return; + } + + const shouldManage = this.selectable && !this.disabled; + + if (shouldManage) { + if (!this.hasAttribute('tabindex')) { + this.setManagedTabIndex(0); + } + } else { + this.removeManagedTabIndex(); + } + + this.updateRemoveButtonTabIndex(); + } + + 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'); + } + + private updateRemoveButtonTabIndex(): void { + this.removeButtonTabIndex = this.selectable + ? '-1' + : (this.hasAttribute('tabindex') ? `${this.tabIndex}` : null); + } } 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..cab317418d 100644 --- a/packages/nimble-components/src/chip/specs/README.md +++ b/packages/nimble-components/src/chip/specs/README.md @@ -64,10 +64,13 @@ _The key elements of the component's public API surface:_ - `removable` - set to show the remove button - `appearance` - supports `outline` and `block` appearances - `disabled` - styles the chip in the typical Nimble manner, including any user-slotted content (text and icon), and additionally hides the remove button. + - `selectable` - makes the chip act as a toggle button with `role="button"` and `aria-pressed`. + - `selected` - indicates the current selection state when `selectable` is set. When `true`, the chip displays with selected background styling. - _Methods_ - _None_ - _Events_ - - `remove` - fired when the chip remove button is pressed. + - `remove` - fired when the chip remove button is pressed, or when Escape key is pressed on a removable chip. + - `selected-change` - fired when the user toggles the chip's selected state via click or keyboard (Space/Enter). Only emitted when `selectable` is set. - _Slots_ - `start` - icon placed to the left of the chip text - (default) - for the primary label text @@ -145,12 +148,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 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 comply with [WCAG 4.1.2](https://dequeuniversity.com/rules/axe/4.11/nested-interactive), which prohibits nested interactive controls + - When the chip is removable but not selectable: + - 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 @@ -160,8 +169,6 @@ _Consider the accessibility of the component, including:_ ### Future work -- Make chip selectable (there are already UX designs) - - Currently there are no use-cases for chips requiring them to be selectable, but there are many use-cases in the wild where this is needed. - Provide error state for the chip (there are already UX designs) - Again, there are no current use-cases requiring a chip to present with error information, but it is not unreasonable to expect we may have such a use-case in the future. - Create a chip container component that manages chip layout, and removal 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..9d38dfcb4c --- /dev/null +++ b/packages/nimble-components/src/chip/specs/selection-toggle.md @@ -0,0 +1,135 @@ +# 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) +- [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 a `selectable` boolean attribute to control whether it exposes toggle behavior. + +- **Attribute:** `selectable` (boolean, default: `false`) + - When absent, the chip is not selectable. It does not have `role="button"` or `aria-pressed`. User-supplied `tabindex` is forwarded to the remove button if removable. + - When present, the chip can be toggled on/off via click or Space/Enter keys. It has `role="button"` and `aria-pressed`. It automatically receives `tabindex="0"` unless the user provides a different value. + - This API is intentionally local to each chip. It does not imply coordinated single-selection behavior across a group of chips. +- **Attribute:** `selected` (boolean, default: `false`) + - Indicates the current selection state when `selectable` is set. + - 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 `selectable` is set. +- **Event:** `remove` + - Emitted when the user activates the remove button (click) or presses Escape key (when removable and not disabled). + +### Keyboard Interaction + +- **Space/Enter:** Toggles `selected` state (when `selectable` is set). + - **Note:** Enter key is handled on `keydown` to match native button behavior. Space key is handled on `keyup` to allow for active state styling. +- **Escape:** Removes the chip (when `removable` and not `disabled`). + - When the chip is selectable, the remove button is set to `tabindex="-1"` to comply with WCAG 4.1.2, which prohibits nested interactive controls. The Escape key provides keyboard access to the remove functionality in this case. + - For non-selectable chips, Escape also works as a convenient alternative to clicking the remove button. + - **Note:** Escape key is handled on `keydown` to prevent event propagation (e.g., to parent dialogs). + +### Accessibility + +- **Role:** + - If `selectable` is not set: No explicit role (generic container). + - If `selectable` is set: `role="button"` with `aria-pressed="true"` or `aria-pressed="false"`. +- **Tabindex Management:** + - When `selectable` is set: The chip automatically manages its own `tabindex` (defaults to `0`). User-supplied values are preserved. + - When `selectable` is not set: The chip does not manage `tabindex`. User-supplied values are forwarded to the remove button if `removable`. +- **Attribute Reflection:** + - The `selectable` attribute directly reflects whether the chip is interactive. + - The `selected` attribute reflects local chip state, but only affects accessibility and styling when `selectable` is set. +- **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). The Escape key provides keyboard access to the remove functionality for all removable chips. + +### 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 non-selectable chips; `rgba(borderRgbPartialColor, 0.3)` for selectable, unselected, non-block chips + - Background: transparent + - Cursor: pointer (when `selectable` is set) +- **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 `selectable` 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. +- **Event Propagation:** + - `click`, `keydown` (Enter/Escape), and `keyup` (Space) handlers call `stopPropagation()` when they successfully handle an event. This prevents unintended side effects in parent containers (e.g., closing a dialog when removing a chip, or triggering a parent click handler when toggling selection). + +## 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 `selectable` attribute provides a clean API for opt-in behavior without requiring a separate component. + +### Alternative 2: `selection-mode` Enum + +- Use a `selection-mode` enum with values such as `single`. + - **Pros:** Leaves room for future expansion. + - **Cons:** Implies coordinated multi-chip selection behavior that the component does not provide today. + - **Decision:** Rejected. A boolean `selectable` attribute better matches the chip's local behavior and avoids implying a future container contract. + +### Alternative 3: 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 4: 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 comply with WCAG 4.1.2 and avoid nested interactive controls. diff --git a/packages/nimble-components/src/chip/styles.ts b/packages/nimble-components/src/chip/styles.ts index ffb5f2464b..f6d49d39d2 100644 --- a/packages/nimble-components/src/chip/styles.ts +++ b/packages/nimble-components/src/chip/styles.ts @@ -1,82 +1,160 @@ 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([selectable]) { + cursor: pointer; + } + + :host([selectable]:not([selected]):not([appearance='block'])) { + border-color: rgba(${borderRgbPartialColor}, 0.3); + } + + :host([selectable][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([selectable]: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([selectable]${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( + [selectable]: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 27605b9535..f50a930c1b 100644 --- a/packages/nimble-components/src/chip/template.ts +++ b/packages/nimble-components/src/chip/template.ts @@ -14,31 +14,41 @@ 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 74f11e0566..12791d6db3 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