From cf2cf8edb0d38b158fddf9047eb6917c7de5edc3 Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Thu, 15 Oct 2020 13:55:42 -0400 Subject: [PATCH] Fix concurrent active modals bug --- docs/getting-started/changelog.md | 1 + src/components/dialog/dialog.tsx | 19 ++++++++----------- src/components/drawer/drawer.tsx | 21 ++++++++------------- 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/docs/getting-started/changelog.md b/docs/getting-started/changelog.md index 526d1723..ddc86ce0 100644 --- a/docs/getting-started/changelog.md +++ b/docs/getting-started/changelog.md @@ -11,6 +11,7 @@ _During the beta period, these restrictions may be relaxed in the event of a mis - Added `label` slot to `sl-input`, `sl-select`, and `sl-textarea` [#248](https://github.com/shoelace-style/shoelace/issues/248) - Fixed a bug where initial transitions didn't show in `sl-dialog` and `sl-drawer` [#247](https://github.com/shoelace-style/shoelace/issues/247) - Fixed a bug where indeterminate checkboxes would maintain the indeterminate state when toggled +- Fixed a bug where concurrent active modals (i.e. dialog, drawer) would try to steal focus from each other - Improved `sl-color-picker` grid and slider handles [#246](https://github.com/shoelace-style/shoelace/issues/246) - Reworked show/hide logic in `sl-alert`, `sl-dialog`, and `sl-drawer` to not use reflow hacks and the `hidden` attribute - Updated to Popper 2.5.3 to address a fixed position bug in Firefox diff --git a/src/components/dialog/dialog.tsx b/src/components/dialog/dialog.tsx index 2eb1a57d..5eb18cd0 100644 --- a/src/components/dialog/dialog.tsx +++ b/src/components/dialog/dialog.tsx @@ -1,6 +1,7 @@ import { Component, Element, Event, EventEmitter, Method, Prop, State, Watch, h } from '@stencil/core'; import { lockBodyScrolling, unlockBodyScrolling } from '../../utilities/scroll'; import { hasSlot } from '../../utilities/slot'; +import Modal from '../../utilities/modal'; let id = 0; @@ -29,6 +30,7 @@ let id = 0; export class Dialog { componentId = `dialog-${++id}`; dialog: HTMLElement; + modal: Modal; panel: HTMLElement; @Element() host: HTMLSlDialogElement; @@ -72,12 +74,15 @@ export class Dialog { @Event({ eventName: 'sl-overlay-dismiss' }) slOverlayDismiss: EventEmitter; connectedCallback() { - this.handleDocumentFocusIn = this.handleDocumentFocusIn.bind(this); this.handleCloseClick = this.handleCloseClick.bind(this); this.handleTransitionEnd = this.handleTransitionEnd.bind(this); this.handleKeyDown = this.handleKeyDown.bind(this); this.handleOverlayClick = this.handleOverlayClick.bind(this); this.updateSlots = this.updateSlots.bind(this); + + this.modal = new Modal(this.host, { + onFocusOut: () => this.panel.focus() + }); } componentWillLoad() { @@ -114,9 +119,9 @@ export class Dialog { this.isVisible = true; this.open = true; + this.modal.activate(); lockBodyScrolling(this.host); - document.addEventListener('focusin', this.handleDocumentFocusIn); } /** Hides the dialog */ @@ -134,23 +139,15 @@ export class Dialog { } this.open = false; + this.modal.deactivate(); unlockBodyScrolling(this.host); - document.removeEventListener('focusin', this.handleDocumentFocusIn); } handleCloseClick() { this.hide(); } - handleDocumentFocusIn(event: Event) { - const target = event.target as HTMLElement; - - if (target.closest('sl-dialog') !== this.host) { - this.panel.focus(); - } - } - handleKeyDown(event: KeyboardEvent) { if (event.key === 'Escape') { this.hide(); diff --git a/src/components/drawer/drawer.tsx b/src/components/drawer/drawer.tsx index 7533fa56..496ef917 100644 --- a/src/components/drawer/drawer.tsx +++ b/src/components/drawer/drawer.tsx @@ -1,6 +1,7 @@ import { Component, Element, Event, EventEmitter, Method, Prop, State, Watch, h } from '@stencil/core'; import { lockBodyScrolling, unlockBodyScrolling } from '../../utilities/scroll'; import { hasSlot } from '../../utilities/slot'; +import Modal from '../../utilities/modal'; let id = 0; @@ -28,6 +29,7 @@ let id = 0; export class Drawer { componentId = `drawer-${++id}`; drawer: HTMLElement; + modal: Modal; panel: HTMLElement; @Element() host: HTMLSlDrawerElement; @@ -82,10 +84,13 @@ export class Drawer { connectedCallback() { this.handleCloseClick = this.handleCloseClick.bind(this); this.handleTransitionEnd = this.handleTransitionEnd.bind(this); - this.handleDocumentFocusIn = this.handleDocumentFocusIn.bind(this); this.handleKeyDown = this.handleKeyDown.bind(this); this.handleOverlayClick = this.handleOverlayClick.bind(this); this.updateSlots = this.updateSlots.bind(this); + + this.modal = new Modal(this.host, { + onFocusOut: () => (this.contained ? null : this.panel.focus()) + }); } componentWillLoad() { @@ -125,10 +130,9 @@ export class Drawer { // Lock body scrolling only if the drawer isn't contained if (!this.contained) { + this.modal.activate(); lockBodyScrolling(this.host); } - - document.addEventListener('focusin', this.handleDocumentFocusIn); } /** Hides the drawer */ @@ -146,24 +150,15 @@ export class Drawer { } this.open = false; + this.modal.deactivate(); unlockBodyScrolling(this.host); - document.removeEventListener('focusin', this.handleDocumentFocusIn); } handleCloseClick() { this.hide(); } - handleDocumentFocusIn(event: Event) { - const target = event.target as HTMLElement; - - // Trap focus only if the drawer is NOT contained - if (!this.contained && target.closest('sl-drawer') !== this.host) { - this.panel.focus(); - } - } - handleKeyDown(event: KeyboardEvent) { if (event.key === 'Escape') { this.hide();