From 16bfac1343128f641b34e74e21c69a05a28a9185 Mon Sep 17 00:00:00 2001 From: Elizabeth Mitchell Date: Mon, 18 Sep 2023 10:57:21 -0700 Subject: [PATCH] feat(chips): swap to toolbar a11y pattern BREAKING CHANGE: chips now follow the [aria toolbar pattern](https://www.w3.org/WAI/ARIA/apg/patterns/toolbar/examples/toolbar/). Chip sets are toolbars and chips are buttons or links. Filter chips are toggle buttons. What to change: - Remove `type` attribute from `` (you can mix and match chip types!) - Remove `single-select` from ``. Use JS to control filter chips if single selection is required. Radio filter chips will come in a future update. - Disabled chips CAN be focused with the keyboard if `always-focusable` is set. - Filter chips no longer dispatch a `"selected"` event. Listen to `"click"` and use `event.target.selected` instead. - ArrowUp and ArrowDown no longer navigate between chips. These are reserved for chip actions, like dropdown menu chips. PiperOrigin-RevId: 566352830 --- chips/demo/demo.ts | 1 - chips/demo/stories.ts | 105 +++++++---------- chips/internal/_chip-set.scss | 6 - chips/internal/assist-chip.ts | 16 +-- chips/internal/chip-set.ts | 108 ++++++----------- chips/internal/chip-set_test.ts | 143 +++-------------------- chips/internal/chip.ts | 71 ++++++----- chips/internal/chip_test.ts | 46 ++++++++ chips/internal/filter-chip.ts | 38 ++---- chips/internal/filter-chip_test.ts | 24 ---- chips/internal/input-chip.ts | 26 ++--- chips/internal/multi-action-chip.ts | 26 ++++- chips/internal/multi-action-chip_test.ts | 13 +-- chips/internal/trailing-icons.ts | 10 +- docs/components/chip.md | 104 +++++++++-------- 15 files changed, 297 insertions(+), 440 deletions(-) create mode 100644 chips/internal/chip_test.ts diff --git a/chips/demo/demo.ts b/chips/demo/demo.ts index 982e3fb6b..7aee954e6 100644 --- a/chips/demo/demo.ts +++ b/chips/demo/demo.ts @@ -17,7 +17,6 @@ const collection = new Knob('label', {defaultValue: '', ui: textInput()}), new Knob('elevated', {defaultValue: false, ui: boolInput()}), new Knob('disabled', {defaultValue: false, ui: boolInput()}), - new Knob('singleSelect', {defaultValue: false, ui: boolInput()}), new Knob('scrolling', {defaultValue: false, ui: boolInput()}), ]); diff --git a/chips/demo/stories.ts b/chips/demo/stories.ts index 3b5e5020e..d5e522110 100644 --- a/chips/demo/stories.ts +++ b/chips/demo/stories.ts @@ -20,7 +20,6 @@ export interface StoryKnobs { label: string; elevated: boolean; disabled: boolean; - singleSelect: boolean; scrolling: boolean; } @@ -44,14 +43,13 @@ const styles = css` } `; -const standard: MaterialStoryInit = { +const assist: MaterialStoryInit = { name: 'Assist chips', styles, render({label, elevated, disabled, scrolling}) { - const classes = {scrolling}; + const classes = {'scrolling': scrolling}; return html` - + = { > local_laundry_service - - `; - } -}; - -const links: MaterialStoryInit = { - name: 'Assist link chips', - styles, - render({label, elevated, disabled, scrolling}) { - const classes = {scrolling}; - return html` - ${GOOGLE_LOGO} + `; } @@ -92,12 +82,11 @@ const links: MaterialStoryInit = { const filters: MaterialStoryInit = { name: 'Filter chips', styles, - render({label, elevated, disabled, scrolling, singleSelect}) { - const classes = {scrolling}; + render({label, elevated, disabled, scrolling}) { + const classes = {'scrolling': scrolling}; return html` - + = { ?elevated=${elevated} removable > + `; } @@ -125,10 +121,9 @@ const inputs: MaterialStoryInit = { name: 'Input chips', styles, render({label, disabled, scrolling}) { - const classes = {scrolling}; + const classes = {'scrolling': scrolling}; return html` - + = { > + ${GOOGLE_LOGO} - - `; - } -}; - -const inputLinks: MaterialStoryInit = { - name: 'Input link chips', - styles, - render({label, disabled, scrolling}) { - const classes = {scrolling}; - return html` - ${GOOGLE_LOGO} + label=${label || 'Disabled input chip (focusable)'} + disabled + always-focusable + > `; } @@ -179,10 +165,9 @@ const suggestions: MaterialStoryInit = { name: 'Suggestion chips', styles, render({label, elevated, disabled, scrolling}) { - const classes = {scrolling}; + const classes = {'scrolling': scrolling}; return html` - + = { > local_laundry_service - - `; - } -}; - -const suggestionLinks: MaterialStoryInit = { - name: 'Suggestion link chips', - styles, - render({label, elevated, disabled, scrolling}) { - const classes = {scrolling}; - return html` - ${GOOGLE_LOGO} + `; } }; /** Chips stories. */ -export const stories = [ - standard, links, filters, inputs, inputLinks, suggestions, suggestionLinks -]; +export const stories = [assist, filters, inputs, suggestions]; diff --git a/chips/internal/_chip-set.scss b/chips/internal/_chip-set.scss index aae69bf7b..d52a4c19a 100644 --- a/chips/internal/_chip-set.scss +++ b/chips/internal/_chip-set.scss @@ -9,10 +9,4 @@ flex-wrap: wrap; gap: 8px; } - - .content { - display: flex; - flex-wrap: inherit; - gap: inherit; - } } diff --git a/chips/internal/assist-chip.ts b/chips/internal/assist-chip.ts index 014baabc0..3892ebabb 100644 --- a/chips/internal/assist-chip.ts +++ b/chips/internal/assist-chip.ts @@ -11,7 +11,7 @@ import {property} from 'lit/decorators.js'; import {ARIAMixinStrict} from '../../internal/aria/aria.js'; -import {Chip, renderGridAction, renderGridContainer} from './chip.js'; +import {Chip} from './chip.js'; /** * An assist chip component. @@ -30,10 +30,6 @@ export class AssistChip extends Chip { return !this.href && this.disabled; } - protected override renderContainer(content: unknown) { - return renderGridContainer(content, this.getContainerClasses()); - } - protected override getContainerClasses() { return { ...super.getContainerClasses(), @@ -47,24 +43,24 @@ export class AssistChip extends Chip { protected override renderPrimaryAction(content: unknown) { const {ariaLabel} = this as ARIAMixinStrict; if (this.href) { - return renderGridAction(html` + return html` ${content} - `); + `; } - return renderGridAction(html` + return html` - `); + `; } protected override renderOutline() { diff --git a/chips/internal/chip-set.ts b/chips/internal/chip-set.ts index 9c7c8c6e0..5b33e3268 100644 --- a/chips/internal/chip-set.ts +++ b/chips/internal/chip-set.ts @@ -4,25 +4,19 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {html, isServer, LitElement, nothing, PropertyValues} from 'lit'; -import {property, queryAssignedElements} from 'lit/decorators.js'; +import {html, isServer, LitElement} from 'lit'; +import {queryAssignedElements} from 'lit/decorators.js'; -import {ARIAMixinStrict} from '../../internal/aria/aria.js'; -import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js'; +import {polyfillElementInternalsAria, setupHostAria} from '../../internal/aria/aria.js'; import {Chip} from './chip.js'; -/** - * The type of chip a chip set controls. - */ -export type ChipSetType = 'assist'|'suggestion'|'filter'|'input'|''; - /** * A chip set component. */ export class ChipSet extends LitElement { static { - requestUpdateOnAriaChange(ChipSet); + setupHostAria(ChipSet, {focusable: false}); } get chips() { @@ -30,60 +24,31 @@ export class ChipSet extends LitElement { (child): child is Chip => child instanceof Chip); } - @property() type: ChipSetType = ''; - @property({type: Boolean, attribute: 'single-select'}) singleSelect = false; - @queryAssignedElements() private readonly childElements!: HTMLElement[]; + private readonly internals = polyfillElementInternalsAria( + this, (this as HTMLElement /* needed for closure */).attachInternals()); constructor() { super(); if (!isServer) { this.addEventListener('focusin', this.updateTabIndices.bind(this)); + this.addEventListener('update-focus', this.updateTabIndices.bind(this)); this.addEventListener('keydown', this.handleKeyDown.bind(this)); - this.addEventListener('selected', this.handleSelected.bind(this)); - } - } - - protected override updated(changed: PropertyValues) { - if (changed.has('singleSelect') && this.singleSelect) { - let hasSelectedChip = false; - for (const chip of this.chips as MaybeSelectableChip[]) { - if (chip.selected === true) { - if (!hasSelectedChip) { - hasSelectedChip = true; - continue; - } - - chip.selected = false; - } - } + this.internals.role = 'toolbar'; } } protected override render() { - const {ariaLabel} = this as ARIAMixinStrict; - const isFilter = this.type === 'filter'; - const role = isFilter ? 'listbox' : 'grid'; - const multiselectable = isFilter ? !this.singleSelect : nothing; - return html` -
- -
- `; + return html``; } private handleKeyDown(event: KeyboardEvent) { - const isDown = event.key === 'ArrowDown'; - const isUp = event.key === 'ArrowUp'; const isLeft = event.key === 'ArrowLeft'; const isRight = event.key === 'ArrowRight'; const isHome = event.key === 'Home'; const isEnd = event.key === 'End'; // Ignore non-navigation keys - if (!isLeft && !isRight && !isDown && !isUp && !isHome && !isEnd) { + if (!isLeft && !isRight && !isHome && !isEnd) { return; } @@ -105,7 +70,7 @@ export class ChipSet extends LitElement { // Check if moving forwards or backwards const isRtl = getComputedStyle(this).direction === 'rtl'; - const forwards = isRtl ? isLeft || isUp : isRight || isDown; + const forwards = isRtl ? isLeft : isRight; const focusedChip = chips.find(chip => chip.matches(':focus-within')); if (!focusedChip) { // If there is not already a chip focused, select the first or last chip @@ -131,8 +96,11 @@ export class ChipSet extends LitElement { // Check if the next sibling is disabled. If so, // move the index and continue searching. + // + // Some toolbar items may be focusable when disabled for increased + // visibility. const nextChip = chips[nextIndex]; - if (nextChip.disabled) { + if (nextChip.disabled && !nextChip.alwaysFocusable) { if (forwards) { nextIndex++; } else { @@ -149,33 +117,31 @@ export class ChipSet extends LitElement { } private updateTabIndices() { + // The chip that should be focusable is either the chip that currently has + // focus or the first chip that can be focused. const {chips} = this; - let hasFocusedChip = false; + let chipToFocus: Chip|undefined; for (const chip of chips) { - if (chip.matches(':focus-within')) { - chip.removeAttribute('tabindex'); - hasFocusedChip = true; - } else { - chip.tabIndex = -1; + const isChipFocusable = chip.alwaysFocusable || !chip.disabled; + const chipIsFocused = chip.matches(':focus-within'); + if (chipIsFocused && isChipFocusable) { + // Found the first chip that is actively focused. This overrides the + // first focusable chip found. + chipToFocus = chip; + continue; } - } - if (!hasFocusedChip) { - chips[0]?.removeAttribute('tabindex'); - } - } - - private handleSelected(event: Event) { - if (!this.singleSelect) { - return; - } - - if ((event.target as MaybeSelectableChip).selected === true) { - for (const chip of this.chips as MaybeSelectableChip[]) { - if (chip !== event.target && chip.selected) { - chip.selected = false; - } + if (isChipFocusable && !chipToFocus) { + chipToFocus = chip; } + + // Disable non-focused chips. If we disable all of them, we'll grant focus + // to the first focusable child that was found. + chip.tabIndex = -1; + } + + if (chipToFocus) { + chipToFocus.tabIndex = 0; } } } @@ -183,7 +149,3 @@ export class ChipSet extends LitElement { interface MaybeMultiActionChip extends Chip { focus(options?: FocusOptions&{trailing?: boolean}): void; } - -interface MaybeSelectableChip extends Chip { - selected?: boolean; -} diff --git a/chips/internal/chip-set_test.ts b/chips/internal/chip-set_test.ts index d9ef31671..244c05105 100644 --- a/chips/internal/chip-set_test.ts +++ b/chips/internal/chip-set_test.ts @@ -16,7 +16,6 @@ import {ChipHarness} from '../harness.js'; import {AssistChip} from './assist-chip.js'; import {Chip} from './chip.js'; import {ChipSet} from './chip-set.js'; -import {FilterChip} from './filter-chip.js'; import {InputChip} from './input-chip.js'; @customElement('test-chip-set') @@ -28,9 +27,6 @@ class TestAssistChip extends AssistChip { @customElement('test-chip-set-input-chip') class TestInputChip extends InputChip { } -@customElement('test-chip-set-filter-chip') -class TestFilterChip extends FilterChip { -} describe('Chip set', () => { const env = new Environment(); @@ -68,9 +64,9 @@ describe('Chip set', () => { const chipSet = await setupTest( [new TestAssistChip(), new TestAssistChip(), new TestAssistChip()]); - expect(chipSet.chips[0].hasAttribute('tabindex')) - .withContext('first chip does not have tabindex') - .toBeFalse(); + expect(chipSet.chips[0].getAttribute('tabindex')) + .withContext('first tabindex') + .toBe('0'); expect(chipSet.chips[1].getAttribute('tabindex')) .withContext('second tabindex') .toBe('-1'); @@ -117,20 +113,6 @@ describe('Chip set', () => { }); }); - it('should navigate forward on vertical arrow keys', async () => { - const first = new TestAssistChip(); - const second = new TestAssistChip(); - const third = new TestAssistChip(); - const chipSet = await setupTest([first, second, third]); - await testNavigation({ - chipSet, - ltrKey: 'ArrowDown', - rtlKey: 'ArrowUp', - current: first, - next: second - }); - }); - it('should navigate backward on horizontal keys', async () => { const first = new TestAssistChip(); const second = new TestAssistChip(); @@ -145,20 +127,6 @@ describe('Chip set', () => { }); }); - it('should navigate backward on vertical keys', async () => { - const first = new TestAssistChip(); - const second = new TestAssistChip(); - const third = new TestAssistChip(); - const chipSet = await setupTest([first, second, third]); - await testNavigation({ - chipSet, - ltrKey: 'ArrowUp', - rtlKey: 'ArrowDown', - current: second, - next: first - }); - }); - it('should navigate to the first chip on Home', async () => { const first = new TestAssistChip(); const second = new TestAssistChip(); @@ -232,6 +200,22 @@ describe('Chip set', () => { }); }); + it('should NOT skip over disabled always focusable chips', async () => { + const first = new TestAssistChip(); + const second = new TestAssistChip(); + second.disabled = true; + second.alwaysFocusable = true; + const third = new TestAssistChip(); + const chipSet = await setupTest([first, second, third]); + await testNavigation({ + chipSet, + ltrKey: 'ArrowRight', + rtlKey: 'ArrowLeft', + current: first, + next: second + }); + }); + it('should focus trailing actions when navigating backwards', async () => { const first = new TestInputChip(); const second = new TestInputChip(); @@ -277,93 +261,4 @@ describe('Chip set', () => { .toBeTrue(); }); }); - - describe('single selection', () => { - it('should allow multi-selection if singleSelect is false', async () => { - const first = new TestFilterChip(); - const second = new TestFilterChip(); - const chipSet = await setupTest([first, second]); - chipSet.singleSelect = false; - - await new ChipHarness(first).clickWithMouse(); - await new ChipHarness(second).clickWithMouse(); - - expect(first.selected).withContext('first chip is selected').toBeTrue(); - expect(second.selected).withContext('second chip is selected').toBeTrue(); - }); - - it('should deselect other chips when another chip is selected', - async () => { - const first = new TestFilterChip(); - const second = new TestFilterChip(); - second.selected = true; - const chipSet = await setupTest([first, second]); - chipSet.singleSelect = true; - - await new ChipHarness(first).clickWithMouse(); - - expect(first.selected) - .withContext('first chip is selected') - .toBeTrue(); - expect(second.selected) - .withContext('second chip is not selected') - .toBeFalse(); - }); - - it('should not do anything if all chips are deselected and the property changes', - async () => { - const first = new TestFilterChip(); - const second = new TestFilterChip(); - - const chipSet = await setupTest([first, second]); - chipSet.singleSelect = true; - await env.waitForStability(); - - expect(first.selected) - .withContext('first chip is still unselected') - .toBeFalse(); - expect(second.selected) - .withContext('second chip is still unselected') - .toBeFalse(); - }); - - it('should ensure one chip is selected when property changes', async () => { - const first = new TestFilterChip(); - first.selected = true; - const second = new TestFilterChip(); - second.selected = true; - - const chipSet = await setupTest([first, second]); - chipSet.singleSelect = true; - await env.waitForStability(); - - expect(first.selected).withContext('first chip is selected').toBeTrue(); - expect(second.selected) - .withContext('second chip is deselected') - .toBeFalse(); - }); - - it('should prefer setting the first selected chip as the single selected chip when property changes', - async () => { - const first = new TestFilterChip(); - const second = new TestFilterChip(); - second.selected = true; - const third = new TestFilterChip(); - second.selected = true; - - const chipSet = await setupTest([first, second, third]); - chipSet.singleSelect = true; - await env.waitForStability(); - - expect(first.selected) - .withContext('first chip is still unselected') - .toBeFalse(); - expect(second.selected) - .withContext('second chip remains selected') - .toBeTrue(); - expect(third.selected) - .withContext('third chip is deselected') - .toBeFalse(); - }); - }); }); diff --git a/chips/internal/chip.ts b/chips/internal/chip.ts index 607b942ce..0d590cbdd 100644 --- a/chips/internal/chip.ts +++ b/chips/internal/chip.ts @@ -7,7 +7,7 @@ import '../../focus/md-focus-ring.js'; import '../../ripple/ripple.js'; -import {html, LitElement, nothing, TemplateResult} from 'lit'; +import {html, LitElement, PropertyValues, TemplateResult} from 'lit'; import {property} from 'lit/decorators.js'; import {ClassInfo, classMap} from 'lit/directives/class-map.js'; @@ -27,7 +27,26 @@ export abstract class Chip extends LitElement { delegatesFocus: true }; + /** + * Whether or not the chip is disabled. + * + * Disabled chips are not focusable, unless `always-focusable` is set. + */ @property({type: Boolean}) disabled = false; + + /** + * When true, allow disabled chips to be focused with arrow keys. + * + * Add this when a chip needs increased visibility when disabled. See + * https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#kbd_disabled_controls + * for more guidance on when this is needed. + */ + @property({type: Boolean, attribute: 'always-focusable'}) + alwaysFocusable = false; + + /** + * The label of the chip. + */ @property() label = ''; /** @@ -43,15 +62,31 @@ export abstract class Chip extends LitElement { return this.disabled; } - protected override render() { - return this.renderContainer(this.renderContainerContent()); + override focus(options?: FocusOptions) { + if (this.disabled && !this.alwaysFocusable) { + return; + } + + super.focus(options); } - protected abstract renderContainer(content: unknown): unknown; + protected override render() { + return html` +
+ ${this.renderContainerContent()} +
+ `; + } - protected getContainerClasses() { + protected override updated(changed: PropertyValues) { + if (changed.has('disabled') && changed.get('disabled') !== undefined) { + this.dispatchEvent(new Event('update-focus', {bubbles: true})); + } + } + + protected getContainerClasses(): ClassInfo { return { - disabled: this.disabled, + 'disabled': this.disabled, }; } @@ -86,27 +121,3 @@ export abstract class Chip extends LitElement { `; } } - -/** - * Renders a chip container that follows the grid/row/cell a11y pattern. - * - * This renders the container with `role="row"`. - */ -export function renderGridContainer(content: unknown, classes: ClassInfo) { - return html` -
${content}
- `; -} - -/** - * Renders a chip action that follows the grid/row/cell a11y pattern. - * - * This wraps actions in a `role="cell"` div. - */ -export function renderGridAction(content: unknown) { - if (content === nothing) { - return content; - } - - return html`
${content}
`; -} diff --git a/chips/internal/chip_test.ts b/chips/internal/chip_test.ts new file mode 100644 index 000000000..a61714f38 --- /dev/null +++ b/chips/internal/chip_test.ts @@ -0,0 +1,46 @@ +/** + * @license + * Copyright 2023 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +// import 'jasmine'; (google3-only) + +import {html} from 'lit'; +import {customElement} from 'lit/decorators.js'; + +import {Environment} from '../../testing/environment.js'; +import {ChipHarness} from '../harness.js'; + +import {Chip} from './chip.js'; + +@customElement('test-chip') +class TestChip extends Chip { + primaryId = 'button'; + + override renderPrimaryAction() { + return html``; + } +} + +describe('Chip', () => { + const env = new Environment(); + + async function setupTest() { + const chip = new TestChip(); + env.render(html`${chip}`); + await env.waitForStability(); + return {chip, harness: new ChipHarness(chip)}; + } + + it('should dispatch `update-focus` for chip set when disabled changes', + async () => { + const {chip} = await setupTest(); + const updateFocusListener = jasmine.createSpy('updateFocusListener'); + chip.addEventListener('update-focus', updateFocusListener); + + chip.disabled = true; + await env.waitForStability(); + expect(updateFocusListener).toHaveBeenCalled(); + }); +}); diff --git a/chips/internal/filter-chip.ts b/chips/internal/filter-chip.ts index aaf925b51..f2371b67d 100644 --- a/chips/internal/filter-chip.ts +++ b/chips/internal/filter-chip.ts @@ -6,9 +6,8 @@ import '../../elevation/elevation.js'; -import {html, nothing, PropertyValues, svg} from 'lit'; +import {html, nothing} from 'lit'; import {property, query} from 'lit/decorators.js'; -import {classMap} from 'lit/directives/class-map.js'; import {ARIAMixinStrict} from '../../internal/aria/aria.js'; import {redispatchEvent} from '../../internal/controller/events.js'; @@ -25,28 +24,13 @@ export class FilterChip extends MultiActionChip { @property({type: Boolean, reflect: true}) selected = false; protected get primaryId() { - return 'option'; + return 'button'; } @query('.primary.action') protected readonly primaryAction!: HTMLElement|null; @query('.trailing.action') protected readonly trailingAction!: HTMLElement|null; - protected override updated(changed: PropertyValues) { - if (changed.has('selected') && changed.get('selected') !== undefined) { - // Dispatch when `selected` changes, except for the first update. - this.dispatchEvent(new Event('selected', {bubbles: true})); - } - } - - protected override renderContainer(content: unknown) { - const classes = this.getContainerClasses(); - // Note: an explicit `role="presentation"` is needed for VoiceOver. Without - // it, linear navigation gets stuck on the first filter chip. - return html``; - } - protected override getContainerClasses() { return { ...super.getContainerClasses(), @@ -60,11 +44,10 @@ export class FilterChip extends MultiActionChip { const {ariaLabel} = this as ARIAMixinStrict; return html` `; @@ -75,17 +58,20 @@ export class FilterChip extends MultiActionChip { return super.renderLeadingIcon(); } - return svg` + return html` `; } - protected override renderTrailingAction() { + protected override renderTrailingAction(focusListener: EventListener) { if (this.removable) { - return renderRemoveButton( - {ariaLabel: this.ariaLabelRemove, disabled: this.disabled}); + return renderRemoveButton({ + focusListener, + ariaLabel: this.ariaLabelRemove, + disabled: this.disabled + }); } return nothing; diff --git a/chips/internal/filter-chip_test.ts b/chips/internal/filter-chip_test.ts index f7c3e4c1e..266551598 100644 --- a/chips/internal/filter-chip_test.ts +++ b/chips/internal/filter-chip_test.ts @@ -53,30 +53,6 @@ describe('Filter chip', () => { expect(chip.selected).withContext('chip.selected').toBeFalse(); }); - it('should dispatch "selected" event when selected changes programmatically', - async () => { - const {chip} = await setupTest(); - const handler = jasmine.createSpy(); - chip.addEventListener('selected', handler); - - chip.selected = true; - await env.waitForStability(); - chip.selected = false; - await env.waitForStability(); - expect(handler).toHaveBeenCalledTimes(2); - }); - - it('should dispatch "selected" event when selected changes by click', - async () => { - const {chip, harness} = await setupTest(); - const handler = jasmine.createSpy(); - chip.addEventListener('selected', handler); - - await harness.clickWithMouse(); - await harness.clickWithMouse(); - expect(handler).toHaveBeenCalledTimes(2); - }); - it('can prevent default', async () => { const {chip, harness} = await setupTest(); const handler = jasmine.createSpy(); diff --git a/chips/internal/input-chip.ts b/chips/internal/input-chip.ts index cd4542859..d19215b11 100644 --- a/chips/internal/input-chip.ts +++ b/chips/internal/input-chip.ts @@ -9,7 +9,6 @@ import {property, query} from 'lit/decorators.js'; import {ARIAMixinStrict} from '../../internal/aria/aria.js'; -import {renderGridAction, renderGridContainer} from './chip.js'; import {MultiActionChip} from './multi-action-chip.js'; import {renderRemoveButton} from './trailing-icons.js'; @@ -53,10 +52,6 @@ export class InputChip extends MultiActionChip { @query('.trailing.action') protected readonly trailingAction!: HTMLElement|null; - protected override renderContainer(content: unknown) { - return renderGridContainer(content, this.getContainerClasses()); - } - protected override getContainerClasses() { return { ...super.getContainerClasses(), @@ -72,39 +67,40 @@ export class InputChip extends MultiActionChip { protected override renderPrimaryAction(content: unknown) { const {ariaLabel} = this as ARIAMixinStrict; if (this.href) { - return renderGridAction(html` + return html` ${content} - `); + `; } if (this.removeOnly) { - return renderGridAction(html` + return html` ${content} - `); + `; } - return renderGridAction(html` + return html` - `); + `; } - protected override renderTrailingAction() { - return renderGridAction(renderRemoveButton({ + protected override renderTrailingAction(focusListener: EventListener) { + return renderRemoveButton({ + focusListener, ariaLabel: this.ariaLabelRemove, disabled: !this.href && this.disabled, tabbable: this.removeOnly, - })); + }); } } diff --git a/chips/internal/multi-action-chip.ts b/chips/internal/multi-action-chip.ts index 4465a17b4..af427b7fe 100644 --- a/chips/internal/multi-action-chip.ts +++ b/chips/internal/multi-action-chip.ts @@ -44,28 +44,31 @@ export abstract class MultiActionChip extends Chip { constructor() { super(); + this.handleTrailingActionFocus = this.handleTrailingActionFocus.bind(this); if (!isServer) { this.addEventListener('keydown', this.handleKeyDown.bind(this)); } } override focus(options?: FocusOptions&{trailing?: boolean}) { - if (options?.trailing && this.trailingAction) { + const isFocusable = this.alwaysFocusable || !this.disabled; + if (isFocusable && options?.trailing && this.trailingAction) { this.trailingAction.focus(options); return; } - super.focus(options); + super.focus(options as FocusOptions); } protected override renderContainerContent() { return html` ${super.renderContainerContent()} - ${this.renderTrailingAction()} + ${this.renderTrailingAction(this.handleTrailingActionFocus)} `; } - protected abstract renderTrailingAction(): unknown; + protected abstract renderTrailingAction(focusListener: EventListener): + unknown; private handleKeyDown(event: KeyboardEvent) { const isLeft = event.key === 'ArrowLeft'; @@ -98,4 +101,19 @@ export abstract class MultiActionChip extends Chip { const actionToFocus = forwards ? this.trailingAction : this.primaryAction; actionToFocus.focus(); } + + private handleTrailingActionFocus() { + const {primaryAction, trailingAction} = this; + if (!primaryAction || !trailingAction) { + return; + } + + // Temporarily turn off the primary action's focusability. This allows + // shift+tab from the trailing action to move to the previous chip rather + // than the primary action in the same chip. + primaryAction.tabIndex = -1; + trailingAction.addEventListener('focusout', () => { + primaryAction.tabIndex = 0; + }, {once: true}); + } } diff --git a/chips/internal/multi-action-chip_test.ts b/chips/internal/multi-action-chip_test.ts index aa73c8e79..04a41674b 100644 --- a/chips/internal/multi-action-chip_test.ts +++ b/chips/internal/multi-action-chip_test.ts @@ -25,21 +25,20 @@ class TestMultiActionChip extends MultiActionChip { protected primaryId = 'primary'; - protected override renderContainer(content: unknown) { - return html`
${content}
`; - } - protected override renderPrimaryAction() { return html``; } - protected override renderTrailingAction() { + protected override renderTrailingAction(focusListener: EventListener) { if (this.noTrailingAction) { return nothing; } - return renderRemoveButton( - {ariaLabel: this.ariaLabelRemove, disabled: this.disabled}); + return renderRemoveButton({ + focusListener, + ariaLabel: this.ariaLabelRemove, + disabled: this.disabled + }); } } diff --git a/chips/internal/trailing-icons.ts b/chips/internal/trailing-icons.ts index fb04bc732..f3fd6ba89 100644 --- a/chips/internal/trailing-icons.ts +++ b/chips/internal/trailing-icons.ts @@ -14,22 +14,24 @@ import {Chip} from './chip.js'; interface RemoveButtonProperties { ariaLabel: string; disabled: boolean; + focusListener: EventListener; tabbable?: boolean; } /** @protected */ export function renderRemoveButton( - {ariaLabel, disabled, tabbable = false}: RemoveButtonProperties) { + {ariaLabel, disabled, focusListener, tabbable = false}: + RemoveButtonProperties) { return html`