Skip to content

Commit

Permalink
fix(select): close select on menu close
Browse files Browse the repository at this point in the history
There are some situations where the menu can close when the outer select
is still open and unaware.

For example, clicking the field will open the menu, then clicking it
again will close the menu **but not the select**.

This is because the menu observes outside clicks in order to close
itself. We don't currently observe those closes to update our own state
inside `md-select`, so the UI still shows as if it is open but without a
menu.

The fix here is to always reflect the state of the menu on `closed`.
  • Loading branch information
43081j committed Sep 10, 2023
1 parent 86aaacd commit 6888e92
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 2 deletions.
15 changes: 15 additions & 0 deletions select/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ export class SelectHarness extends Harness<Select> {
protected getField() {
return this.element.renderRoot.querySelector('.field') as Field;
}
protected getMenu() {
return this.element.renderRoot.querySelector('md-menu')!;
}
/**
* Shows the menu and returns the first list item element.
*/
Expand All @@ -46,6 +49,18 @@ export class SelectHarness extends Harness<Select> {
field.click();
}

async clickAndWaitForMenu() {
const menu = this.getMenu();
const menuOpen = menu.open === true;
const waitForMenu = new Promise<void>((resolve) => {
menu.addEventListener(menuOpen ? 'closed' : 'opened', () => {
resolve();
}, {once: true});
});
await this.click();
await waitForMenu;
}

async clickOption(index: number) {
const menu = this.element.renderRoot.querySelector('md-menu')!;
if (!menu.open) {
Expand Down
10 changes: 9 additions & 1 deletion select/internal/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ export abstract class Select extends LitElement {
@opening=${this.handleOpening}
@opened=${this.redispatchEvent}
@closing=${this.redispatchEvent}
@closed=${this.redispatchEvent}
@closed=${this.handleClosed}
@close-menu=${this.handleCloseMenu}
@request-selection=${this.handleRequestSelection}
@request-deselection=${this.handleRequestDeselection}>
Expand Down Expand Up @@ -425,6 +425,14 @@ export abstract class Select extends LitElement {
super.firstUpdated(changed);
}

/**
* Closes the select if the inner menu closed
*/
private handleClosed(e: Event) {
this.open = false;
redispatchEvent(this, e);
}

/**
* Focuses and activates the last selected item upon opening, and resets other
* active items.
Expand Down
31 changes: 30 additions & 1 deletion select/select_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,39 @@ describe('<md-outlined-select>', () => {
const selectEl = root.querySelector('md-outlined-select')!;
await selectEl.updateComplete;

await new SelectHarness(selectEl).clickOption(1);
const harness = new SelectHarness(selectEl);
await harness.clickAndWaitForMenu();
await harness.clickOption(1);

expect(changed).toBeTrue();
});

it('closes select when field re-clicked', async () => {
render(
html`
<md-outlined-select>
<md-select-option selected></md-select-option>
<md-select-option></md-select-option>
</md-outlined-select>`,
root);
const selectEl = root.querySelector('md-outlined-select')!;
await selectEl.updateComplete;

const spanEl = selectEl.shadowRoot!.querySelector<HTMLElement>(
'span.select'
)!;
const menuEl = selectEl.shadowRoot!.querySelector('md-menu')!;

const harness = new SelectHarness(selectEl);
await harness.clickAndWaitForMenu();
expect(spanEl.classList.contains('open')).toBeTrue();
expect(menuEl.open).toBeTrue();

await harness.clickAndWaitForMenu();

expect(menuEl.open).toBeFalse();
expect(spanEl.classList.contains('open')).toBeFalse();
});
});

describe('<md-filled-select>', () => {
Expand Down

0 comments on commit 6888e92

Please sign in to comment.