From 217505d0c9bfaec17d598208686ef6de54c41927 Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Sat, 2 Sep 2023 08:14:56 -0700 Subject: [PATCH] web: Simplify, simplify, simplify Sort-of. This commit changes the way the "wizard step coordinator" layer works, giving the wizard writer much more power over button bar. It still assumes there are only three actions the wizard frame wants to commit: next, back, and close. This empowers the steps themselves to re-arrange their buttons and describe the rules through which transitions occur. --- .../admin/applications/wizard/BasePanel.ts | 6 +- .../wizard/ak-application-wizard.ts | 64 +++++++++++-------- ...-application-wizard-application-details.ts | 12 ++-- ...ion-wizard-authentication-method-choice.ts | 7 +- .../wizard/methods/BaseProviderPanel.ts | 12 +++- web/src/admin/applications/wizard/steps.ts | 31 +++++---- web/src/admin/applications/wizard/types.ts | 8 ++- .../ak-wizard-main/ak-wizard-frame.ts | 13 ++-- .../ak-wizard-main/ak-wizard-main.ts | 12 +++- .../stories/ak-wizard-main.stories.ts | 14 ++-- web/src/components/ak-wizard-main/types.ts | 2 +- 11 files changed, 116 insertions(+), 65 deletions(-) diff --git a/web/src/admin/applications/wizard/BasePanel.ts b/web/src/admin/applications/wizard/BasePanel.ts index f0822e7ed..f05396b8b 100644 --- a/web/src/admin/applications/wizard/BasePanel.ts +++ b/web/src/admin/applications/wizard/BasePanel.ts @@ -8,7 +8,7 @@ import { query } from "@lit/reactive-element/decorators.js"; import { styles as AwadStyles } from "./BasePanel.css"; import { applicationWizardContext } from "./ak-application-wizard-context-name"; -import type { WizardState } from "./types"; +import type { WizardState, WizardStateUpdate } from "./types"; export class ApplicationWizardPageBase extends CustomEmitterElement(AKElement) @@ -27,8 +27,8 @@ export class ApplicationWizardPageBase public wizard!: WizardState; // This used to be more complex; now it just establishes the event name. - dispatchWizardUpdate(update: Partial) { - this.dispatchCustomEvent("ak-application-wizard-update", { update }); + dispatchWizardUpdate(update: WizardStateUpdate) { + this.dispatchCustomEvent("ak-application-wizard-update", update); } } diff --git a/web/src/admin/applications/wizard/ak-application-wizard.ts b/web/src/admin/applications/wizard/ak-application-wizard.ts index c5d5d25bb..94ffa741d 100644 --- a/web/src/admin/applications/wizard/ak-application-wizard.ts +++ b/web/src/admin/applications/wizard/ak-application-wizard.ts @@ -1,3 +1,4 @@ +import { type AkWizardMain } from "@goauthentik/app/components/ak-wizard-main/ak-wizard-main"; import { merge } from "@goauthentik/common/merge"; import "@goauthentik/components/ak-wizard-main"; import { AKElement } from "@goauthentik/elements/Base"; @@ -7,6 +8,7 @@ import { ContextProvider, ContextRoot } from "@lit-labs/context"; import { msg } from "@lit/localize"; import { CSSResult, html } from "lit"; import { customElement, property, state } from "lit/decorators.js"; +import { type Ref, createRef, ref } from "lit/directives/ref.js"; import PFButton from "@patternfly/patternfly/components/Button/button.css"; import PFRadio from "@patternfly/patternfly/components/Radio/radio.css"; @@ -16,11 +18,6 @@ import applicationWizardContext from "./ak-application-wizard-context-name"; import { steps } from "./steps"; import { OneOfProvider, WizardState, WizardStateEvent } from "./types"; -// my-context.ts - -// All this thing is doing is recording the input the user makes to the forms. It should NOT be -// triggering re-renders; that's the wizard frame's jobs. - @customElement("ak-application-wizard") export class ApplicationWizard extends CustomListenerElement(AKElement) { static get styles(): CSSResult[] { @@ -29,7 +26,6 @@ export class ApplicationWizard extends CustomListenerElement(AKElement) { @state() wizardState: WizardState = { - step: 0, providerModel: "", app: {}, provider: {}, @@ -43,6 +39,7 @@ export class ApplicationWizard extends CustomListenerElement(AKElement) { initialValue: this.wizardState, }); + @state() steps = steps; @property() @@ -50,6 +47,12 @@ export class ApplicationWizard extends CustomListenerElement(AKElement) { providerCache: Map = new Map(); + wizardRef: Ref = createRef(); + + get step() { + return this.wizardRef.value?.currentStep ?? -1; + } + constructor() { super(); this.handleUpdate = this.handleUpdate.bind(this); @@ -66,30 +69,38 @@ export class ApplicationWizard extends CustomListenerElement(AKElement) { super.disconnectedCallback(); } + maybeProviderSwap(providerModel: string | undefined): boolean { + if ( + providerModel === undefined || + typeof providerModel !== "string" || + providerModel === this.wizardState.providerModel + ) { + return false; + } + + this.providerCache.set(this.wizardState.providerModel, this.wizardState.provider); + const prevProvider = this.providerCache.get(providerModel); + this.wizardState.provider = prevProvider ?? { + name: `Provider for ${this.wizardState.app.name}`, + }; + const method = this.steps.find(({ id }) => id === "provider-details"); + if (!method) { + throw new Error("Could not find Authentication Method page?"); + } + method.disabled = false; + } + // And this is where all the special cases go... handleUpdate(event: CustomEvent) { const update = event.detail.update; - // Are we changing provider type? If so, swap the caches of the various provider types the - // user may have filled in, and enable the next step. - const providerModel = update.providerModel; - if ( - providerModel && - typeof providerModel === "string" && - providerModel !== this.wizardState.providerModel - ) { - this.providerCache.set(this.wizardState.providerModel, this.wizardState.provider); - const prevProvider = this.providerCache.get(providerModel); - this.wizardState.provider = prevProvider ?? { - name: `Provider for ${this.wizardState.app.name}`, - }; - const newSteps = [...this.steps]; - const method = newSteps.find(({ id }) => id === "auth-method"); - if (!method) { - throw new Error("Could not find Authentication Method page?"); - } - method.disabled = false; - this.steps = newSteps; + if (this.maybeProviderSwap(update.providerModel)) { + this.steps = [...this.steps]; + } + + if (event.detail.status === "valid" && this.steps[this.step + 1]) { + this.steps[this.step + 1].disabled = false; + this.steps = [...this.steps]; } this.wizardState = merge(this.wizardState, update) as WizardState; @@ -99,6 +110,7 @@ export class ApplicationWizard extends CustomListenerElement(AKElement) { render() { return html` radio instanceof HTMLInputElement && radio.checked, ); - return chosen; + return !!chosen; } renderProvider(type: LocalTypeCreate) { diff --git a/web/src/admin/applications/wizard/methods/BaseProviderPanel.ts b/web/src/admin/applications/wizard/methods/BaseProviderPanel.ts index d41a9392d..22ad94413 100644 --- a/web/src/admin/applications/wizard/methods/BaseProviderPanel.ts +++ b/web/src/admin/applications/wizard/methods/BaseProviderPanel.ts @@ -9,12 +9,20 @@ export class ApplicationWizardProviderPageBase extends BasePanel { const target = ev.target as HTMLInputElement; const value = target.type === "checkbox" ? target.checked : target.value; this.dispatchWizardUpdate({ - provider: { - [target.name]: value, + update: { + provider: { + [target.name]: value, + }, }, + status: this.form.checkValidity() ? "valid" : "invalid" }); } + shouldUpdate(changed: Map) { + console.log("CHANGED:", JSON.stringify(Array.from(changed.entries()), null, 2)); + return true; + } + validator() { return this.form.reportValidity(); } diff --git a/web/src/admin/applications/wizard/steps.ts b/web/src/admin/applications/wizard/steps.ts index e1f6171fb..125383bec 100644 --- a/web/src/admin/applications/wizard/steps.ts +++ b/web/src/admin/applications/wizard/steps.ts @@ -2,12 +2,10 @@ import { WizardStep } from "@goauthentik/components/ak-wizard-main"; import { BackStep, CancelWizard, - CloseWizard, NextStep, SubmitStep, } from "@goauthentik/components/ak-wizard-main/commonWizardButtons"; -import { msg } from "@lit/localize"; import { html } from "lit"; import "./application/ak-application-wizard-application-details"; @@ -15,41 +13,46 @@ import "./auth-method-choice/ak-application-wizard-authentication-method-choice" import "./commit/ak-application-wizard-commit-application"; import "./methods/ak-application-wizard-authentication-method"; -export const steps: WizardStep[] = [ +type NamedStep = WizardStep & { + id: string, + valid: boolean, +}; + +export const steps: NamedStep[] = [ { id: "application", label: "Application Details", - renderer: () => + render: () => html``, disabled: false, + valid: false, buttons: [NextStep, CancelWizard], - valid: true, }, { - id: "auth-method-choice", + id: "provider-method", label: "Authentication Method", - renderer: () => + render: () => html``, disabled: false, + valid: false, buttons: [NextStep, BackStep, CancelWizard], - valid: true, }, { - id: "auth-method", + id: "provider-details", label: "Authentication Details", - renderer: () => + render: () => html``, disabled: true, + valid: false, buttons: [SubmitStep, BackStep, CancelWizard], - valid: true, }, { - id: "commit-application", + id: "submit", label: "Submit New Application", - renderer: () => + render: () => html``, disabled: true, + valid: false, buttons: [BackStep, CancelWizard], - valid: true, }, ]; diff --git a/web/src/admin/applications/wizard/types.ts b/web/src/admin/applications/wizard/types.ts index 12cce4f9c..0f3403367 100644 --- a/web/src/admin/applications/wizard/types.ts +++ b/web/src/admin/applications/wizard/types.ts @@ -17,10 +17,14 @@ export type OneOfProvider = | Partial; export interface WizardState { - step: number; providerModel: string; app: Partial; provider: OneOfProvider; } -export type WizardStateEvent = { update: Partial }; +type StatusType = "invalid" | "valid" | "submitted" | "failed"; + +export type WizardStateUpdate = { + update: Partial, + status?: StatusType, +}; diff --git a/web/src/components/ak-wizard-main/ak-wizard-frame.ts b/web/src/components/ak-wizard-main/ak-wizard-frame.ts index 0237a31ff..6c27c7e1f 100644 --- a/web/src/components/ak-wizard-main/ak-wizard-frame.ts +++ b/web/src/components/ak-wizard-main/ak-wizard-frame.ts @@ -104,14 +104,19 @@ export class AkWizardFrame extends CustomEmitterElement(ModalButton) { } renderNavigation() { + let disabled = false; + return html``; } - renderNavigationStep(step: WizardStep, idx: number) { + renderNavigationStep(step: WizardStep, disabled: boolean, idx: number) { const buttonClasses = { "pf-c-wizard__nav-link": true, "pf-m-current": idx === this.currentStep, @@ -121,7 +126,7 @@ export class AkWizardFrame extends CustomEmitterElement(ModalButton) {