-
-
Notifications
You must be signed in to change notification settings - Fork 995
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
web: manage stacked modals with a stack (#9193)
* web: fix esbuild issue with style sheets
Getting ESBuild, Lit, and Storybook to all agree on how to read and parse stylesheets is a serious
pain. This fix better identifies the value types (instances) being passed from various sources in
the repo to the three *different* kinds of style processors we're using (the native one, the
polyfill one, and whatever the heck Storybook does internally).
Falling back to using older CSS instantiating techniques one era at a time seems to do the trick.
It's ugly, but in the face of the aggressive styling we use to avoid Flashes of Unstyled Content
(FLoUC), it's the logic with which we're left.
In standard mode, the following warning appears on the console when running a Flow:
```
Autofocus processing was blocked because a document already has a focused element.
```
In compatibility mode, the following **error** appears on the console when running a Flow:
```
crawler-inject.js:1106 Uncaught TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'.
at initDomMutationObservers (crawler-inject.js:1106:18)
at crawler-inject.js:1114:24
at Array.forEach (<anonymous>)
at initDomMutationObservers (crawler-inject.js:1114:10)
at crawler-inject.js:1549:1
initDomMutationObservers @ crawler-inject.js:1106
(anonymous) @ crawler-inject.js:1114
initDomMutationObservers @ crawler-inject.js:1114
(anonymous) @ crawler-inject.js:1549
```
Despite this error, nothing seems to be broken and flows work as anticipated.
* web: manage stacked modals with a stack
"Events flow up. Instructions flow down."
This commit creates a top-level listening controller associated with the main
Interface that listens for ModalShow events and registers the modal with a
stack. When it receives a corresponding KeyUp:Escape, it closes the topmost
modal and removes all references to that modal from the stack. When it receives
a ModalHide event, it removes all references to the target modal and removes all
references to that modal from the stack.
This commit includes a few new techniques. First, thanks to Justin Fagnani and
the Shoelace team, this commit includes an alternative technique for declaring
custom events by leveraging the GlobalEventHandlers type. This actually works
better: the event is explicit, easy to understand, and the typescript language
server actually gets them to correspond correctly; if you listen for a specific
custom event, the handler had better be of the right type to receive that
specific event!
Second, this introduces the first custom decorator, @bound(), which eliminates
the need to say `this.eventHandler = this.eventHandler.bind(this)` from event
handling methods that will have to be passed outside the `this` context of an
HTMLElement. After conducting several experiments to see if I understood the
PropertyDescriptor protocol correctly, I conclud that this is a safe technique
for wiring up `removeEventListener()` handlers.
* Prettier had opinions.
* web: manage stacked modals with a stack
By reviewer request, the `.closeModal()` protocol has been updated
so that if the method returns `false` (explicitly; `undefined` is
not `false`!), the `.closeModal()` protocol is aborted, the modal
remains at the top of the stack, and cleanup is not initiated.
Modal forms can now have an "are you sure?" pass if the user triggers
a close without saving the form. Figuring out how to close *two*
modals if the user *is* sure, and making the Form modal return `true`
when the user *is* sure, are left for a future exercise. :-)
* web: fix stack handling bug for `Escape`, and make Lint happier about loops- Loading branch information
1 parent
6df2875
commit 8e4929c
Showing
6 changed files
with
176 additions
and
30 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
122 changes: 122 additions & 0 deletions
122
web/src/elements/controllers/ModalOrchestrationController.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import { bound } from "@goauthentik/elements/decorators/bound.js"; | ||
|
|
||
| import { LitElement, ReactiveController, ReactiveControllerHost } from "lit"; | ||
|
|
||
| type ReactiveElementHost = Partial<ReactiveControllerHost> & LitElement; | ||
|
|
||
| type ModalElement = LitElement & { closeModal(): void | boolean }; | ||
|
|
||
| export class ModalShowEvent extends Event { | ||
| modal: ModalElement; | ||
| constructor(modal: ModalElement) { | ||
| super("ak-modal-show", { bubbles: true, composed: true }); | ||
| this.modal = modal; | ||
| } | ||
| } | ||
|
|
||
| export class ModalHideEvent extends Event { | ||
| modal: ModalElement; | ||
| constructor(modal: ModalElement) { | ||
| super("ak-modal-hide", { bubbles: true, composed: true }); | ||
| this.modal = modal; | ||
| } | ||
| } | ||
|
|
||
| declare global { | ||
| interface GlobalEventHandlersEventMap { | ||
| "ak-modal-show": ModalShowEvent; | ||
| "ak-modal-hide": ModalHideEvent; | ||
| } | ||
| } | ||
|
|
||
| const modalIsLive = (modal: ModalElement) => modal.isConnected && modal.checkVisibility(); | ||
|
|
||
| /** | ||
| * class ModalOrchetrationController | ||
| * | ||
| * A top-level controller that listens for requests from modals to be added to | ||
| * the management list, such that the *topmost* modal will be closed (and all | ||
| * references to it eliminated) whenever the user presses the Escape key. | ||
| * Can also take ModalHideEvent requests and automatically close the modal | ||
| * sending the event. | ||
| * | ||
| * Both events that this responds to expect a reference to the modal to be part | ||
| * of the event payload. | ||
| * | ||
| * If the `.closeModal()` method on the target modal returns `false` | ||
| * *explicitly*, it will abort cleanup and the stack will keep the record that | ||
| * the modal is still open. This allows `.closeModal()` to return `undefined` | ||
| * and still behave correctly. | ||
| */ | ||
|
|
||
| export class ModalOrchestrationController implements ReactiveController { | ||
| host!: ReactiveElementHost; | ||
|
|
||
| knownModals: ModalElement[] = []; | ||
|
|
||
| constructor(host: ReactiveElementHost) { | ||
| this.host = host; | ||
| host.addController(this); | ||
| } | ||
|
|
||
| hostConnected() { | ||
| window.addEventListener("keyup", this.handleKeyup); | ||
| window.addEventListener("ak-modal-show", this.addModal); | ||
| window.addEventListener("ak-modal-hide", this.closeModal); | ||
| } | ||
|
|
||
| hostDisconnected() { | ||
| window.removeEventListener("keyup", this.handleKeyup); | ||
| window.removeEventListener("ak-modal-show", this.addModal); | ||
| window.removeEventListener("ak-modal-hide", this.closeModal); | ||
| } | ||
|
|
||
| @bound | ||
| addModal(e: ModalShowEvent) { | ||
| this.knownModals = [...this.knownModals, e.modal]; | ||
| } | ||
|
|
||
| scheduleCleanup(modal: ModalElement) { | ||
| setTimeout(() => { | ||
| this.knownModals = this.knownModals.filter((m) => modalIsLive(m) && modal !== m); | ||
| }, 0); | ||
| } | ||
|
|
||
| @bound | ||
| closeModal(e: ModalHideEvent) { | ||
| const modal = e.modal; | ||
| if (!modalIsLive(modal)) { | ||
| return; | ||
| } | ||
| if (modal.closeModal() !== false) { | ||
| this.scheduleCleanup(modal); | ||
| } | ||
| } | ||
|
|
||
| removeTopmostModal() { | ||
| let checking = true; | ||
| while (checking) { | ||
| const modal = this.knownModals.pop(); | ||
| if (!modal) { | ||
| break; | ||
| } | ||
| if (!modalIsLive(modal)) { | ||
| continue; | ||
| } | ||
|
|
||
| if (modal.closeModal() !== false) { | ||
| this.scheduleCleanup(modal); | ||
| } | ||
| checking = false; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| @bound | ||
| handleKeyup(e: KeyboardEvent) { | ||
| // The latter handles Firefox 37 and earlier. | ||
| if (e.key === "Escape" || e.key === "Esc") { | ||
| this.removeTopmostModal(); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| // Automatically binds a method to the `this` instance during instantiation. | ||
| // Uses the Typescript Experimental Decorator syntax, so we may be living with | ||
| // that for a long time. | ||
|
|
||
| // MDN is *not* very helpful. The type for a PropertyDescriptor is kept in | ||
| // typescript/lib/lib.es5.d.ts, but the description of what everything in | ||
| // a descriptor does isn't specified in MDN in its own page, only in | ||
| // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty | ||
|
|
||
| // This decorator feels awkward. It gets a new instance of the method every time | ||
| // you reference the field. I wonder if there would be a way to create a lookup | ||
| // table; once you'd bound the method you could reuse that bound method for that | ||
| // instance, instead of throwing it away? | ||
|
|
||
| export function bound( | ||
| target: unknown, | ||
| key: string, | ||
| descriptor: PropertyDescriptor, | ||
| ): PropertyDescriptor { | ||
| if (typeof descriptor?.value !== "function") { | ||
| throw new Error("Only methods can be @bound."); | ||
| } | ||
| return { | ||
| configurable: true, | ||
| get() { | ||
| const method = descriptor.value.bind(this); | ||
| Object.defineProperty(this, key, { value: method, configurable: true, writable: true }); | ||
| return method; | ||
| }, | ||
| }; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters