Skip to content

Commit

Permalink
fix: prevent leaving multiple submenus open at a time
Browse files Browse the repository at this point in the history
  • Loading branch information
@Westbrook
Westbrook committed Jun 2, 2022
1 parent 7317a3f commit 6db8f41
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 112 deletions.
55 changes: 4 additions & 51 deletions packages/menu/src/Menu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@ function elementIsOrContains(
return !!isOrContains && (el === isOrContains || el.contains(isOrContains));
}

/* c8 ignore next 3 */
const noop = (): void => {
return;
};

/**
* Spectrum Menu Component
* @element sp-menu
Expand All @@ -70,44 +65,8 @@ export class Menu extends SpectrumElement {
return [menuStyles];
}

public get closeSelfAsSubmenu(): (leave?: boolean) => void {
this.isSubmenu = false;
return this._closeSelfAsSubmenu;
}

public setCloseSelfAsSubmenu(cb: (leave?: boolean) => void): void {
if (cb === this._closeSelfAsSubmenu) {
this.isSubmenu = false;
this._closeSelfAsSubmenu = noop;
return;
}
this.isSubmenu = true;
this._closeSelfAsSubmenu = cb;
}

_closeSelfAsSubmenu = noop;

public isSubmenu = false;

public get closeOpenSubmenu(): (leave?: boolean) => void {
this.hasOpenSubmenu = false;
return this._closeOpenSubmenu;
}

public setCloseOpenSubmenu(cb: (leave?: boolean) => void): void {
if (cb === this._closeOpenSubmenu) {
this.hasOpenSubmenu = false;
this._closeOpenSubmenu = noop;
return;
}
this.hasOpenSubmenu = true;
this._closeOpenSubmenu = cb;
}

_closeOpenSubmenu = noop;

public hasOpenSubmenu = false;

@property({ type: String, reflect: true })
public label = '';

Expand Down Expand Up @@ -342,12 +301,6 @@ export class Menu extends SpectrumElement {
if (target.hasSubmenu || target.open) {
return;
}
if (this.hasOpenSubmenu) {
this.closeOpenSubmenu();
}
if (this.isSubmenu) {
this.closeSelfAsSubmenu();
}
this.selectOrToggleItem(target);
} else {
return;
Expand Down Expand Up @@ -514,10 +467,10 @@ export class Menu extends SpectrumElement {
// Remove focus while opening overlay from keyboard or the visible focus
// will slip back to the first item in the menu.
this.blur();
lastFocusedItem.openOverlay({ immediate: true });
lastFocusedItem.openOverlay();
}
} else if (this.isSubmenu && shouldCloseSelfAsSubmenu) {
this.closeSelfAsSubmenu(true);
} else if (shouldCloseSelfAsSubmenu && this.isSubmenu) {
this.dispatchEvent(new Event('close', { bubbles: true }));
}
}

Expand All @@ -533,7 +486,7 @@ export class Menu extends SpectrumElement {
// Remove focus while opening overlay from keyboard or the visible focus
// will slip back to the first item in the menu.
this.blur();
lastFocusedItem.openOverlay({ immediate: true });
lastFocusedItem.openOverlay();
return;
}
}
Expand Down
42 changes: 7 additions & 35 deletions packages/menu/src/MenuItem.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ export class MenuItem extends LikeAnchor(Focusable) {
public closeOverlay?: (leave?: boolean) => Promise<void>;

protected handleSubmenuClick(): void {
this.openOverlay({ immediate: true });
this.openOverlay();
}

protected handlePointerenter(): void {
Expand Down Expand Up @@ -368,9 +368,7 @@ export class MenuItem extends LikeAnchor(Focusable) {
}
};

public async openOverlay({
immediate,
}: { immediate?: boolean } = {}): Promise<void> {
public async openOverlay(): Promise<void> {
if (!this.hasSubmenu || this.open || this.disabled) {
return;
}
Expand All @@ -393,60 +391,34 @@ export class MenuItem extends LikeAnchor(Focusable) {
const slotName = el.slot;
el.tabIndex = 0;
el.removeAttribute('slot');
el.isSubmenu = true;
return (el) => {
el.tabIndex = -1;
el.slot = slotName;
el.isSubmenu = false;
};
},
});
const closeOverlay = openOverlay(this, 'click', popover, {
placement: this.isLTR ? 'right-start' : 'left-start',
receivesFocus: 'auto',
delayed: !immediate && false,
root: this.menuData.focusRoot,
});
let closing = false;
const closeSubmenu = async (leave = false): Promise<void> => {
const closeSubmenu = async (): Promise<void> => {
delete this.closeOverlay;
if (submenu.hasOpenSubmenu) {
await submenu.closeOpenSubmenu(leave);
}
if (!leave) {
closing = true;
}
this.menuData.focusRoot?.submenuWillCloseOn(this);
(await closeOverlay)();
};
this.closeOverlay = closeSubmenu;
if (this.menuData.focusRoot?.hasOpenSubmenu) {
this.menuData.focusRoot.closeOpenSubmenu(true);
}
const setup = (): void => {
submenu.setCloseSelfAsSubmenu(closeSubmenu);
this.menuData.focusRoot?.setCloseOpenSubmenu(closeSubmenu);
};
const cleanup = (event: CustomEvent<OverlayOpenCloseDetail>): void => {
event.stopPropagation();
returnSubmenu();
submenu.setCloseSelfAsSubmenu(closeSubmenu);
this.menuData.focusRoot?.setCloseOpenSubmenu(closeSubmenu);
this.open = false;
this.active = false;
if (closing || event.detail.reason === 'external-click') {
this.menuData.focusRoot?.dispatchEvent(
new CustomEvent('close', {
bubbles: true,
composed: true,
detail: { reason: 'external-click' },
})
);
}
};
this.addEventListener('sp-opened', setup as EventListener, {
once: true,
});
this.addEventListener('sp-closed', cleanup as EventListener, {
once: true,
});
popover.addEventListener('change', closeSubmenu);
}

updateAriaSelected(): void {
Expand Down
85 changes: 85 additions & 0 deletions packages/menu/test/submenu.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ import { spy } from 'sinon';
import { Theme } from '@spectrum-web-components/theme';
import { TemplateResult } from '@spectrum-web-components/base';
import { sendKeys } from '@web/test-runner-commands';
import { ActionMenu } from '@spectrum-web-components/action-menu';
import '@spectrum-web-components/action-menu/sp-action-menu.js';
import '@spectrum-web-components/menu/sp-menu-group.js';
import '@spectrum-web-components/icons-workflow/icons/sp-icon-show-menu.js';

async function styledFixture<T extends Element>(
story: TemplateResult,
Expand Down Expand Up @@ -602,4 +606,85 @@ describe('Submenu', () => {

expect(rootItem.open).to.be.false;
});
it('closes all decendent submenus when closing a ancestor menu', async () => {
const el = await styledFixture<ActionMenu>(html`
<sp-action-menu>
<sp-icon-show-menu slot="icon"></sp-icon-show-menu>
<sp-menu-group role="none">
<span slot="header">New York</span>
<sp-menu-item>Bronx</sp-menu-item>
<sp-menu-item id="submenu-item-1">
Brooklyn
<sp-menu slot="submenu">
<sp-menu-item id="submenu-item-2">
Ft. Greene
<sp-menu slot="submenu">
<sp-menu-item>S. Oxford St</sp-menu-item>
<sp-menu-item>S. Portland Ave</sp-menu-item>
<sp-menu-item>S. Elliot Pl</sp-menu-item>
</sp-menu>
</sp-menu-item>
<sp-menu-item disabled>Park Slope</sp-menu-item>
<sp-menu-item>Williamsburg</sp-menu-item>
</sp-menu>
</sp-menu-item>
<sp-menu-item id="submenu-item-3">
Manhattan
<sp-menu slot="submenu">
<sp-menu-item disabled>SoHo</sp-menu-item>
<sp-menu-item>
Union Square
<sp-menu slot="submenu">
<sp-menu-item>14th St</sp-menu-item>
<sp-menu-item>Broadway</sp-menu-item>
<sp-menu-item>Park Ave</sp-menu-item>
</sp-menu>
</sp-menu-item>
<sp-menu-item>Upper East Side</sp-menu-item>
</sp-menu>
</sp-menu-item>
</sp-menu-group>
</sp-action-menu>
`);

const rootMenu1 = el.querySelector('#submenu-item-1') as Menu;
const rootMenu2 = el.querySelector('#submenu-item-3') as Menu;
const childMenu2 = el.querySelector('#submenu-item-2') as Menu;

expect(el.open).to.be.false;
let opened = oneEvent(el, 'sp-opened');
el.click();
await opened;
expect(el.open).to.be.true;

let activeOverlays = document.querySelectorAll('active-overlay');
expect(activeOverlays.length).to.equal(1);
opened = oneEvent(rootMenu1, 'sp-opened');
rootMenu1.dispatchEvent(
new PointerEvent('pointerenter', { bubbles: true })
);
await opened;
activeOverlays = document.querySelectorAll('active-overlay');
expect(activeOverlays.length).to.equal(2);

opened = oneEvent(childMenu2, 'sp-opened');
childMenu2.dispatchEvent(
new PointerEvent('pointerenter', { bubbles: true })
);
await opened;
activeOverlays = document.querySelectorAll('active-overlay');
expect(activeOverlays.length).to.equal(3);

const overlaysManaged = Promise.all([
oneEvent(childMenu2, 'sp-closed'),
oneEvent(rootMenu1, 'sp-closed'),
oneEvent(rootMenu2, 'sp-opened'),
]);
rootMenu2.dispatchEvent(
new PointerEvent('pointerenter', { bubbles: true })
);
await overlaysManaged;
activeOverlays = document.querySelectorAll('active-overlay');
expect(activeOverlays.length).to.equal(2);
});
});
15 changes: 3 additions & 12 deletions packages/overlay/src/ActiveOverlay.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import type {
ThemeVariant,
} from '@spectrum-web-components/theme/src/Theme.js';
import styles from './active-overlay.css.js';
import { parentOverlayOf } from './overlay-utils.js';
import {
OverlayOpenCloseDetail,
OverlayOpenDetail,
Expand Down Expand Up @@ -105,18 +106,6 @@ const stateTransition = (
return stateMachine.states[state].on[event] || state;
};

const parentOverlayOf = (el: Element): ActiveOverlay | null => {
const closestOverlay = el.closest('active-overlay');
if (closestOverlay) {
return closestOverlay;
}
const rootNode = el.getRootNode() as ShadowRoot;
if (rootNode.host) {
return parentOverlayOf(rootNode.host);
}
return null;
};

const getFallbackPlacements = (
placement: FloatingUIPlacement
): FloatingUIPlacement[] => {
Expand Down Expand Up @@ -146,6 +135,7 @@ export class ActiveOverlay extends SpectrumElement {
public overlayContent!: HTMLElement;
public overlayContentTip?: HTMLElement;
public trigger!: HTMLElement;
public root?: HTMLElement;
public virtualTrigger?: VirtualTrigger;

protected childrenReady!: Promise<unknown[]>;
Expand Down Expand Up @@ -353,6 +343,7 @@ export class ActiveOverlay extends SpectrumElement {
this.interaction = detail.interaction;
this.theme = detail.theme;
this.receivesFocus = detail.receivesFocus;
this.root = detail.root;
}

public dispose(): void {
Expand Down
Loading

0 comments on commit 6db8f41

Please sign in to comment.