From 58441b1d1fb5d4d0fb3923a56fa88706878132c3 Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Sat, 1 Jan 2022 16:38:46 +0100 Subject: [PATCH 1/4] refactor: Modals rewrite to use native HTML Dialog element --- js/package.json | 1 + js/src/@types/modals/index.d.ts | 46 ++++++++++++ js/src/common/components/Modal.tsx | 70 +++++++++++++++--- js/src/common/components/ModalManager.tsx | 88 ++++++++++++++++------- js/src/common/index.js | 5 +- js/src/common/states/ModalManagerState.ts | 4 +- js/yarn.lock | 8 +++ less/common/Modal.less | 74 ++++++++----------- 8 files changed, 215 insertions(+), 81 deletions(-) create mode 100644 js/src/@types/modals/index.d.ts diff --git a/js/package.json b/js/package.json index f2f66be927..0c4ef7fffa 100644 --- a/js/package.json +++ b/js/package.json @@ -9,6 +9,7 @@ "clsx": "^1.1.1", "color-thief-browser": "^2.0.2", "dayjs": "^1.10.7", + "dialog-polyfill": "^0.5.6", "focus-trap": "^6.7.1", "jquery": "^3.6.0", "jquery.hotkeys": "^0.1.0", diff --git a/js/src/@types/modals/index.d.ts b/js/src/@types/modals/index.d.ts new file mode 100644 index 0000000000..3dc0a870a8 --- /dev/null +++ b/js/src/@types/modals/index.d.ts @@ -0,0 +1,46 @@ +/** + * Only supported natively in Chrome. In testing in Safari Technology Preview. + * + * Please register modals with the dialog polyfill before use: + * + * ```js + * dialogPolyfill.registerDialog(dialogElementReference); + * ``` + * + * ### Events + * + * Two events are fired by dialogs: + * - `cancel` - Fired when the user instructs the browser that they wish to dismiss the current open dialog. + * - `close` - Fired when the dialog is closed. + * + * @see https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement + */ +interface HTMLDialogElement { + /** + * Shows the `` element as a top-layered element in the document. + */ + show(): void; + /** + * Displays the dialog as a modal, over the top of any other dialogs that + * might be present. Interaction outside the dialog is blocked. + */ + showModal(): void; + /** + * If the `` element is currently being shown, dismiss it. + * + * @param returnValue An optional return value for the dialog to hold. *This is currently unused by Flarum.* + */ + close(returnValue?: string): void; + + /** + * A return value for the dialog to hold. + * + * *This is currently unused by Flarum.* + */ + returnValue: string; + + /** + * Whether the dialog is currently open. + */ + open: boolean; +} diff --git a/js/src/common/components/Modal.tsx b/js/src/common/components/Modal.tsx index b8656860d6..31202d9b3b 100644 --- a/js/src/common/components/Modal.tsx +++ b/js/src/common/components/Modal.tsx @@ -8,6 +8,7 @@ import type ModalManagerState from '../states/ModalManagerState'; import type RequestError from '../utils/RequestError'; import type ModalManager from './ModalManager'; import fireDebugWarning from '../helpers/fireDebugWarning'; +import classList from '../utils/classList'; export interface IInternalModalAttrs { state: ModalManagerState; @@ -15,16 +16,63 @@ export interface IInternalModalAttrs { animateHide: ModalManager['animateHide']; } +export interface IDismissibleOptions { + /** + * @deprecated Check specific individual attributes instead. Will be removed in Flarum 2.0. + */ + isDismissible: boolean; + viaCloseButton: boolean; + viaEscKey: boolean; + viaBackdropClick: boolean; +} + /** * The `Modal` component displays a modal dialog, wrapped in a form. Subclasses * should implement the `className`, `title`, and `content` methods. */ export default abstract class Modal extends Component { + // TODO: [Flarum 2.0] remove `isDismissible` static attribute /** * Determine whether or not the modal should be dismissible via an 'x' button. + * + * @deprecated Use the individual `isDismissibleVia...` attributes instead and remove references to this. */ static readonly isDismissible: boolean = true; + /** + * Can the model be dismissed with a close button (X)? + * + * If `false`, no close button is shown. + */ + protected static readonly isDismissibleViaCloseButton: boolean = true; + /** + * Can the modal be dismissed by pressing the Esc key on a keyboard? + */ + protected static readonly isDismissibleViaEscKey: boolean = true; + /** + * Can the modal be dismissed via a click on the backdrop. + */ + protected static readonly isDismissibleViaBackdropClick: boolean = true; + + static get dismissibleOptions(): IDismissibleOptions { + // If someone sets this to `false`, provide the same behaviour as previous versions of Flarum. + if (!this.isDismissible) { + return { + isDismissible: false, + viaCloseButton: false, + viaEscKey: false, + viaBackdropClick: false, + }; + } + + return { + isDismissible: true, + viaCloseButton: this.isDismissibleViaCloseButton, + viaEscKey: this.isDismissibleViaEscKey, + viaBackdropClick: this.isDismissibleViaBackdropClick, + }; + } + protected loading: boolean = false; /** @@ -87,16 +135,16 @@ export default abstract class Modal +
- {(this.constructor as typeof Modal).isDismissible && ( + {this.dismissibleOptions.viaCloseButton && (
- {Button.component({ - icon: 'fas fa-times', - onclick: () => this.hide(), - className: 'Button Button--icon Button--link', - 'aria-label': app.translator.trans('core.lib.modal.close'), - })} +
)} @@ -110,7 +158,7 @@ export default abstract class Modal
- + ); } @@ -175,4 +223,8 @@ export default abstract class Modal { */ protected modalShown: boolean = false; + protected modalClosing: boolean = false; + view(vnode: Mithril.VnodeDOM): Mithril.Children { const modal = this.attrs.state.modal; const Tag = modal?.componentClass; return ( -
+
{!!Tag && ( { oncreate(vnode: Mithril.VnodeDOM): void { super.oncreate(vnode); - // Ensure the modal state is notified about a closed modal, even when the - // DOM-based Bootstrap JavaScript code triggered the closing of the modal, - // e.g. via ESC key or a click on the modal backdrop. - this.$().on('hidden.bs.modal', this.attrs.state.close.bind(this.attrs.state)); - this.focusTrap = createFocusTrap(this.element as HTMLElement); } @@ -65,35 +62,74 @@ export default class ModalManager extends Component { }); } + private get dialogElement(): HTMLDialogElement { + return this.element.querySelector('dialog') as HTMLDialogElement; + } + animateShow(readyCallback: () => void): void { if (!this.attrs.state.modal) return; - const dismissible = !!this.attrs.state.modal.componentClass.isDismissible; + const dismissibleState = this.attrs.state.modal.componentClass.dismissibleOptions; this.modalShown = true; - // If we are opening this modal while another modal is already open, - // the shown event will not run, because the modal is already open. - // So, we need to manually trigger the readyCallback. - if (this.$().hasClass('in')) { - readyCallback(); - return; - } - - this.$() - .one('shown.bs.modal', readyCallback) - // @ts-expect-error: No typings available for Bootstrap modals. - .modal({ - backdrop: dismissible || 'static', - keyboard: dismissible, - }) - .modal('show'); + // Register with polyfill + dialogPolyfill.registerDialog(this.dialogElement); + + if (!dismissibleState.viaEscKey) this.dialogElement.addEventListener('cancel', this.preventEscPressHandler); + if (dismissibleState.viaBackdropClick) this.dialogElement.addEventListener('click', (e) => this.handleBackdropClick.call(this, e)); + + this.dialogElement.addEventListener('transitionend', () => readyCallback(), { once: true }); + // Ensure the modal state is ALWAYS notified about a closed modal + this.dialogElement.addEventListener('close', this.attrs.state.close.bind(this.attrs.state)); + + // Use close animation instead + this.dialogElement.addEventListener('cancel', this.animateCloseHandler.bind(this)); + + this.dialogElement.showModal(); + + // Fade in + requestAnimationFrame(() => { + this.dialogElement.classList.add('in'); + }); } animateHide(): void { - // @ts-expect-error: No typings available for Bootstrap modals. - this.$().modal('hide'); + if (this.modalClosing || !this.modalShown) return; + this.modalClosing = true; + + this.dialogElement.addEventListener( + 'transitionend', + () => { + this.dialogElement.close(); + this.dialogElement.removeEventListener('cancel', this.preventEscPressHandler); + + this.modalShown = false; + this.modalClosing = false; + m.redraw(); + }, + { once: true } + ); + + this.dialogElement.classList.remove('in'); + this.dialogElement.classList.add('out'); + } + + animateCloseHandler(this: this, e: Event) { + e.preventDefault(); + + this.animateHide(); + } + + preventEscPressHandler(this: this, e: Event) { + e.preventDefault(); + } + + handleBackdropClick(this: this, e: MouseEvent) { + // If it's a click on the dialog element, the backdrop has been clicked. + // If it was a click in the modal, the element would be `div.Modal-content` or some other element. + if (e.target !== this.dialogElement) return; - this.modalShown = false; + this.animateHide(); } } diff --git a/js/src/common/index.js b/js/src/common/index.js index ac8c62f1c9..10bcffd221 100644 --- a/js/src/common/index.js +++ b/js/src/common/index.js @@ -3,9 +3,12 @@ import 'expose-loader?exposes=$,jQuery!jquery'; import 'expose-loader?exposes=m!mithril'; import 'expose-loader?exposes=dayjs!dayjs'; +// HTML dialog element polyfill from the Chrome Dev team: https://github.com/GoogleChrome/dialog-polyfill +import dialogPolyfill from 'dialog-polyfill'; +window.dialogPolyfill = dialogPolyfill; + import 'bootstrap/js/affix'; import 'bootstrap/js/dropdown'; -import 'bootstrap/js/modal'; import 'bootstrap/js/tooltip'; import 'bootstrap/js/transition'; import 'jquery.hotkeys/jquery.hotkeys'; diff --git a/js/src/common/states/ModalManagerState.ts b/js/src/common/states/ModalManagerState.ts index a4a36093d2..585eb17e8c 100644 --- a/js/src/common/states/ModalManagerState.ts +++ b/js/src/common/states/ModalManagerState.ts @@ -1,5 +1,5 @@ import type Component from '../Component'; -import Modal from '../components/Modal'; +import Modal, { IDismissibleOptions } from '../components/Modal'; /** * Ideally, `show` would take a higher-kinded generic, ala: @@ -8,7 +8,7 @@ import Modal from '../components/Modal'; * https://github.com/Microsoft/TypeScript/issues/1213 * Therefore, we have to use this ugly, messy workaround. */ -type UnsafeModalClass = ComponentClass & { isDismissible: boolean; component: typeof Component.component }; +type UnsafeModalClass = ComponentClass & { get dismissibleOptions(): IDismissibleOptions; component: typeof Component.component }; /** * Class used to manage modal state. diff --git a/js/yarn.lock b/js/yarn.lock index 7842dc86f7..8f21745840 100644 --- a/js/yarn.lock +++ b/js/yarn.lock @@ -1401,6 +1401,7 @@ __metadata: color-thief-browser: ^2.0.2 cross-env: ^7.0.3 dayjs: ^1.10.7 + dialog-polyfill: ^0.5.6 expose-loader: ^3.1.0 flarum-tsconfig: ^1.0.2 flarum-webpack-config: ^2.0.0 @@ -2310,6 +2311,13 @@ __metadata: languageName: node linkType: hard +"dialog-polyfill@npm:^0.5.6": + version: 0.5.6 + resolution: "dialog-polyfill@npm:0.5.6" + checksum: dde8d4b6a62b63b710e0d0d70b615d92ff08f3cb2b521e1f99b17e8220d9d55d0fdf9fc17fbe77b0ff1be817094e14b60c4c4bacd5456fba427ede48d2d90230 + languageName: node + linkType: hard + "duplexer@npm:^0.1.1, duplexer@npm:^0.1.2": version: 0.1.2 resolution: "duplexer@npm:0.1.2" diff --git a/less/common/Modal.less b/less/common/Modal.less index 892057850b..6a1565abf1 100644 --- a/less/common/Modal.less +++ b/less/common/Modal.less @@ -7,54 +7,40 @@ } // Modal background -.modal-backdrop { - position: fixed; - top: 0; - right: 0; - bottom: 0; - left: 0; - z-index: var(--zindex-modal-background); - background-color: var(--overlay-bg); - opacity: 0; - transition: opacity 0.2s; - - &.in { - opacity: 1; +.ModalManager dialog { + //! These MUST be defined separately + // + // When browsers do not understand a pseudoselector, the entire block is ignored, + // meaning no backdrop would be visible at all. + &::backdrop { + // `::backdrop` does not inherit anything from parents, so we need to declare this here. + // See: https://stackoverflow.com/a/63322762/11091039 + + --overlay-bg: @overlay-bg; + background: var(--overlay-bg); + } + + + .backdrop { + background: var(--overlay-bg); } } // Container that the modal scrolls within .ModalManager { - display: none; - overflow: hidden; - position: fixed; - top: 0; - right: 0; - bottom: 0; - left: 0; - z-index: var(--zindex-modal); - -webkit-overflow-scrolling: touch; - // When fading in the modal, animate it to slide down - .Modal { + dialog { + border-width: 0; + padding: 0; + overflow: visible; + border-radius: @border-radius; + transform: scale(0.9); - transition: transform 0.2s ease-out; - } - &.in .Modal { - transform: scale(1); - } -} -.modal-open .ModalManager { - overflow-x: hidden; - overflow-y: auto; -} + transition: transform 0.2s ease-out, opacity 0.2s ease-out; -// Shell div to position the modal with bottom padding -.Modal { - position: relative; - width: auto; - margin: 10px; - max-width: 600px; + &.in { + transform: scale(1); + } + } } // Actual modal @@ -173,6 +159,11 @@ } @media @tablet-up { + .ModalManager dialog { + border-radius: var(--border-radius); + box-shadow: 0 7px 15px var(--shadow-color); + width: 100%; + } .Modal { margin: 120px auto; } @@ -183,10 +174,7 @@ z-index: 1; } .Modal-content { - - border: 0; border-radius: var(--border-radius); - box-shadow: 0 7px 15px var(--shadow-color); } .Modal--small { max-width: 375px; From d8841bba6155f443590bbcc3ebe03745291ebdb9 Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Sat, 1 Jan 2022 16:56:06 +0100 Subject: [PATCH 2/4] fix: drag from inside to outside modal closes it (fixes #2715) --- js/src/common/components/ModalManager.tsx | 31 +++++++++++++++++++---- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/js/src/common/components/ModalManager.tsx b/js/src/common/components/ModalManager.tsx index 69735eeef4..d8c46c5d59 100644 --- a/js/src/common/components/ModalManager.tsx +++ b/js/src/common/components/ModalManager.tsx @@ -24,6 +24,8 @@ export default class ModalManager extends Component { protected modalClosing: boolean = false; + protected clickStartedOnBackdrop: boolean = false; + view(vnode: Mithril.VnodeDOM): Mithril.Children { const modal = this.attrs.state.modal; const Tag = modal?.componentClass; @@ -77,7 +79,10 @@ export default class ModalManager extends Component { dialogPolyfill.registerDialog(this.dialogElement); if (!dismissibleState.viaEscKey) this.dialogElement.addEventListener('cancel', this.preventEscPressHandler); - if (dismissibleState.viaBackdropClick) this.dialogElement.addEventListener('click', (e) => this.handleBackdropClick.call(this, e)); + if (dismissibleState.viaBackdropClick) { + this.dialogElement.addEventListener('mousedown', (e) => this.handleBackdropMouseDown.call(this, e)); + this.dialogElement.addEventListener('click', (e) => this.handleBackdropClick.call(this, e)); + } this.dialogElement.addEventListener('transitionend', () => readyCallback(), { once: true }); // Ensure the modal state is ALWAYS notified about a closed modal @@ -115,21 +120,37 @@ export default class ModalManager extends Component { this.dialogElement.classList.add('out'); } - animateCloseHandler(this: this, e: Event) { + protected animateCloseHandler(this: this, e: Event) { e.preventDefault(); this.animateHide(); } - preventEscPressHandler(this: this, e: Event) { + protected preventEscPressHandler(this: this, e: Event) { e.preventDefault(); } - handleBackdropClick(this: this, e: MouseEvent) { + protected handleBackdropMouseDown(this: this, e: MouseEvent) { + // If it's a mousedown on the dialog element, the backdrop has been clicked. + // If it was a mousedown in the modal, the element would be `div.Modal-content` or some other element. + if (e.target !== this.dialogElement) return; + + this.clickStartedOnBackdrop = true; + window.addEventListener( + 'mouseup', + () => { + if (e.target !== this.dialogElement) this.clickStartedOnBackdrop = false; + }, + { once: true } + ); + } + + protected handleBackdropClick(this: this, e: MouseEvent) { // If it's a click on the dialog element, the backdrop has been clicked. // If it was a click in the modal, the element would be `div.Modal-content` or some other element. - if (e.target !== this.dialogElement) return; + if (e.target !== this.dialogElement || !this.clickStartedOnBackdrop) return; + this.clickStartedOnBackdrop = false; this.animateHide(); } } From 8b3ddd856e8ba95fb402226dd3a94fd67f1f3dcc Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Sat, 1 Jan 2022 17:21:27 +0100 Subject: [PATCH 3/4] styling fixes --- less/common/Modal.less | 48 ++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/less/common/Modal.less b/less/common/Modal.less index 6a1565abf1..1ba6983709 100644 --- a/less/common/Modal.less +++ b/less/common/Modal.less @@ -6,8 +6,24 @@ overflow: hidden; } -// Modal background -.ModalManager dialog { +.Modal { + border-width: 0; + padding: 0; + overflow: visible; + border-radius: @border-radius; + + transform: scale(0.9); + transition: transform 0.2s ease-out, opacity 0.2s ease-out; + + // only in polyfilled browsers + &[role] { + top: 0 !important; + } + + &.in { + transform: scale(1); + } + //! These MUST be defined separately // // When browsers do not understand a pseudoselector, the entire block is ignored, @@ -22,24 +38,11 @@ + .backdrop { background: var(--overlay-bg); - } -} - -// Container that the modal scrolls within -.ModalManager { - // When fading in the modal, animate it to slide down - dialog { - border-width: 0; - padding: 0; - overflow: visible; - border-radius: @border-radius; - - transform: scale(0.9); - transition: transform 0.2s ease-out, opacity 0.2s ease-out; - - &.in { - transform: scale(1); - } + position: fixed; + top: 0; + left: 0; + right: 0; + bottom: 0; } } @@ -159,12 +162,11 @@ } @media @tablet-up { - .ModalManager dialog { + .Modal { border-radius: var(--border-radius); box-shadow: 0 7px 15px var(--shadow-color); width: 100%; - } - .Modal { + max-width: 600px; margin: 120px auto; } .Modal-close { From 3db258390da6eda651d712d4f1bf5e990a5b0188 Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Tue, 4 Jan 2022 13:20:37 +0000 Subject: [PATCH 4/4] style fixes --- less/common/Modal.less | 6 ------ 1 file changed, 6 deletions(-) diff --git a/less/common/Modal.less b/less/common/Modal.less index 1ba6983709..41d893b585 100644 --- a/less/common/Modal.less +++ b/less/common/Modal.less @@ -128,13 +128,7 @@ bottom: 0; top: 0; overflow: auto; - transition: transform 0.2s ease-out; - transform: translate(0, 100vh); - &.in { - -webkit-transform: none !important; - transform: none !important; - } &:before { content: " "; .header-background();