diff --git a/.changeset/green-sloths-press.md b/.changeset/green-sloths-press.md new file mode 100644 index 0000000000..7c89cf1e9a --- /dev/null +++ b/.changeset/green-sloths-press.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': patch +--- + +Improve performance of Dialog element diff --git a/app/components/primer/dialog_helper.ts b/app/components/primer/dialog_helper.ts index 1382029bd1..a3a24216b4 100644 --- a/app/components/primer/dialog_helper.ts +++ b/app/components/primer/dialog_helper.ts @@ -1,3 +1,31 @@ +import 'dialog-toggle-events-polyfill' + +function dialogBeforeToggleHandler(event: Event) { + const {target, newState} = event as ToggleEvent + if (!(target instanceof HTMLDialogElement)) return + if (newState !== 'open') return + const {body} = target.ownerDocument + body.style.setProperty('--dialog-scrollgutter', `${window.innerWidth - body.clientWidth}px`) +} + +function dialogToggleHandler(event: Event) { + const {target, newState} = event as ToggleEvent + if (!(target instanceof HTMLDialogElement)) return + if (newState !== 'open') return + if (target.closest('dialog-helper')?.dialog !== target) return + // We don't want to show the Dialog component as non-modal + forceDialogToOpenAsModal(target) +} + +function forceDialogToOpenAsModal(dialog: HTMLDialogElement) { + if (dialog.matches('[open]:not(:modal)')) { + // eslint-disable-next-line no-restricted-syntax + dialog.addEventListener('close', e => e.stopImmediatePropagation(), {once: true}) + dialog.close() + dialog.showModal() + } +} + function dialogInvokerButtonHandler(event: Event) { const target = event.target as HTMLElement const button = target?.closest('button') @@ -76,35 +104,16 @@ export class DialogHelperElement extends HTMLElement { const {signal} = (this.#abortController = new AbortController()) document.addEventListener('click', dialogInvokerButtonHandler, true) document.addEventListener('click', this, {signal}) - this.ownerDocument.body.style.setProperty( - '--dialog-scrollgutter', - `${window.innerWidth - this.ownerDocument.body.clientWidth}px`, - ) - new MutationObserver(records => { - for (const record of records) { - if (record.target === this.dialog) { - this.#handleDialogOpenAttribute() - } - } - }).observe(this, {subtree: true, attributeFilter: ['open']}) - this.#handleDialogOpenAttribute() + document.addEventListener('beforetoggle', dialogBeforeToggleHandler, true) + document.addEventListener('toggle', dialogToggleHandler, true) + const {dialog} = this + if (dialog?.open) forceDialogToOpenAsModal(dialog) } disconnectedCallback() { this.#abortController?.abort() } - #handleDialogOpenAttribute() { - if (!this.dialog) return - // We don't want to show the Dialog component as non-modal - if (this.dialog.matches('[open]:not(:modal)')) { - // eslint-disable-next-line no-restricted-syntax - this.dialog.addEventListener('close', e => e.stopImmediatePropagation(), {once: true}) - this.dialog.close() - this.dialog.showModal() - } - } - handleEvent(event: MouseEvent) { const target = event.target as HTMLElement const dialog = this.dialog diff --git a/package-lock.json b/package-lock.json index ae85c066c0..9743b22e1c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,7 +21,8 @@ "@github/tab-container-element": "^3.1.2", "@oddbird/popover-polyfill": "^0.5.2", "@primer/behaviors": "^1.3.4", - "@primer/live-region-element": "^0.7.1" + "@primer/live-region-element": "^0.7.1", + "dialog-toggle-events-polyfill": "^1.0.1" }, "devDependencies": { "@changesets/changelog-github": "^0.5.0", @@ -4716,6 +4717,12 @@ "node": ">=8" } }, + "node_modules/dialog-toggle-events-polyfill": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/dialog-toggle-events-polyfill/-/dialog-toggle-events-polyfill-1.0.1.tgz", + "integrity": "sha512-wS8Ku4Z+BNdDoOEkAow/8c7Ilip/hHmV+75WhRxkWghbM8f909F4+vn+p63SE3Waq9K/nEt0gQrX+zUMVV0Kew==", + "license": "MIT" + }, "node_modules/diff": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/diff/-/diff-5.2.0.tgz", @@ -14986,6 +14993,11 @@ "integrity": "sha512-reYkTUJAZb9gUuZ2RvVCNhVHdg62RHnJ7WJl8ftMi4diZ6NWlciOzQN88pUhSELEwflJht4oQDv0F0BMlwaYtA==", "dev": true }, + "dialog-toggle-events-polyfill": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/dialog-toggle-events-polyfill/-/dialog-toggle-events-polyfill-1.0.1.tgz", + "integrity": "sha512-wS8Ku4Z+BNdDoOEkAow/8c7Ilip/hHmV+75WhRxkWghbM8f909F4+vn+p63SE3Waq9K/nEt0gQrX+zUMVV0Kew==" + }, "diff": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/diff/-/diff-5.2.0.tgz", diff --git a/package.json b/package.json index 968567cd0e..a39d71b304 100644 --- a/package.json +++ b/package.json @@ -53,9 +53,10 @@ "@github/relative-time-element": "^4.0.0", "@github/remote-input-element": "^0.4.0", "@github/tab-container-element": "^3.1.2", - "@primer/live-region-element": "^0.7.1", "@oddbird/popover-polyfill": "^0.5.2", - "@primer/behaviors": "^1.3.4" + "@primer/behaviors": "^1.3.4", + "@primer/live-region-element": "^0.7.1", + "dialog-toggle-events-polyfill": "^1.0.1" }, "peerDependencies": { "@primer/primitives": "9.x || 10.x"