Skip to content

Commit

Permalink
fix: ensure that an overlay can be released even if it does not compl…
Browse files Browse the repository at this point in the history 
…ete its fade in animation
  • Loading branch information
@Westbrook
Westbrook committed Nov 18, 2022
1 parent 925af0a commit 4cbb36f
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 14 deletions.
140 changes: 140 additions & 0 deletions packages/menu/test/submenu.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ 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';
import { ActiveOverlay } from '@spectrum-web-components/overlay';

async function styledFixture<T extends Element>(
story: TemplateResult,
Expand Down Expand Up @@ -907,4 +908,143 @@ describe('Submenu', () => {
activeOverlays = document.querySelectorAll('active-overlay');
expect(activeOverlays.length).to.equal(0);
});
it('cleans up submenus that close before they are "open"', async () => {
await sendMouse({
steps: [
{
type: 'move',
position: [-5, -5],
},
],
});
const el = await styledFixture<Menu>(
html`
<sp-menu>
<sp-menu-item class="root-1">
Has submenu
<sp-menu slot="submenu">
<sp-menu-item class="submenu-item-1">
One
</sp-menu-item>
<sp-menu-item class="submenu-item-2">
Two
</sp-menu-item>
<sp-menu-item class="submenu-item-3">
Three
</sp-menu-item>
</sp-menu>
</sp-menu-item>
<sp-menu-item class="root-2">
Has submenu
<sp-menu slot="submenu">
<sp-menu-item class="submenu-item-1">
One
</sp-menu-item>
<sp-menu-item class="submenu-item-2">
Two
</sp-menu-item>
<sp-menu-item class="submenu-item-3">
Three
</sp-menu-item>
</sp-menu>
</sp-menu-item>
</sp-menu>
`
);

await elementUpdated(el);
const rootItem1 = el.querySelector('.root-1') as MenuItem;
const rootItem2 = el.querySelector('.root-2') as MenuItem;
expect(rootItem1.open, 'initially closed 1').to.be.false;
expect(rootItem2.open, 'initially closed 2').to.be.false;

const rootItemBoundingRect1 = rootItem1.getBoundingClientRect();
const rootItemBoundingRect2 = rootItem2.getBoundingClientRect();
let activeOverlay!: ActiveOverlay | null;

// Open the first submenu
await sendMouse({
steps: [
{
type: 'move',
position: [
rootItemBoundingRect1.left +
rootItemBoundingRect1.width / 2,
rootItemBoundingRect1.top +
rootItemBoundingRect1.height / 2,
],
},
],
});
// Open the second submenu, closing the first
await sendMouse({
steps: [
{
type: 'move',
position: [
rootItemBoundingRect2.left +
rootItemBoundingRect2.width / 2,
rootItemBoundingRect2.top +
rootItemBoundingRect2.height / 2,
],
},
],
});
// Open the first submenu, closing the second
await sendMouse({
steps: [
{
type: 'move',
position: [
rootItemBoundingRect1.left +
rootItemBoundingRect1.width / 2,
rootItemBoundingRect1.top +
rootItemBoundingRect1.height / 2,
],
},
],
});
// Open the second submenu, closing the first
await sendMouse({
steps: [
{
type: 'move',
position: [
rootItemBoundingRect2.left +
rootItemBoundingRect2.width / 2,
rootItemBoundingRect2.top +
rootItemBoundingRect2.height / 2,
],
},
],
});
const closed = oneEvent(rootItem2, 'sp-closed');
// Close the second submenu
await sendMouse({
steps: [
{
type: 'move',
position: [
rootItemBoundingRect2.left +
rootItemBoundingRect2.width / 2,
rootItemBoundingRect2.top +
rootItemBoundingRect2.top +
rootItemBoundingRect2.height / 2,
],
},
],
});
activeOverlay = document.querySelector(
'active-overlay'
) as ActiveOverlay;
expect(activeOverlay).to.not.be.null;
await closed;

activeOverlay = document.querySelector(
'active-overlay'
) as ActiveOverlay;
expect(activeOverlay).to.be.null;
expect(rootItem1.open, 'finally closed 1').to.be.false;
expect(rootItem2.open, 'finally closed 2').to.be.false;
});
});
33 changes: 19 additions & 14 deletions packages/overlay/src/ActiveOverlay.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ export class ActiveOverlay extends SpectrumElement {
public virtualTrigger?: VirtualTrigger;
private cleanup?: () => void;

protected childrenReady!: Promise<unknown[]>;
protected contentAnimationPromise: Promise<boolean> = Promise.resolve(true);
protected resolveContentAnimationPromise = (): void => {
return;
};

@property()
public _state = stateTransition();
Expand Down Expand Up @@ -298,19 +301,10 @@ export class ActiveOverlay extends SpectrumElement {
this.updateOverlayPosition
);
}
const actions: Promise<unknown>[] = [];
if (this.placement && this.placement !== 'none') {
actions.push(this.applyContentAnimation('sp-overlay-fade-in'));
}
if (
typeof (this.overlayContent as SpectrumElement).updateComplete !==
'undefined'
) {
actions.push(
(this.overlayContent as SpectrumElement).updateComplete
);
this.contentAnimationPromise =
this.applyContentAnimation('sp-overlay-fade-in');
}
this.childrenReady = Promise.all(actions);
}

public async openCallback(
Expand Down Expand Up @@ -536,6 +530,7 @@ export class ActiveOverlay extends SpectrumElement {
};

public async hide(animated = true): Promise<void> {
if (this.state !== 'active') return;
this.state = 'hiding';
if (animated) {
await this.applyContentAnimation('sp-overlay-fade-out');
Expand Down Expand Up @@ -580,7 +575,11 @@ export class ActiveOverlay extends SpectrumElement {
if (this.placement === 'none') {
return Promise.resolve(true);
}
this.resolveContentAnimationPromise();
return new Promise((resolve): void => {
this.resolveContentAnimationPromise = () => {
resolve(false);
};
const contents = this.shadowRoot.querySelector(
'#contents'
) as HTMLElement;
Expand Down Expand Up @@ -641,8 +640,14 @@ export class ActiveOverlay extends SpectrumElement {
super.getUpdateComplete(),
this.stealOverlayContentPromise,
];
if (this.childrenReady) {
actions.push(this.childrenReady);
actions.push(this.contentAnimationPromise);
if (
typeof (this.overlayContent as SpectrumElement).updateComplete !==
'undefined'
) {
actions.push(
(this.overlayContent as SpectrumElement).updateComplete
);
}
const [complete] = await Promise.all(actions);
return complete as boolean;
Expand Down

0 comments on commit 4cbb36f

Please sign in to comment.