From fcdb126f2fcccf696e199e617c5df3f696e79c4b Mon Sep 17 00:00:00 2001
From: Elizabeth Mitchell <lizmitchell@google.com>
Date: Wed, 14 Jun 2023 14:41:30 -0700
Subject: [PATCH] fix(chips): add aria grid/listbox models to chip set

PiperOrigin-RevId: 540384126
---
 chips/demo/stories.ts          | 22 ++++++++++++++-------
 chips/lib/_chip-set.scss       |  6 ++++++
 chips/lib/_shared.scss         |  4 ++++
 chips/lib/chip-set.ts          | 28 +++++++++++++++++++++++++--
 chips/lib/chip.ts              | 34 ++++++++++++++++++++++++++-------
 chips/lib/filter-chip.ts       | 16 +++++++++++++---
 chips/lib/input-chip.ts        |  7 +++++--
 chips/lib/multi-action-chip.ts | 35 +++++++++++++++++++++++++++++-----
 chips/lib/trailing-icons.ts    |  9 ++++++++-
 9 files changed, 134 insertions(+), 27 deletions(-)

diff --git a/chips/demo/stories.ts b/chips/demo/stories.ts
index 37e4502ad4..b9252f88d0 100644
--- a/chips/demo/stories.ts
+++ b/chips/demo/stories.ts
@@ -50,7 +50,8 @@ const standard: MaterialStoryInit<StoryKnobs> = {
   render({label, elevated, disabled, scrolling}) {
     const classes = {scrolling};
     return html`
-      <md-chip-set class=${classMap(classes)}>
+      <md-chip-set type="assist" class=${classMap(classes)}
+          aria-label="Assist chips">
         <md-assist-chip
           label=${label || 'Assist chip'}
           ?disabled=${disabled}
@@ -74,7 +75,8 @@ const links: MaterialStoryInit<StoryKnobs> = {
   render({label, elevated, disabled, scrolling}) {
     const classes = {scrolling};
     return html`
-      <md-chip-set class=${classMap(classes)}>
+      <md-chip-set type="assist" class=${classMap(classes)}
+          aria-label="Assist link chips">
         <md-assist-chip
           label=${label || 'Assist link chip'}
           ?disabled=${disabled}
@@ -93,7 +95,9 @@ const filters: MaterialStoryInit<StoryKnobs> = {
   render({label, elevated, disabled, scrolling, singleSelect}) {
     const classes = {scrolling};
     return html`
-      <md-chip-set class=${classMap(classes)} ?single-select=${singleSelect}>
+      <md-chip-set type="filter" class=${classMap(classes)}
+          aria-label="Filter chips"
+          ?single-select=${singleSelect}>
         <md-filter-chip
           label=${label || 'Filter chip'}
           ?disabled=${disabled}
@@ -123,7 +127,8 @@ const inputs: MaterialStoryInit<StoryKnobs> = {
   render({label, disabled, scrolling}) {
     const classes = {scrolling};
     return html`
-      <md-chip-set class=${classMap(classes)}>
+      <md-chip-set type="input" class=${classMap(classes)}
+          aria-label="Input chips">
         <md-input-chip
           label=${label || 'Input chip'}
           ?disabled=${disabled}
@@ -157,7 +162,8 @@ const inputLinks: MaterialStoryInit<StoryKnobs> = {
   render({label, disabled, scrolling}) {
     const classes = {scrolling};
     return html`
-      <md-chip-set class=${classMap(classes)}>
+      <md-chip-set type="input" class=${classMap(classes)}
+          aria-label="Input link chips">
         <md-input-chip
           label=${label || 'Input link chip'}
           ?disabled=${disabled}
@@ -175,7 +181,8 @@ const suggestions: MaterialStoryInit<StoryKnobs> = {
   render({label, elevated, disabled, scrolling}) {
     const classes = {scrolling};
     return html`
-      <md-chip-set class=${classMap(classes)}>
+      <md-chip-set type="suggestion" class=${classMap(classes)}
+          aria-label="Suggestion chips">
         <md-suggestion-chip
           label=${label || 'Suggestion chip'}
           ?disabled=${disabled}
@@ -199,7 +206,8 @@ const suggestionLinks: MaterialStoryInit<StoryKnobs> = {
   render({label, elevated, disabled, scrolling}) {
     const classes = {scrolling};
     return html`
-      <md-chip-set class=${classMap(classes)}>
+      <md-chip-set type="suggestion" class=${classMap(classes)}
+          aria-label="Suggestion link chips">
         <md-suggestion-chip
           label=${label || 'Suggestion link chip'}
           ?disabled=${disabled}
diff --git a/chips/lib/_chip-set.scss b/chips/lib/_chip-set.scss
index d52a4c19a7..aae69bf7bf 100644
--- a/chips/lib/_chip-set.scss
+++ b/chips/lib/_chip-set.scss
@@ -9,4 +9,10 @@
     flex-wrap: wrap;
     gap: 8px;
   }
+
+  .content {
+    display: flex;
+    flex-wrap: inherit;
+    gap: inherit;
+  }
 }
diff --git a/chips/lib/_shared.scss b/chips/lib/_shared.scss
index 1cf3b984da..46088a3cbb 100644
--- a/chips/lib/_shared.scss
+++ b/chips/lib/_shared.scss
@@ -66,6 +66,10 @@
     pointer-events: none;
   }
 
+  .cell {
+    display: flex;
+  }
+
   .action {
     align-items: baseline;
     appearance: none;
diff --git a/chips/lib/chip-set.ts b/chips/lib/chip-set.ts
index 24c0dbd9ba..94e12d97d4 100644
--- a/chips/lib/chip-set.ts
+++ b/chips/lib/chip-set.ts
@@ -4,20 +4,33 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import {html, isServer, LitElement, PropertyValues} from 'lit';
+import {html, isServer, LitElement, nothing, PropertyValues} from 'lit';
 import {property, queryAssignedElements} from 'lit/decorators.js';
 
+import {ARIAMixinStrict} from '../../internal/aria/aria.js';
+import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.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(this);
+  }
+
   get chips() {
     return this.childElements.filter(
         (child): child is Chip => child instanceof Chip);
   }
 
+  @property() type: ChipSetType = '';
   @property({type: Boolean, attribute: 'single-select'}) singleSelect = false;
 
   @queryAssignedElements({flatten: true})
@@ -49,7 +62,18 @@ export class ChipSet extends LitElement {
   }
 
   protected override render() {
-    return html`<slot @slotchange=${this.updateTabIndices}></slot>`;
+    const {ariaLabel} = this as ARIAMixinStrict;
+    const isFilter = this.type === 'filter';
+    const role = isFilter ? 'listbox' : 'grid';
+    const multiselectable = isFilter ? !this.singleSelect : nothing;
+    return html`
+      <div class="content"
+          role=${role}
+          aria-label=${ariaLabel || nothing}
+          aria-multiselectable=${multiselectable}>
+        <slot @slotchange=${this.updateTabIndices}></slot>
+      </div>
+    `;
   }
 
   private handleKeyDown(event: KeyboardEvent) {
diff --git a/chips/lib/chip.ts b/chips/lib/chip.ts
index f55520b835..bd36427642 100644
--- a/chips/lib/chip.ts
+++ b/chips/lib/chip.ts
@@ -7,8 +7,8 @@
 import '../../focus/focus-ring.js';
 import '../../ripple/ripple.js';
 
-import {html, LitElement, TemplateResult} from 'lit';
-import {property} from 'lit/decorators.js';
+import {html, LitElement, nothing, TemplateResult} from 'lit';
+import {property, state} from 'lit/decorators.js';
 import {classMap} from 'lit/directives/class-map.js';
 
 import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';
@@ -43,14 +43,21 @@ export abstract class Chip extends LitElement {
     return this.disabled;
   }
 
+  /**
+   * The aria role of the container. Defaults to `row` for grid chip sets.
+   * Listbox chip sets should remove this since they do not contain cells.
+   */
+  @state() protected containerRole?: 'row' = 'row';
+
   protected override render() {
     return html`
-      <div class="container ${classMap(this.getContainerClasses())}">
+      <div class="container ${classMap(this.getContainerClasses())}"
+          role=${this.containerRole || nothing}>
         ${this.renderOutline()}
         <md-focus-ring for=${this.primaryId}></md-focus-ring>
         <md-ripple for=${this.primaryId}
           ?disabled=${this.rippleDisabled}></md-ripple>
-        ${this.renderAction()}
+        ${this.renderActions()}
       </div>
     `;
   }
@@ -61,9 +68,24 @@ export abstract class Chip extends LitElement {
     };
   }
 
+  protected renderActions() {
+    return this.renderActionCell(this.renderAction());
+  }
+
+  protected renderActionCell(content: TemplateResult|
+                             typeof nothing): TemplateResult|typeof nothing {
+    if (content === nothing) {
+      return content;
+    }
+
+    return html`<div class="cell" role="cell">${content}</div>`;
+  }
+
+  protected abstract renderAction(): TemplateResult;
+
   protected renderContent() {
     return html`
-      <span class="leading icon">
+      <span class="leading icon" aria-hidden="true">
         ${this.renderLeadingIcon()}
       </span>
       <span class="label">${this.label}</span>
@@ -71,8 +93,6 @@ export abstract class Chip extends LitElement {
     `;
   }
 
-  protected abstract renderAction(): TemplateResult;
-
   protected renderOutline() {
     return html`<span class="outline"></span>`;
   }
diff --git a/chips/lib/filter-chip.ts b/chips/lib/filter-chip.ts
index 75745dd1a4..f759cb8b37 100644
--- a/chips/lib/filter-chip.ts
+++ b/chips/lib/filter-chip.ts
@@ -6,7 +6,7 @@
 
 import '../../elevation/elevation.js';
 
-import {html, nothing, PropertyValues, svg} from 'lit';
+import {html, nothing, PropertyValues, svg, TemplateResult} from 'lit';
 import {property, query} from 'lit/decorators.js';
 
 import {ARIAMixinStrict} from '../../internal/aria/aria.js';
@@ -32,6 +32,9 @@ export class FilterChip extends MultiActionChip {
 
   constructor() {
     super();
+    // Remove the `row` role from the container, since filter chips do not use a
+    // `grid` navigation model.
+    this.containerRole = undefined;
     this.addEventListener('click', () => {
       if (this.disabled) {
         return;
@@ -56,7 +59,13 @@ export class FilterChip extends MultiActionChip {
     };
   }
 
-  protected override renderPrimaryAction() {
+  protected override renderActionCell(content: TemplateResult|typeof nothing) {
+    // Filter chips use a `listbox`/`option` model, and do not need `gridcell`
+    // wrappers around their actions.
+    return content;
+  }
+
+  protected override renderAction() {
     const {ariaLabel} = this as ARIAMixinStrict;
     return html`
       <button class="primary action"
@@ -83,7 +92,8 @@ export class FilterChip extends MultiActionChip {
 
   protected override renderTrailingAction() {
     if (this.removable) {
-      return renderRemoveButton({disabled: this.disabled});
+      return renderRemoveButton(
+          {ariaLabel: this.ariaLabelRemove, disabled: this.disabled});
     }
 
     return nothing;
diff --git a/chips/lib/input-chip.ts b/chips/lib/input-chip.ts
index 5c9232234e..16ba68c3df 100644
--- a/chips/lib/input-chip.ts
+++ b/chips/lib/input-chip.ts
@@ -71,7 +71,7 @@ export class InputChip extends MultiActionChip {
     };
   }
 
-  protected override renderPrimaryAction() {
+  protected override renderAction() {
     const {ariaLabel} = this as ARIAMixinStrict;
     if (this.href) {
       return html`
@@ -103,6 +103,9 @@ export class InputChip extends MultiActionChip {
   }
 
   protected override renderTrailingAction() {
-    return renderRemoveButton({disabled: this.disabled});
+    return renderRemoveButton({
+      ariaLabel: this.ariaLabelRemove,
+      disabled: this.disabled,
+    });
   }
 }
diff --git a/chips/lib/multi-action-chip.ts b/chips/lib/multi-action-chip.ts
index 9d01b90a2f..664073fb2d 100644
--- a/chips/lib/multi-action-chip.ts
+++ b/chips/lib/multi-action-chip.ts
@@ -6,12 +6,39 @@
 
 import {html, isServer, nothing, TemplateResult} from 'lit';
 
+import {ARIAMixinStrict} from '../../internal/aria/aria.js';
+
 import {Chip} from './chip.js';
 
+const ARIA_LABEL_REMOVE = 'aria-label-remove';
+
 /**
  * A chip component with multiple actions.
  */
 export abstract class MultiActionChip extends Chip {
+  get ariaLabelRemove(): string {
+    if (this.hasAttribute(ARIA_LABEL_REMOVE)) {
+      return this.getAttribute(ARIA_LABEL_REMOVE)!;
+    }
+
+    const {ariaLabel} = this as ARIAMixinStrict;
+    return `Remove ${ariaLabel || this.label}`;
+  }
+  set ariaLabelRemove(ariaLabel: string|null) {
+    const prev = this.ariaLabelRemove;
+    if (ariaLabel === prev) {
+      return;
+    }
+
+    if (ariaLabel === null) {
+      this.removeAttribute(ARIA_LABEL_REMOVE);
+    } else {
+      this.setAttribute(ARIA_LABEL_REMOVE, ariaLabel);
+    }
+
+    this.requestUpdate();
+  }
+
   protected abstract readonly primaryAction: HTMLElement|null;
   protected abstract readonly trailingAction: HTMLElement|null;
 
@@ -37,15 +64,13 @@ export abstract class MultiActionChip extends Chip {
     this.updateTabIndices();
   }
 
-  protected override renderAction() {
+  protected override renderActions() {
     return html`
-      ${this.renderPrimaryAction()}
-      ${this.renderTrailingAction()}
+      ${super.renderActions()}
+      ${this.renderActionCell(this.renderTrailingAction())}
     `;
   }
 
-  protected abstract renderPrimaryAction(): TemplateResult;
-
   protected abstract renderTrailingAction(): TemplateResult|typeof nothing;
 
   private handleKeyDown(event: KeyboardEvent) {
diff --git a/chips/lib/trailing-icons.ts b/chips/lib/trailing-icons.ts
index bbe1194f9e..c3ede174d2 100644
--- a/chips/lib/trailing-icons.ts
+++ b/chips/lib/trailing-icons.ts
@@ -11,10 +11,17 @@ import {html} from 'lit';
 
 import {Chip} from './chip.js';
 
+interface RemoveButtonProperties {
+  ariaLabel: string;
+  disabled: boolean;
+}
+
 /** @protected */
-export function renderRemoveButton({disabled}: {disabled: boolean}) {
+export function renderRemoveButton(
+    {ariaLabel, disabled}: RemoveButtonProperties) {
   return html`
     <button class="trailing action"
+      aria-label=${ariaLabel}
       ?disabled=${disabled}
       @click=${handleRemoveClick}
     >