Skip to content

Commit

Permalink
fix(dialog)!: remove modeless
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: Material dialogs are always modal.

PiperOrigin-RevId: 553603172
  • Loading branch information
@asyncLiz @copybara-github
asyncLiz authored and copybara-github committed Aug 3, 2023
1 parent 4ab2e39 commit d8ac9ce
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 191 deletions.
1 change: 0 additions & 1 deletion dialog/demo/demo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {stories, StoryKnobs} from './stories.js';
const collection =
new MaterialCollection<KnobTypesToKnobs<StoryKnobs>>('Dialog', [
new Knob('fullscreen', {defaultValue: false, ui: boolInput()}),
new Knob('modeless', {defaultValue: false, ui: boolInput()}),
new Knob('footerHidden', {defaultValue: false, ui: boolInput()}),
new Knob('stacked', {defaultValue: false, ui: boolInput()}),
new Knob('icon', {defaultValue: '', ui: textInput()}),
Expand Down
67 changes: 6 additions & 61 deletions dialog/demo/stories.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {css, html} from 'lit';
/** Knob types for dialog stories. */
export interface StoryKnobs {
fullscreen: boolean;
modeless: boolean;
footerHidden: boolean;
stacked: boolean;
icon: string;
Expand All @@ -35,20 +34,11 @@ function clickHandler(event: Event) {

const standard: MaterialStoryInit<StoryKnobs> = {
name: '<md-dialog>',
render({
fullscreen,
modeless,
footerHidden,
stacked,
icon,
headline,
supportingText
}) {
render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) {
return html`
<md-filled-button @click=${clickHandler}>Open</md-filled-button>
<md-dialog
.fullscreen=${fullscreen}
.modeless=${modeless}
.footerHidden=${footerHidden}
.stacked=${stacked}
>
Expand All @@ -63,20 +53,11 @@ const standard: MaterialStoryInit<StoryKnobs> = {
const alert: MaterialStoryInit<StoryKnobs> = {

name: 'Alert',
render({
fullscreen,
modeless,
footerHidden,
stacked,
icon,
headline,
supportingText
}) {
render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) {
return html`
<md-filled-button @click=${clickHandler}>Open</md-filled-button>
<md-dialog
.fullscreen=${fullscreen}
.modeless=${modeless}
.footerHidden=${footerHidden}
.stacked=${stacked}
>
Expand All @@ -89,20 +70,11 @@ const alert: MaterialStoryInit<StoryKnobs> = {

const confirm: MaterialStoryInit<StoryKnobs> = {
name: 'Confirm',
render({
fullscreen,
modeless,
footerHidden,
stacked,
icon,
headline,
supportingText
}) {
render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) {
return html`
<md-filled-button @click=${clickHandler}>Open</md-filled-button>
<md-dialog style="--md-dialog-container-max-inline-size: 320px;"
.fullscreen=${fullscreen}
.modeless=${modeless}
.footerHidden=${footerHidden}
.stacked=${stacked}
>
Expand All @@ -126,20 +98,11 @@ const choose: MaterialStoryInit<StoryKnobs> = {
align-items: center;
}
`,
render({
fullscreen,
modeless,
footerHidden,
stacked,
icon,
headline,
supportingText
}) {
render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) {
return html`
<md-filled-button @click=${clickHandler}>Open</md-filled-button>
<md-dialog
.fullscreen=${fullscreen}
.modeless=${modeless}
.footerHidden=${footerHidden}
.stacked=${stacked}
>
Expand Down Expand Up @@ -190,20 +153,11 @@ const contacts: MaterialStoryInit<StoryKnobs> = {
.contact-row > * {
flex: 1;
}`,
render({
fullscreen,
modeless,
footerHidden,
stacked,
icon,
headline,
supportingText
}) {
render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) {
return html`
<md-filled-button @click=${clickHandler}>Open</md-filled-button>
<md-dialog class="contacts"
.fullscreen=${fullscreen}
.modeless=${modeless}
.footerHidden=${footerHidden}
.stacked=${stacked}
>
Expand Down Expand Up @@ -231,20 +185,11 @@ const contacts: MaterialStoryInit<StoryKnobs> = {

const floatingSheet: MaterialStoryInit<StoryKnobs> = {
name: 'Floating sheet',
render({
fullscreen,
modeless,
footerHidden,
stacked,
icon,
headline,
supportingText
}) {
render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) {
return html`
<md-filled-button @click=${clickHandler}>Open</md-filled-button>
<md-dialog
.fullscreen=${fullscreen}
.modeless=${modeless}
.footerHidden=${footerHidden}
.stacked=${stacked}
>
Expand Down
74 changes: 9 additions & 65 deletions dialog/dialog_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,6 @@ import {createTokenTests} from '../testing/tokens.js';
import {MdDialog} from './dialog.js';
import {DialogHarness} from './harness.js';

function isDocumentScrollingDisabled() {
return getComputedStyle(document.body).overflow === 'hidden';
}

interface DialogTestProps {
modeless?: boolean;
}

function getDialogTemplate(props?: DialogTestProps) {
return html`
<md-dialog .modeless=${props?.modeless ?? false}>
<div class="content">Content
<input dialog-focus>
</div>
<button slot="footer" dialog-action="button">Close</button>
</md-dialog>`;
}


describe('<md-dialog>', () => {
const realTimeout = globalThis.setTimeout;
const env = new Environment();
Expand All @@ -46,10 +27,16 @@ describe('<md-dialog>', () => {
}
}

async function setupTest() {
const root = env.render(html`
<md-dialog>
<div class="content">Content
<input dialog-focus>
</div>
<button slot="footer" dialog-action="button">Close</button>
</md-dialog>
`);

async function setupTest(
props?: DialogTestProps, template = getDialogTemplate) {
const root = env.render(template(props));
await env.waitForStability();
setClockEnabled(false);
const dialog = root.querySelector<MdDialog>('md-dialog')!;
Expand Down Expand Up @@ -112,17 +99,6 @@ describe('<md-dialog>', () => {
expect(await harness.isScrimVisible()).toBeFalse();
});

it('prevents document scrolling when open', async () => {
const {harness} = await setupTest();
expect(isDocumentScrollingDisabled()).toBeFalse();
harness.element.open = true;
await harness.transitionComplete();
expect(isDocumentScrollingDisabled()).toBeTrue();
harness.element.open = false;
await harness.transitionComplete();
expect(isDocumentScrollingDisabled()).toBeFalse();
});

it('fires open/close events', async () => {
const {harness} = await setupTest();
const openingHandler = jasmine.createSpy('openingHandler');
Expand Down Expand Up @@ -193,37 +169,5 @@ describe('<md-dialog>', () => {
expect(document.activeElement).toBe(button);
button.remove();
});

describe('modeless', () => {
it('does not render srcrim', async () => {
const {harness} = await setupTest({modeless: true});
expect(await harness.isScrimVisible()).toBeFalse();
harness.element.open = true;
expect(await harness.isScrimVisible()).toBeFalse();
harness.element.open = false;
expect(await harness.isScrimVisible()).toBeFalse();
});

it('does not close on external click', async () => {
const {harness} = await setupTest({modeless: true});
harness.element.show();
const dialogElement = await harness.getInteractiveElement();
dialogElement.click();
await harness.transitionComplete();
expect(harness.element.open).toBeTrue();
harness.element.close();
await harness.transitionComplete();
});
it('does not prevent document scrolling', async () => {
const {harness} = await setupTest({modeless: true});
expect(isDocumentScrollingDisabled()).toBeFalse();
harness.element.open = true;
await harness.transitionComplete();
expect(isDocumentScrollingDisabled()).toBeFalse();
harness.element.open = false;
await harness.transitionComplete();
expect(isDocumentScrollingDisabled()).toBeFalse();
});
});
});
});
12 changes: 0 additions & 12 deletions dialog/internal/_dialog.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,6 @@ $_closing-transition-easing: map.get(
overflow: clip;
}

// stylelint-disable-next-line selector-pseudo-class-no-unknown --
// New platform pseudo-class
.dialog:not(:modal) {
z-index: 10000;
pointer-events: none;
}

.dialog[open] {
display: flex;
}
Expand All @@ -72,11 +65,6 @@ $_closing-transition-easing: map.get(
background: none;
}

// Hide scrim when modeless.
:host([modeless]) .dialog::backdrop {
display: none;
}

.container {
position: absolute;

Expand Down
56 changes: 4 additions & 52 deletions dialog/internal/dialog.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,34 +28,6 @@ export const CLOSE_ACTION = 'close';
* @fires cancel The native HTMLDialogElement cancel event.
*/
export class Dialog extends LitElement {
private static preventedScrollingCount = 0;
private static scrollbarTester: HTMLDivElement;

private static setDocumentScrollingDisabled(disabled = false) {
let {preventedScrollingCount, scrollbarTester} = Dialog;
Dialog.preventedScrollingCount = preventedScrollingCount =
Math.max(preventedScrollingCount + (Number(disabled) || -1), 0);
const shouldPrevent = Boolean(preventedScrollingCount);
const {style} = document.body;
if (shouldPrevent && style.overflow === 'hidden') {
return;
}
if (scrollbarTester === undefined) {
Dialog.scrollbarTester = scrollbarTester = document.createElement('div');
scrollbarTester.style.cssText =
`position: absolute; height: 0; width: 100%; visibility: hidden; pointer-events: none;`;
}
// Appends an element to see if its offsetWidth changes when overflow is
// altered. If it does, then add end inline padding to restore the width to
// avoid a visible layout shift.
document.body.append(scrollbarTester);
const {offsetWidth} = scrollbarTester;
style.overflow = shouldPrevent ? 'hidden' : '';
const scrollbarWidth = scrollbarTester.offsetWidth - offsetWidth;
scrollbarTester.remove();
style.paddingInlineEnd = shouldPrevent ? `${scrollbarWidth}px` : '';
}

/**
* Opens the dialog when set to `true` and closes it when set to `false`.
*/
Expand Down Expand Up @@ -150,13 +122,6 @@ export class Dialog extends LitElement {
*/
@property({attribute: 'escape-key-action'}) escapeKeyAction = CLOSE_ACTION;

/**
* When opened, the dialog is displayed modeless or non-modal. This
* allows users to interact with content outside the dialog without
* closing the dialog and does not display the scrim around the dialog.
*/
@property({type: Boolean, reflect: true}) modeless = false;

private readonly throttle = createThrottle();

@query('.dialog', true)
Expand Down Expand Up @@ -294,20 +259,11 @@ export class Dialog extends LitElement {
if (!changed.has('open')) {
return;
}
// prevent body scrolling early only when opening to avoid layout
// shift when closing.
if (!this.modeless && this.open) {
Dialog.setDocumentScrollingDisabled(this.open);
}
if (this.open) {
this.contentElement!.scrollTop = 0;
if (this.modeless) {
this.dialogElement!.show();
} else {
// Note, native focus handling fails when focused element is in an
// overflow: auto container.
this.dialogElement!.showModal();
}
// Note, native focus handling fails when focused element is in an
// overflow: auto container.
this.dialogElement!.showModal();
}
// Avoids dispatching initial state.
const shouldDispatchAction = changed.get('open') !== undefined;
Expand Down Expand Up @@ -376,10 +332,6 @@ export class Dialog extends LitElement {
});
this.dialogElement?.close(this.currentAction || this.defaultAction);
await closedPromise;
// enable scrolling late to avoid layout shift when closing
if (!this.modeless) {
Dialog.setDocumentScrollingDisabled(this.open);
}
}
if (shouldDispatchAction) {
this.dispatchActionEvent(this.open ? 'opened' : 'closed');
Expand Down Expand Up @@ -443,7 +395,7 @@ export class Dialog extends LitElement {
}
this.currentAction =
(event.target as Element).getAttribute(this.actionAttribute) ??
(!this.modeless && this.containerElement &&
(this.containerElement &&
!event.composedPath().includes(this.containerElement) ?
this.scrimClickAction :
'');
Expand Down

0 comments on commit d8ac9ce

Please sign in to comment.