From d8ac9ce29e146516d84c3f6ba38cd9b8d39db8ec Mon Sep 17 00:00:00 2001 From: Elizabeth Mitchell Date: Thu, 3 Aug 2023 15:11:47 -0700 Subject: [PATCH] fix(dialog)!: remove modeless BREAKING CHANGE: Material dialogs are always modal. PiperOrigin-RevId: 553603172 --- dialog/demo/demo.ts | 1 - dialog/demo/stories.ts | 67 +++----------------------------- dialog/dialog_test.ts | 74 +++++------------------------------- dialog/internal/_dialog.scss | 12 ------ dialog/internal/dialog.ts | 56 ++------------------------- 5 files changed, 19 insertions(+), 191 deletions(-) diff --git a/dialog/demo/demo.ts b/dialog/demo/demo.ts index becca08c7e..8eed5eb5c7 100644 --- a/dialog/demo/demo.ts +++ b/dialog/demo/demo.ts @@ -15,7 +15,6 @@ import {stories, StoryKnobs} from './stories.js'; const collection = new MaterialCollection>('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()}), diff --git a/dialog/demo/stories.ts b/dialog/demo/stories.ts index b6f83eb4e5..2a7a086b82 100644 --- a/dialog/demo/stories.ts +++ b/dialog/demo/stories.ts @@ -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; @@ -35,20 +34,11 @@ function clickHandler(event: Event) { const standard: MaterialStoryInit = { name: '', - render({ - fullscreen, - modeless, - footerHidden, - stacked, - icon, - headline, - supportingText - }) { + render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) { return html` Open @@ -63,20 +53,11 @@ const standard: MaterialStoryInit = { const alert: MaterialStoryInit = { name: 'Alert', - render({ - fullscreen, - modeless, - footerHidden, - stacked, - icon, - headline, - supportingText - }) { + render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) { return html` Open @@ -89,20 +70,11 @@ const alert: MaterialStoryInit = { const confirm: MaterialStoryInit = { name: 'Confirm', - render({ - fullscreen, - modeless, - footerHidden, - stacked, - icon, - headline, - supportingText - }) { + render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) { return html` Open @@ -126,20 +98,11 @@ const choose: MaterialStoryInit = { align-items: center; } `, - render({ - fullscreen, - modeless, - footerHidden, - stacked, - icon, - headline, - supportingText - }) { + render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) { return html` Open @@ -190,20 +153,11 @@ const contacts: MaterialStoryInit = { .contact-row > * { flex: 1; }`, - render({ - fullscreen, - modeless, - footerHidden, - stacked, - icon, - headline, - supportingText - }) { + render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) { return html` Open @@ -231,20 +185,11 @@ const contacts: MaterialStoryInit = { const floatingSheet: MaterialStoryInit = { name: 'Floating sheet', - render({ - fullscreen, - modeless, - footerHidden, - stacked, - icon, - headline, - supportingText - }) { + render({fullscreen, footerHidden, stacked, icon, headline, supportingText}) { return html` Open diff --git a/dialog/dialog_test.ts b/dialog/dialog_test.ts index 09749f8aff..c8f8dc2085 100644 --- a/dialog/dialog_test.ts +++ b/dialog/dialog_test.ts @@ -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` - -
Content - -
- - `; -} - - describe('', () => { const realTimeout = globalThis.setTimeout; const env = new Environment(); @@ -46,10 +27,16 @@ describe('', () => { } } + async function setupTest() { + const root = env.render(html` + +
Content + +
+ + + `); - async function setupTest( - props?: DialogTestProps, template = getDialogTemplate) { - const root = env.render(template(props)); await env.waitForStability(); setClockEnabled(false); const dialog = root.querySelector('md-dialog')!; @@ -112,17 +99,6 @@ describe('', () => { 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'); @@ -193,37 +169,5 @@ describe('', () => { 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(); - }); - }); }); }); diff --git a/dialog/internal/_dialog.scss b/dialog/internal/_dialog.scss index 79e9165402..c65fc5dd90 100644 --- a/dialog/internal/_dialog.scss +++ b/dialog/internal/_dialog.scss @@ -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; } @@ -72,11 +65,6 @@ $_closing-transition-easing: map.get( background: none; } - // Hide scrim when modeless. - :host([modeless]) .dialog::backdrop { - display: none; - } - .container { position: absolute; diff --git a/dialog/internal/dialog.ts b/dialog/internal/dialog.ts index a3264ae564..62f815b448 100644 --- a/dialog/internal/dialog.ts +++ b/dialog/internal/dialog.ts @@ -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`. */ @@ -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) @@ -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; @@ -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'); @@ -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 : '');