Skip to content

Commit

Permalink
fix(menu): properly implement selected state
Browse files Browse the repository at this point in the history
Before, I set the focused, actively-keyboard-interacted, or pressed element as visual selected state. That doesn't seem to be how GMDC handles it and created problems down the line with md-select where we could not show the currently-selected item + keyboard navigation at the same time.

This CL brings the behavior in line with GMDC and fixes this problem down the line for select.

Also in this CL I add the selected state to sub-menu-item when it's open

PiperOrigin-RevId: 519166067
  • Loading branch information
material-web-copybara authored and copybara-github committed Mar 24, 2023
1 parent f034461 commit bfa1bec
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 6 deletions.
6 changes: 3 additions & 3 deletions list/lib/listitem/list-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ export class ListItemEl extends LitElement implements ListItem {
@property({type: Number}) itemTabIndex = -1;

/**
* Whether or not the element is in the selected visual state. When active,
* tabindex is set to 0, and in some list item variants (like md-list-item),
* focuses the underlying item.
* Whether or not the element is actively being interacted with by md-list.
* When active, tabindex is set to 0, and in some list item variants (like
* md-list-item), focuses the underlying item.
*/
@property({type: Boolean, reflect: true}) active = false;

Expand Down
4 changes: 1 addition & 3 deletions menu/lib/menuitem/_menu-item.scss
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ $_custom-property-prefix: 'menu';
}

.list-item {
:host([active]) &,
:host(:active) &,
&:focus {
:host([selected]) & {
background-color: var(--_list-item-selected-container-color);
}
}
Expand Down
8 changes: 8 additions & 0 deletions menu/lib/submenuitem/sub-menu-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ export class SubMenuItem extends MenuItemEl {
*/
@property({type: Number, attribute: 'hover-close-delay'})
hoverCloseDelay = 400;
/**
* Sets the item in the selected visual state when a submenu is opened.
*/
@property({type: Boolean, reflect: true}) selected = false;

@queryAssignedElements({slot: 'submenu', flatten: true})
protected menus!: Menu[];
Expand Down Expand Up @@ -157,12 +161,14 @@ export class SubMenuItem extends MenuItemEl {
e.reason.key === KEYDOWN_CLOSE_KEYS.ESCAPE) {
e.stopPropagation();
this.active = true;
this.selected = false;
// It might already be active so manually focus
this.listItemRoot.focus();
return;
}

this.active = false;
this.selected = false;
}

protected async onSubMenuKeydown(e: KeyboardEvent) {
Expand Down Expand Up @@ -212,6 +218,7 @@ export class SubMenuItem extends MenuItemEl {
this.dispatchEvent(new DeactivateItemsEvent());
this.dispatchEvent(new DeactivateTypeaheadEvent());
this.active = true;
this.selected = true;

// This is the case of mouse hovering when already opened via keyboard or
// vice versa
Expand All @@ -235,6 +242,7 @@ export class SubMenuItem extends MenuItemEl {
menu.quick = true;
menu.close();
this.active = false;
this.selected = false;
menu.addEventListener('closed', onClosed, {once: true});
}

Expand Down

0 comments on commit bfa1bec

Please sign in to comment.