Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ executors:
parameters:
current_golden_images_hash:
type: string
default: dd3a2e0c32c10a166d37b82df17bebb9f2b1b17a
default: c5094224659a6e73ec66f0bd28731c309fab5136
commands:
downstream:
steps:
Expand Down
3 changes: 3 additions & 0 deletions .storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ const rollupJson = require('@rollup/plugin-json');

module.exports = {
stories: ['../packages/*/stories/*.stories.js'],
nodeResolve: {
exportConditions: ['browser', 'development'],
},

rollupConfig(config) {
// add a new plugin to the build
Expand Down
10 changes: 9 additions & 1 deletion .storybook/preview-head.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
<!-- NOTE: you'll need to rebuild storybook to test this locally.
<link rel="stylesheet" href="https://use.typekit.net/yfg6hha.css" />
-->

<style>
body {
margin: 0;
Expand All @@ -31,3 +30,12 @@
min-height: 200px;
}
</style>
<script>
// Needed while tachometer does not surface support for export conditions.
// @floating-ui/core leverages `process.env.NODE_ENV` in their 'default' export.
window.process = window.process || {
env: {
NODE_ENV: 'production',
},
};
</script>
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@
"node-fetch": "^3.1.0",
"npm-run-all": "^4.1.5",
"patch-package": "^6.4.7",
"playwright": "^1.16.3",
"playwright": "^1.18.1",
"postcss": "^8.4.5",
"postcss-custom-properties": "^12.1.2",
"postcss-focus-visible": "^6.0.3",
Expand Down
50 changes: 47 additions & 3 deletions packages/dialog/src/DialogWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import '@spectrum-web-components/underlay/sp-underlay.js';
import '@spectrum-web-components/button/sp-button.js';

import '../sp-dialog.js';
import styles from '@spectrum-web-components/modal/src/modal.css.js';
import modalWrapperStyles from '@spectrum-web-components/modal/src/modal-wrapper.css.js';
import modalStyles from '@spectrum-web-components/modal/src/modal.css.js';
import { Dialog } from './Dialog.js';
import { FocusVisiblePolyfillMixin } from '@spectrum-web-components/shared';
import { firstFocusableIn } from '@spectrum-web-components/shared/src/first-focusable-in.js';
Expand All @@ -43,7 +44,7 @@ import { firstFocusableIn } from '@spectrum-web-components/shared/src/first-focu
*/
export class DialogWrapper extends FocusVisiblePolyfillMixin(SpectrumElement) {
public static get styles(): CSSResultArray {
return [styles];
return [modalWrapperStyles, modalStyles];
}

@property({ type: Boolean, reflect: true })
Expand Down Expand Up @@ -88,6 +89,10 @@ export class DialogWrapper extends FocusVisiblePolyfillMixin(SpectrumElement) {
@property({ type: Boolean })
public responsive = false;

private transitionPromise = Promise.resolve();

private resolveTransitionPromise!: () => void;

@property({ type: Boolean })
public underlay = false;

Expand Down Expand Up @@ -156,17 +161,42 @@ export class DialogWrapper extends FocusVisiblePolyfillMixin(SpectrumElement) {
);
}

protected handleUnderlayTransitionend(): void {
if (!this.open) {
this.resolveTransitionPromise();
}
}

protected handleModalTransitionend(): void {
if (this.open || !this.underlay) {
this.resolveTransitionPromise();
}
}

protected update(changes: PropertyValues<this>): void {
if (changes.has('open') && changes.get('open') !== undefined) {
this.transitionPromise = new Promise(
(res) => (this.resolveTransitionPromise = res)
);
}
super.update(changes);
}

protected render(): TemplateResult {
return html`
${this.underlay
? html`
<sp-underlay
?open=${this.open}
@click=${this.dismiss}
@transitionend=${this.handleUnderlayTransitionend}
></sp-underlay>
`
: html``}
<div class="modal ${this.mode}">
<div
class="modal ${this.mode}"
@transitionend=${this.handleModalTransitionend}
>
<sp-dialog
?dismissable=${this.dismissable}
?no-divider=${this.noDivider}
Expand Down Expand Up @@ -247,4 +277,18 @@ export class DialogWrapper extends FocusVisiblePolyfillMixin(SpectrumElement) {
});
}
}

/**
* Bind the open/close transition into the update complete lifecycle so
* that the overlay system can wait for it to be "visibly ready" before
* attempting to throw focus into the content contained herein. Not
* waiting for this can cause small amounts of page scroll to happen
* while opening the Tray when focusable content is included: e.g. Menu
* elements whose selected Menu Item is not the first Menu Item.
*/
protected async getUpdateComplete(): Promise<boolean> {
const complete = (await super.getUpdateComplete()) as boolean;
await this.transitionPromise;
return complete;
}
}
55 changes: 44 additions & 11 deletions packages/dialog/test/dialog-wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,54 +10,87 @@ OF ANY KIND, either express or implied. See the License for the specific languag
governing permissions and limitations under the License.
*/

import { elementUpdated, expect, fixture } from '@open-wc/testing';
import { elementUpdated, expect, fixture, oneEvent } from '@open-wc/testing';
import { spy } from 'sinon';

import '@spectrum-web-components/theme/sp-theme.js';
import '@spectrum-web-components/theme/src/themes.js';
import '../sp-dialog-wrapper.js';
import { Dialog, DialogWrapper } from '../';
import { ActionButton } from '@spectrum-web-components/action-button';
import { Button } from '@spectrum-web-components/button';
import { Underlay } from '@spectrum-web-components/underlay';
import {
longContent,
wrapperButtons,
wrapperButtonsUnderlay,
wrapperDismissable,
wrapperDismissableUnderlayError,
wrapperFullscreen,
wrapperLabeledHero,
} from '../stories/dialog-wrapper.stories.js';
import { OverlayTrigger } from '@spectrum-web-components/overlay';
import { html, TemplateResult } from '@spectrum-web-components/base';
import { Theme } from '@spectrum-web-components/theme';

async function styledFixture<T extends Element>(
story: TemplateResult
): Promise<T> {
const test = await fixture<Theme>(html`
<sp-theme scale="medium" color="dark">${story}</sp-theme>
`);
return test.children[0] as T;
}

describe('Dialog Wrapper', () => {
it('loads wrapped dialog accessibly', async () => {
const el = await fixture<DialogWrapper>(wrapperDismissable());
const el = await styledFixture<DialogWrapper>(wrapperDismissable());

await elementUpdated(el);

await expect(el).to.be.accessible();
});
it('loads labeled hero dialog accessibly', async () => {
const el = await fixture<DialogWrapper>(wrapperLabeledHero());
const el = await styledFixture<DialogWrapper>(wrapperLabeledHero());

await elementUpdated(el);

await expect(el).to.be.accessible();
});
it('loads fullscreen wrapped dialog accessibly', async () => {
const el = await fixture<DialogWrapper>(wrapperFullscreen());
const el = await styledFixture<DialogWrapper>(wrapperFullscreen());

await elementUpdated(el);

await expect(el).to.be.accessible();
});
it('loads with underlay and no headline accessibly', async () => {
const el = await fixture<DialogWrapper>(wrapperButtonsUnderlay());
const el = await styledFixture<DialogWrapper>(wrapperButtonsUnderlay());
await elementUpdated(el);
el.headline = '';
await elementUpdated(el);
expect(el).to.be.accessible();
});
it('opens and closes', async () => {
const test = await styledFixture<OverlayTrigger>(longContent());
const el = test.querySelector('sp-dialog-wrapper') as DialogWrapper;

await elementUpdated(el);

const opened = oneEvent(test, 'sp-opened');
test.open = 'click';
await opened;

expect(el.open).to.be.true;

const closed = oneEvent(test, 'sp-closed');
test.open = undefined;
await closed;

expect(el.open).to.be.false;
});
it('dismisses via clicking the underlay when [dismissable]', async () => {
const test = await fixture<HTMLDivElement>(
const test = await styledFixture<DialogWrapper>(
wrapperDismissableUnderlayError()
);
const el = test.querySelector('sp-dialog-wrapper') as DialogWrapper;
Expand All @@ -70,7 +103,7 @@ describe('Dialog Wrapper', () => {
expect(el.open).to.be.false;
});
it('does not dismiss via clicking the underlay :not([dismissable])', async () => {
const el = await fixture<DialogWrapper>(wrapperButtonsUnderlay());
const el = await styledFixture<DialogWrapper>(wrapperButtonsUnderlay());
await elementUpdated(el);
expect(el.open).to.be.true;
const underlay = el.shadowRoot.querySelector('sp-underlay') as Underlay;
Expand All @@ -79,7 +112,7 @@ describe('Dialog Wrapper', () => {
expect(el.open).to.be.true;
});
it('dismisses', async () => {
const el = await fixture<DialogWrapper>(wrapperDismissable());
const el = await styledFixture<DialogWrapper>(wrapperDismissable());

await elementUpdated(el);
expect(el.open).to.be.true;
Expand All @@ -96,7 +129,7 @@ describe('Dialog Wrapper', () => {
expect(el.open).to.be.false;
});
it('manages entry focus - dismissable', async () => {
const el = await fixture<DialogWrapper>(wrapperDismissable());
const el = await styledFixture<DialogWrapper>(wrapperDismissable());

await elementUpdated(el);
expect(el.open).to.be.true;
Expand All @@ -122,7 +155,7 @@ describe('Dialog Wrapper', () => {
expect(el.open).to.be.false;
});
it('manages entry focus - buttons', async () => {
const el = await fixture<DialogWrapper>(wrapperButtons());
const el = await styledFixture<DialogWrapper>(wrapperButtons());

await elementUpdated(el);
expect(el.open).to.be.true;
Expand All @@ -146,7 +179,7 @@ describe('Dialog Wrapper', () => {
const handleConfirm = (): void => confirmSpy();
const handleCancel = (): void => cancelSpy();
const handleSecondary = (): void => secondarySpy();
const el = await fixture<DialogWrapper>(wrapperButtons());
const el = await styledFixture<DialogWrapper>(wrapperButtons());
el.addEventListener('confirm', handleConfirm);
el.addEventListener('cancel', handleCancel);
el.addEventListener('secondary', handleSecondary);
Expand Down
5 changes: 5 additions & 0 deletions packages/menu/src/menu.css
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ governing permissions and limitations under the License.
var(--spectrum-icon-checkmark-medium-width) +
var(--spectrum-listitem-icon-gap)
);

/**
* Allow menu elements to be foreced to width 100% when placed inside of a Tray
**/
width: var(--swc-menu-width);
}

:host(:focus) {
Expand Down
13 changes: 13 additions & 0 deletions packages/modal/src/modal-wrapper.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
Copyright 2020 Adobe. All rights reserved.
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
governing permissions and limitations under the License.
*/

@import './spectrum-modal-wrapper.css';
1 change: 0 additions & 1 deletion packages/modal/src/modal.css
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ OF ANY KIND, either express or implied. See the License for the specific languag
governing permissions and limitations under the License.
*/

@import './spectrum-modal-wrapper.css';
@import './spectrum-modal.css';

:host {
Expand Down
40 changes: 35 additions & 5 deletions packages/overlay/src/ActiveOverlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ export class ActiveOverlay extends SpectrumElement {
public trigger!: HTMLElement;
public virtualTrigger?: VirtualTrigger;

protected childrenReady!: Promise<unknown[]>;

@property()
public _state = stateTransition();
public get state(): OverlayStateType {
Expand Down Expand Up @@ -245,18 +247,40 @@ export class ActiveOverlay extends SpectrumElement {

this.state = 'active';
this.feature();
if (this.placement && this.placement !== 'none') {
if (this.placement === 'none') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidyxu The code here should address the 100vh situation causing scroll in your previous review, can you confirm that this has corrected the issue on your end (Android, right?) as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 yep looks good on my end

this.style.setProperty(
'--swc-visual-viewport-height',
`${window.innerHeight}px`
);
} else if (this.placement) {
await this.updateOverlayPosition();
document.addEventListener(
'sp-update-overlays',
this.updateOverlayPosition
);
window.addEventListener('scroll', this.updateOverlayPosition);
}
await this.applyContentAnimation('sp-overlay-fade-in');
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.childrenReady = Promise.all(actions);
}

public async openCallback(): Promise<void> {
await this.updateComplete;
if (this.receivesFocus) {
this.focus();
}

this.trigger.dispatchEvent(
new CustomEvent<OverlayOpenCloseDetail>('sp-opened', {
bubbles: true,
Expand Down Expand Up @@ -519,9 +543,15 @@ export class ActiveOverlay extends SpectrumElement {
private stealOverlayContentResolver!: () => void;

protected async getUpdateComplete(): Promise<boolean> {
const complete = (await super.getUpdateComplete()) as boolean;
await this.stealOverlayContentPromise;
return complete;
const actions: Promise<unknown>[] = [
super.getUpdateComplete(),
this.stealOverlayContentPromise,
];
if (this.childrenReady) {
actions.push(this.childrenReady);
}
const [complete] = await Promise.all(actions);
return complete as boolean;
}

disconnectedCallback(): void {
Expand Down
Loading