Skip to content

Commit

Permalink
refactor(overlay): reduce paint/render work when opening a modal
Browse files Browse the repository at this point in the history
  • Loading branch information
@Westbrook
Westbrook committed Jan 4, 2022
1 parent dccb3fe commit c1ba7ef
Show file tree
Hide file tree
Showing 14 changed files with 177 additions and 113 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@
"resolutions": {
"cssnano/**/postcss-calc": "7.0.0"
},
"customElements": "projects/documentation/custom-elements.json",
"customElements": ".storybook/custom-elements.json",
"workspaces": [
"packages/*",
"projects/*"
Expand Down
2 changes: 1 addition & 1 deletion packages/action-menu/src/ActionMenu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class ActionMenu extends ObserveSlotText(PickerBase, 'label') {
];
}

protected get renderButton(): TemplateResult {
protected render(): TemplateResult {
return html`
<sp-action-button
quiet
Expand Down
4 changes: 3 additions & 1 deletion packages/menu/src/Menu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ export class Menu extends SpectrumElement {
private updateCachedMenuItems(): MenuItem[] {
this.cachedChildItems = [];

const slotElements = this.menuSlot.assignedElements({ flatten: true });
const slotElements = this.menuSlot
? this.menuSlot.assignedElements({ flatten: true })
: [];
for (const slotElement of slotElements) {
const childMenuItems: MenuItem[] =
slotElement instanceof MenuItem
Expand Down
2 changes: 1 addition & 1 deletion packages/overlay/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"lit-html"
],
"dependencies": {
"@popperjs/core": "^2.10.2",
"@popperjs/core": "^2.11.0",
"@spectrum-web-components/action-button": "^0.7.1",
"@spectrum-web-components/base": "^0.5.1",
"@spectrum-web-components/shared": "^0.13.1",
Expand Down
84 changes: 46 additions & 38 deletions packages/overlay/src/overlay-stack.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ export class OverlayStack {
private _eventsAreBound = false;

private initTabTrapping(): void {
if (this.trappingInited) return;
this.trappingInited = true;
/* c8 ignore next 4 */
if (this.document.body.shadowRoot) {
this.canTabTrap = false;
Expand All @@ -58,7 +60,6 @@ export class OverlayStack {
}
const root = this.document.body.shadowRoot as ShadowRoot;
root.innerHTML = `
<div id="actual"><slot></slot></div>
<style>
#actual {
position: relative;
Expand All @@ -76,59 +77,72 @@ export class OverlayStack {
top: 0;
left: 0;
position: fixed;
pointer-events: none;
}
[name="open"]::slotted(*) {
pointer-events: all;
}
#holder[hidden] {
display: none !important;
}
#actual[aria-hidden] {
touch-action: none;
#holder {
display: none;
}
#actual[tabindex="-1"] ::slotted(*) {
pointer-events: none; /* just in case? */
#actual[aria-hidden] + #holder {
display: block;
}
</style>
<div id="holder" hidden><slot name="open"></slot></div>
<div id="actual"><slot></slot></div>
<div id="holder"><slot name="open"></slot></div>
`;
this.tabTrapper = root.querySelector('#actual') as HTMLElement;
this.overlayHolder = root.querySelector('#holder') as HTMLElement;
this.tabTrapper.attachShadow({ mode: 'open' });
if (this.tabTrapper.shadowRoot) {
this.tabTrapper.shadowRoot.innerHTML = '<slot></slot>';
}
this.overlayHolder.addEventListener(
'contextmenu',
this.forwardContextmenuEvent,
true
);
requestAnimationFrame(() => {
this.applyBodyMargins();
const observer = new ResizeObserver(() => {
this.applyBodyMargins();
});
observer.observe(document.body);
});
}

private startTabTrapping(): void {
if (!this.trappingInited) {
this.initTabTrapping();
this.trappingInited = true;
private _bodyMarginsApplied = false;

private applyBodyMargins(): void {
const { marginLeft, marginRight, marginTop, marginBottom } =
getComputedStyle(document.body);
const allZero =
parseFloat(marginLeft) === 0 &&
parseFloat(marginRight) === 0 &&
parseFloat(marginTop) === 0 &&
parseFloat(marginBottom) === 0;
if (allZero && !this._bodyMarginsApplied) {
return;
}
this.tabTrapper.style.setProperty(
'--swc-body-margins-inline',
`calc(${marginLeft} + ${marginRight})`
);
this.tabTrapper.style.setProperty(
'--swc-body-margins-block',
`calc(${marginTop} + ${marginBottom})`
);
this._bodyMarginsApplied = !allZero;
}

private startTabTrapping(): void {
this.initTabTrapping();
/* c8 ignore next 3 */
if (!this.canTabTrap) {
return;
}
this.tabTrapper.tabIndex = -1;
this.tabTrapper.addEventListener(
'contextmenu',
this.forwardContextmenuEvent,
true
);
this.tabTrapper.setAttribute('aria-hidden', 'true');
this.overlayHolder.hidden = false;
requestAnimationFrame(() => {
const bodyStyles = getComputedStyle(document.body);
this.tabTrapper.style.setProperty(
'--swc-body-margins-inline',
`calc(${bodyStyles.marginLeft} + ${bodyStyles.marginRight})`
);
this.tabTrapper.style.setProperty(
'--swc-body-margins-block',
`calc(${bodyStyles.marginTop} + ${bodyStyles.marginBottom})`
);
});
}

private stopTabTrapping(): void {
Expand All @@ -137,20 +151,14 @@ export class OverlayStack {
return;
}
this.tabTrapper.removeAttribute('tabindex');
this.tabTrapper.removeEventListener(
'contextmenu',
this.forwardContextmenuEvent,
true
);
this.tabTrapper.removeAttribute('aria-hidden');
this.overlayHolder.hidden = true;
}

private forwardContextmenuEvent = async (
event: MouseEvent
): Promise<void> => {
const topOverlay = this.overlays[this.overlays.length - 1];
if (topOverlay.interaction !== 'modal') {
if (!this.trappingInited || topOverlay.interaction !== 'modal') {
return;
}
event.stopPropagation();
Expand Down
65 changes: 33 additions & 32 deletions packages/picker/src/Picker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
html,
nothing,
PropertyValues,
render,
SizedMixin,
TemplateResult,
} from '@spectrum-web-components/base';
Expand Down Expand Up @@ -132,7 +133,6 @@ export class PickerBase extends SizedMixin(Focusable) {

private closeOverlay?: () => void;

@query('sp-popover')
private popover!: Popover;

protected listRole: 'listbox' | 'menu' = 'listbox';
Expand Down Expand Up @@ -265,11 +265,29 @@ export class PickerBase extends SizedMixin(Focusable) {
this.menuStateResolver();
}

private popoverFragment!: DocumentFragment;

private generatePopover(deprecatedMenu: Menu | null): void {
if (this.popoverFragment) return;

this.popoverFragment = document.createDocumentFragment();
render(this.renderPopover, this.popoverFragment, { host: this });
this.popover = this.popoverFragment.children[0] as Popover;
this.optionsMenu = this.popover.children[1] as Menu;

if (deprecatedMenu) {
console.warn(
`Deprecation Notice: You no longer need to provide an sp-menu child to ${this.tagName.toLowerCase()}. Any styling or attributes on the sp-menu will be ignored.`
);
}
}

private async openMenu(): Promise<void> {
/* c8 ignore next 9 */
let reparentableChildren: Element[] = [];

const deprecatedMenu = this.querySelector('sp-menu');

this.generatePopover(deprecatedMenu);
if (deprecatedMenu) {
reparentableChildren = Array.from(deprecatedMenu.children);
} else {
Expand All @@ -296,7 +314,6 @@ export class PickerBase extends SizedMixin(Focusable) {
});

this.sizePopover(this.popover);
const { popover } = this;
this.addEventListener(
'sp-opened',
async () => {
Expand All @@ -309,18 +326,21 @@ export class PickerBase extends SizedMixin(Focusable) {
},
{ once: true }
);
this.closeOverlay = await Picker.openOverlay(this, 'modal', popover, {
placement: this.placement,
receivesFocus: 'auto',
});
this.closeOverlay = await Picker.openOverlay(
this,
'modal',
this.popover,
{
placement: this.placement,
receivesFocus: 'auto',
}
);
}

protected sizePopover(popover: HTMLElement): void {
if (this.quiet) return;
// only use `this.offsetWidth` when Standard variant
const menuWidth = !this.quiet && `${this.offsetWidth}px`;
if (menuWidth) {
popover.style.setProperty('min-width', menuWidth);
}
popover.style.setProperty('min-width', `${this.offsetWidth}px`);
}

private closeMenu(): void {
Expand Down Expand Up @@ -377,7 +397,7 @@ export class PickerBase extends SizedMixin(Focusable) {

// a helper to throw focus to the button is needed because Safari
// won't include buttons in the tab order even with tabindex="0"
protected get renderButton(): TemplateResult {
protected render(): TemplateResult {
return html`
<span
id="focus-helper"
Expand Down Expand Up @@ -410,12 +430,6 @@ export class PickerBase extends SizedMixin(Focusable) {
super.update(changes);
}

protected render(): TemplateResult {
return html`
${this.renderButton} ${this.renderPopover}
`;
}

protected get dismissHelper(): TemplateResult {
return html`
<div class="visually-hidden">
Expand Down Expand Up @@ -460,6 +474,7 @@ export class PickerBase extends SizedMixin(Focusable) {
protected updateMenuItems(
event?: MenuItemAddedOrUpdatedEvent | MenuItemRemovedEvent
): void {
if (this.open && event?.type === 'sp-menu-item-removed') return;
if (this._willUpdateItems) return;
this._willUpdateItems = true;
if (event?.item === this.selectedItem) {
Expand Down Expand Up @@ -487,20 +502,6 @@ export class PickerBase extends SizedMixin(Focusable) {
});
}

protected firstUpdated(changedProperties: PropertyValues): void {
super.firstUpdated(changedProperties);

// Since the sp-menu gets reparented by the popover, initialize it here
this.optionsMenu = this.shadowRoot.querySelector('sp-menu') as Menu;

const deprecatedMenu = this.querySelector('sp-menu');
if (deprecatedMenu) {
console.warn(
`Deprecation Notice: You no longer need to provide an sp-menu child to ${this.tagName.toLowerCase()}. Any styling or attributes on the sp-menu will be ignored.`
);
}
}

protected updated(changedProperties: PropertyValues): void {
super.updated(changedProperties);
if (
Expand Down
20 changes: 14 additions & 6 deletions packages/picker/test/picker-sync.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
} from '@web/test-runner-commands';
import { iconsOnly } from '../stories/picker.stories.js';
import { sendMouse } from '../../../test/plugins/browser.js';
import type { Popover } from '@spectrum-web-components/popover';

const isMenuActiveElement = function (): boolean {
return document.activeElement instanceof Menu;
Expand Down Expand Up @@ -202,12 +203,18 @@ describe('Picker, sync', () => {
const close = oneEvent(el, 'sp-closed');
item.click();
await close;
// Overlaid content is outside of the context of the Picker element
// and cannot be managed via its updateComplete cycle.
await nextFrame();

expect(el.value, 'first time').to.equal('option-new');

opened = oneEvent(el, 'sp-opened');
el.open = true;
await opened;
// Overlaid content is outside of the context of the Picker element
// and cannot be managed via its updateComplete cycle.
await nextFrame();

expect(el.value, 'second time').to.equal('option-new');
});
Expand Down Expand Up @@ -436,7 +443,7 @@ describe('Picker, sync', () => {
const opened = oneEvent(el, 'sp-opened');
button.click();
await opened;
await elementUpdated(el);
await nextFrame();

expect(el.open).to.be.true;
expect(el.selectedItem?.itemText).to.be.undefined;
Expand All @@ -445,6 +452,7 @@ describe('Picker, sync', () => {
const closed = oneEvent(el, 'sp-closed');
secondItem.click();
await closed;
await nextFrame();

expect(el.open).to.be.false;
expect(el.selectedItem?.itemText).to.equal('Select Inverse');
Expand All @@ -453,7 +461,7 @@ describe('Picker, sync', () => {
const opened2 = oneEvent(el, 'sp-opened');
button.click();
await opened2;
await elementUpdated(el);
await nextFrame();

expect(el.open).to.be.true;
expect(el.selectedItem?.itemText).to.equal('Select Inverse');
Expand All @@ -462,6 +470,7 @@ describe('Picker, sync', () => {
const closed2 = oneEvent(el, 'sp-closed');
firstItem.click();
await closed2;
await nextFrame();

expect(el.open).to.be.false;
expect(el.selectedItem?.itemText).to.equal('Deselect');
Expand Down Expand Up @@ -894,10 +903,9 @@ describe('Picker, sync', () => {
expect(el.open).to.be.false;
});
it('scrolls selected into view on open', async () => {
const popover = el.shadowRoot.querySelector(
'sp-popover'
) as HTMLElement;
popover.style.height = '40px';
(el as unknown as { generatePopover(): void }).generatePopover();
(el as unknown as { popover: Popover }).popover.style.height =
'40px';

const firstItem = el.querySelector(
'sp-menu-item:first-child'
Expand Down
Loading

0 comments on commit c1ba7ef

Please sign in to comment.