fix(list,menu): clicking items in a list followed by keyboard nav fun…

…ctions as expected

PiperOrigin-RevId: 565539367

list/internal/list.ts

@@ -97,7 +97,12 @@ export class List extends LitElement {
    * The content to be slotted into the list.
    */
   private renderContent() {
-    return html`<span><slot @slotchange=${this.onSlotchange}></slot></span>`;
+    return html`
+      <slot
+          @deactivate-items=${this.onDeactivateItems}
+          @request-activation=${this.onRequestActivation}
+          @slotchange=${this.onSlotchange}>
+      </slot>`;
   }
 
   /**
@@ -225,6 +230,20 @@ export class List extends LitElement {
     return this.activatePreviousItemInternal(items, activeItemRecord);
   }
 
+  private onDeactivateItems() {
+    const items = this.items;
+    for (const item of items) {
+      item.tabIndex = -1;
+    }
+  }
+
+  private onRequestActivation(event: Event) {
+    this.onDeactivateItems();
+    const target = event.target as HTMLElement;
+    target.tabIndex = 0;
+    target.focus();
+  }
+
   /**
    * Activates the first non-disabled item of a given array of items.
    *

list/internal/listitem/list-item-only.ts

@@ -7,7 +7,7 @@
 import {nothing} from 'lit';
 import {property} from 'lit/decorators.js';
 
-import {ListItemEl} from './list-item.js';
+import {createRequestActivationEvent, ListItemEl} from './list-item.js';
 
 // tslint:disable-next-line:enforce-comments-on-exported-symbols
 export class ListItemOnly extends ListItemEl {
@@ -30,4 +30,13 @@ export class ListItemOnly extends ListItemEl {
   override renderFocusRing() {
     return this.noninteractive ? nothing : super.renderFocusRing();
   }
+
+  override onFocus() {
+    if (this.tabIndex !== -1) {
+      return;
+    }
+
+    // Handles the case where the user clicks on the element and then tabs.
+    this.dispatchEvent(createRequestActivationEvent());
+  }
 }

list/internal/listitem/list-item.ts

@@ -15,6 +15,39 @@ import {html as staticHtml, literal} from 'lit/static-html.js';
 import {ARIAMixinStrict} from '../../../internal/aria/aria.js';
 import {requestUpdateOnAriaChange} from '../../../internal/aria/delegate.js';
 
+/**
+ * Creates an event that requests the parent md-list to deactivate all other
+ * items.
+ */
+export function createDeactivateItemsEvent() {
+  return new Event('deactivate-items', {bubbles: true, composed: true});
+}
+
+/**
+ * The type of the event that requests the parent md-list to deactivate all
+ * other items.
+ */
+export type DeactivateItemsEvent =
+    ReturnType<typeof createDeactivateItemsEvent>;
+
+/**
+ * Creates an event that requests the menu to set `tabindex=0` on the item and
+ * focus it. We use this pattern because List keeps track of what element is
+ * active in the List by maintaining tabindex. We do not want list items
+ * to set tabindex on themselves or focus themselves so that we can organize all
+ * that logic in the parent List and Menus, and list item stays as dumb as
+ * possible.
+ */
+export function createRequestActivationEvent() {
+  return new Event('request-activation', {bubbles: true, composed: true});
+}
+
+/**
+ * The type of the event that requests the list activates and focuses the item.
+ */
+export type RequestActivationEvent =
+    ReturnType<typeof createRequestActivationEvent>;
+
 /**
  * Supported roles for a list item.
  */
@@ -133,6 +166,7 @@ export class ListItemEl extends LitElement implements ListItem {
         class="list-item ${classMap(this.getRenderClasses())}"
         href=${this.href || nothing}
         target=${target}
+        @focus=${this.onFocus}
         @click=${this.onClick}
         @mouseenter=${this.onMouseenter}
         @mouseleave=${this.onMouseleave}
@@ -251,6 +285,7 @@ export class ListItemEl extends LitElement implements ListItem {
   protected onKeydown?(event: KeyboardEvent): void;
   protected onMouseenter?(event: Event): void;
   protected onMouseleave?(event: Event): void;
+  protected onFocus?(event: FocusEvent): void;
 
   override focus() {
     // TODO(b/300334509): needed for some cases where delegatesFocus doesn't

list/list_test.ts

@@ -13,7 +13,7 @@ import {html} from 'lit';
 import {Environment} from '../testing/environment.js';
 import {createTokenTests} from '../testing/tokens.js';
 
-import {ListHarness} from './harness.js';
+import {ListHarness, ListItemHarness} from './harness.js';
 import {MdList} from './list.js';
 import {MdListItem} from './list-item.js';
 
@@ -574,11 +574,11 @@ describe('<md-list>', () => {
 
     it('End will select the last item if it is already selected', async () => {
       const root = env.render(html`
-        <md-list>
-          <md-list-item tabindex="-1"></md-list-item>
-          <md-list-item tabindex="-1"></md-list-item>
-          <md-list-item tabindex="0"></md-list-item>
-        </md-list>`);
+          <md-list>
+            <md-list-item tabindex="-1"></md-list-item>
+            <md-list-item tabindex="-1"></md-list-item>
+            <md-list-item tabindex="0"></md-list-item>
+          </md-list>`);
       const listEl = root.querySelector('md-list')!;
       const listHarness = new ListHarness(listEl);
       const [first, second, third] =
@@ -598,6 +598,32 @@ describe('<md-list>', () => {
       expect(third.tabIndex).toEqual(0);
       expect(document.activeElement).toEqual(third);
     });
+
+    it('Clicking on an item will activate it', async () => {
+      const root = env.render(html`
+          <md-list>
+            <md-list-item tabindex="-1"></md-list-item>
+            <md-list-item tabindex="-1"></md-list-item>
+            <md-list-item tabindex="0"></md-list-item>
+          </md-list>`);
+      const [first, second, third] =
+          Array.from(root.querySelectorAll('md-list-item'));
+
+      await env.waitForStability();
+
+      expect(first.tabIndex).toEqual(-1);
+      expect(second.tabIndex).toEqual(-1);
+      expect(third.tabIndex).toEqual(0);
+
+      const secondHarness = new ListItemHarness(second);
+
+      await secondHarness.clickWithMouse();
+      await env.waitForStability();
+
+      expect(first.tabIndex).toEqual(-1);
+      expect(second.tabIndex).toEqual(0);
+      expect(third.tabIndex).toEqual(-1);
+    });
   });
 
   describe('activate items methods', () => {

menu/internal/menu.ts

@@ -319,8 +319,8 @@ export abstract class Menu extends LitElement {
   private renderMenuItems() {
     return html`<slot
         @close-menu=${this.onCloseMenu}
-        @request-activation=${this.onRequestActivation}
         @deactivate-items=${this.onDeactivateItems}
+        @request-activation=${this.onRequestActivation}
         @deactivate-typeahead=${this.handleDeactivateTypeahead}
         @activate-typeahead=${this.handleActivateTypeahead}
         @stay-open-on-focusout=${this.handleStayOpenOnFocusout}
@@ -716,16 +716,13 @@ export abstract class Menu extends LitElement {
     event.stopPropagation();
     const items = this.items;
     for (const item of items) {
-      item.tabIndex = -1;
       item.selected = false;
     }
   }
 
   private onRequestActivation(event: Event) {
     event.stopPropagation();
-    const target = event.target as HTMLElement;
-    target.tabIndex = 0;
-    target.focus();
+    this.onDeactivateItems(event);
   }
 
   private handleDeactivateTypeahead(event: DeactivateTypeaheadEvent) {

menu/internal/shared.ts

@@ -106,39 +106,6 @@ export const createDefaultCloseMenuEvent = createCloseMenuEvent<DefaultReasons>;
 export type CloseMenuEvent<T extends Reason = DefaultReasons> =
     ReturnType<typeof createCloseMenuEvent<T>>;
 
-/**
- * Creates an event that requests the parent md-menu to deactivate all other
- * items.
- */
-export function createDeactivateItemsEvent() {
-  return new Event('deactivate-items', {bubbles: true, composed: true});
-}
-
-/**
- * The type of the event that requests the parent md-menu to deactivate all
- * other items.
- */
-export type DeactivateItemsEvent =
-    ReturnType<typeof createDeactivateItemsEvent>;
-
-/**
- * Creates an event that requests the menu to set `tabindex=0` on the item and
- * focus it. We use this pattern because List keeps track of what element is
- * active in the List by maintaining tabindex. We do not want list items
- * to set tabindex on themselves or focus themselves so that we can organize all
- * that logic in the parent List and Menus, and list item stays as dumb as
- * possible.
- */
-export function createRequestActivationEvent() {
-  return new Event('request-activation', {bubbles: true, composed: true});
-}
-
-/**
- * The type of the event that requests the menu activates and focuses the item.
- */
-export type RequestActivationEvent =
-    ReturnType<typeof createRequestActivationEvent>;
-
 /**
  * Creates an event that requests the given item be selected.
  */

menu/internal/submenuitem/sub-menu-item.ts

@@ -8,10 +8,11 @@ import {html, PropertyValues} from 'lit';
 import {property, query, queryAssignedElements, state} from 'lit/decorators.js';
 
 import {List} from '../../../list/internal/list.js';
+import {createDeactivateItemsEvent, createRequestActivationEvent} from '../../../list/internal/listitem/list-item.js';
 import {MdRipple} from '../../../ripple/ripple.js';
 import {Corner, Menu} from '../menu.js';
 import {MenuItemEl} from '../menuitem/menu-item.js';
-import {CLOSE_REASON, CloseMenuEvent, createActivateTypeaheadEvent, createDeactivateItemsEvent, createDeactivateTypeaheadEvent, createRequestActivationEvent, KEYDOWN_CLOSE_KEYS, NAVIGABLE_KEY, SELECTION_KEY} from '../shared.js';
+import {CLOSE_REASON, CloseMenuEvent, createActivateTypeaheadEvent, createDeactivateTypeaheadEvent, KEYDOWN_CLOSE_KEYS, NAVIGABLE_KEY, SELECTION_KEY} from '../shared.js';
 
 /**
  * @fires deactivate-items Requests the parent menu to deselect other items when
@@ -205,7 +206,6 @@ export class SubMenuItem extends MenuItemEl {
     if (reason.kind === CLOSE_REASON.KEYDOWN &&
         reason.key === KEYDOWN_CLOSE_KEYS.ESCAPE) {
       event.stopPropagation();
-      this.dispatchEvent(createDeactivateItemsEvent());
       this.dispatchEvent(createRequestActivationEvent());
       return;
     }