From 3d2bd4d8dd5ec2ba0b5197beb1cedbaff6eba1ec Mon Sep 17 00:00:00 2001 From: Teffen Ellis <592134+GirlBossRush@users.noreply.github.com> Date: Fri, 14 Feb 2025 02:12:46 +0100 Subject: [PATCH] web: Fix issues surrounding wizard step behavior. (#12779) This resolves a few stateful situations which may arise when opening and closing wizard pages. --- web/src/admin/policies/PolicyWizard.ts | 32 +- web/src/elements/wizard/ActionWizardPage.ts | 13 + .../elements/wizard/TypeCreateWizardPage.ts | 35 ++- web/src/elements/wizard/Wizard.ts | 292 +++++++++++------- web/src/elements/wizard/WizardFormPage.ts | 14 +- web/src/elements/wizard/WizardPage.ts | 39 ++- 6 files changed, 287 insertions(+), 138 deletions(-) diff --git a/web/src/admin/policies/PolicyWizard.ts b/web/src/admin/policies/PolicyWizard.ts index 94817c71eeff..46c86322094c 100644 --- a/web/src/admin/policies/PolicyWizard.ts +++ b/web/src/admin/policies/PolicyWizard.ts @@ -52,6 +52,21 @@ export class PolicyWizard extends AKElement { }); } + selectListener = ({ detail }: CustomEvent) => { + if (!this.wizard) return; + + const { component, modelName } = detail; + const idx = this.wizard.steps.indexOf("initial") + 1; + + // Exclude all current steps starting with type-, + // this happens when the user selects a type and then goes back + this.wizard.steps = this.wizard.steps.filter((step) => !step.startsWith("type-")); + + this.wizard.steps.splice(idx, 0, `type-${component}-${modelName}`); + + this.wizard.isValid = true; + }; + render(): TemplateResult { return html` ) => { - if (!this.wizard) return; - const idx = this.wizard.steps.indexOf("initial") + 1; - // Exclude all current steps starting with type-, - // this happens when the user selects a type and then goes back - this.wizard.steps = this.wizard.steps.filter( - (step) => !step.startsWith("type-"), - ); - this.wizard.steps.splice( - idx, - 0, - `type-${ev.detail.component}-${ev.detail.modelName}`, - ); - this.wizard.isValid = true; - }} + @select=${this.selectListener} > + ${this.policyTypes.map((type) => { return html` => { this.states = []; + this.host.actions.map((act, idx) => { this.states.push({ action: act, @@ -48,9 +49,12 @@ export class ActionWizardPage extends WizardPage { idx: idx, }); }); + this.host.canBack = false; this.host.canCancel = false; + await this.run(); + // Ensure wizard is closable, even when run() failed this.host.isValid = true; }; @@ -59,15 +63,20 @@ export class ActionWizardPage extends WizardPage { async run(): Promise { this.currentStep = this.states[0]; + await new Promise((r) => setTimeout(r, 500)); + for await (const bundle of this.states) { this.currentStep = bundle; this.currentStep.state = ActionState.running; this.requestUpdate(); try { await bundle.action.run(); + await new Promise((r) => setTimeout(r, 500)); + this.currentStep.state = ActionState.done; + this.requestUpdate(); } catch (exc) { if (exc instanceof ResponseError) { @@ -75,12 +84,16 @@ export class ActionWizardPage extends WizardPage { } else { this.currentStep.action.subText = (exc as Error).toString(); } + this.currentStep.state = ActionState.failed; this.requestUpdate(); + return; } } + this.host.isValid = true; + this.dispatchEvent( new CustomEvent(EVENT_REFRESH, { bubbles: true, diff --git a/web/src/elements/wizard/TypeCreateWizardPage.ts b/web/src/elements/wizard/TypeCreateWizardPage.ts index f9bfc390c62d..684a16a5eb50 100644 --- a/web/src/elements/wizard/TypeCreateWizardPage.ts +++ b/web/src/elements/wizard/TypeCreateWizardPage.ts @@ -5,6 +5,7 @@ import { WizardPage } from "@goauthentik/elements/wizard/WizardPage"; import { msg, str } from "@lit/localize"; import { CSSResult, TemplateResult, css, html, nothing } from "lit"; import { customElement, property } from "lit/decorators.js"; +import { Ref, createRef, ref } from "lit/directives/ref.js"; import PFCard from "@patternfly/patternfly/components/Card/card.css"; import PFForm from "@patternfly/patternfly/components/Form/form.css"; @@ -21,6 +22,8 @@ export enum TypeCreateWizardPageLayouts { @customElement("ak-wizard-page-type-create") export class TypeCreateWizardPage extends WithLicenseSummary(WizardPage) { + //#region Properties + @property({ attribute: false }) types: TypeCreate[] = []; @@ -30,6 +33,8 @@ export class TypeCreateWizardPage extends WithLicenseSummary(WizardPage) { @property({ type: String }) layout: TypeCreateWizardPageLayouts = TypeCreateWizardPageLayouts.list; + //#endregion + static get styles(): CSSResult[] { return [ PFBase, @@ -49,10 +54,25 @@ export class TypeCreateWizardPage extends WithLicenseSummary(WizardPage) { ]; } - sidebarLabel = () => msg("Select type"); + //#region Refs + + formRef: Ref = createRef(); + + //#endregion + + public sidebarLabel = () => msg("Select type"); + + public reset = () => { + super.reset(); + this.selectedType = undefined; + this.formRef.value?.reset(); + }; + + activeCallback = (): void => { + const form = this.formRef.value; + + this.host.isValid = form?.checkValidity() ?? false; - activeCallback: () => Promise = async () => { - this.host.isValid = false; if (this.selectedType) { this.selectDispatch(this.selectedType); } @@ -92,9 +112,8 @@ export class TypeCreateWizardPage extends WithLicenseSummary(WizardPage) { data-ouid-component-type="ak-type-create-grid-card" data-ouid-component-name=${componentName} @click=${() => { - if (requiresEnterprise) { - return; - } + if (requiresEnterprise) return; + this.selectDispatch(type); this.selectedType = type; }} @@ -120,11 +139,13 @@ export class TypeCreateWizardPage extends WithLicenseSummary(WizardPage) { renderList(): TemplateResult { return html`
${this.types.map((type) => { const requiresEnterprise = type.requiresEnterprise && !this.hasEnterpriseLicense; + return html`
{ + return Promise.resolve(); + }; + + @property({ attribute: false }) + state: { [key: string]: unknown } = {}; + + //#endregion + + //#region State + + /** + * Memoized step tag names. + */ @state() _steps: string[] = []; + /** + * Step tag names present in the wizard. + */ get steps(): string[] { return this._steps; } - set steps(steps: string[]) { - const addApplyActionsSlot = this.steps.includes(ApplyActionsSlot); - this._steps = steps; + set steps(nextSteps: string[]) { + const addApplyActionsSlot = this._steps.includes(ApplyActionsSlot); + + this._steps = nextSteps; + if (addApplyActionsSlot) { this.steps.push(ApplyActionsSlot); } - this.steps.forEach((step) => { - const exists = this.querySelector(`[slot=${step}]`) !== null; - if (!exists) { - const el = document.createElement(step); - el.slot = step; - el.dataset["wizardmanaged"] = "true"; - this.appendChild(el); - } - }); + + for (const step of this._steps) { + const existingStepElement = this.getStepElementByName(step); + + if (existingStepElement) continue; + + const stepElement = document.createElement(step); + + stepElement.slot = step; + stepElement.dataset["wizardmanaged"] = "true"; + + this.appendChild(stepElement); + } + this.requestUpdate(); } + /** + * Initial steps to reset to. + */ _initialSteps: string[] = []; - @property({ attribute: false }) - actions: WizardAction[] = []; - @state() - _currentStep?: WizardPage; + _activeStep?: WizardPage; - set currentStep(value: WizardPage | undefined) { - this._currentStep = value; - if (this._currentStep) { - this._currentStep.activeCallback(); - this._currentStep.requestUpdate(); - } + set activeStepElement(nextActiveStepElement: WizardPage | undefined) { + this._activeStep = nextActiveStepElement; + + if (!this._activeStep) return; + + this._activeStep.activeCallback(); + this._activeStep.requestUpdate(); } - get currentStep(): WizardPage | undefined { - return this._currentStep; + /** + * The active step element being displayed. + */ + get activeStepElement(): WizardPage | undefined { + return this._activeStep; } - @property({ attribute: false }) - finalHandler: () => Promise = () => { - return Promise.resolve(); - }; + getStepElementByIndex(stepIndex: number): WizardPage | null { + const stepName = this._steps[stepIndex]; - @property({ attribute: false }) - state: { [key: string]: unknown } = {}; + return this.querySelector(`[slot=${stepName}]`); + } + + getStepElementByName(stepName: string): WizardPage | null { + return this.querySelector(`[slot=${stepName}]`); + } + + //#endregion + + //#region Lifecycle firstUpdated(): void { this._initialSteps = this._steps; } /** - * Add action to the beginning of the list + * Add action to the beginning of the list. */ addActionBefore(displayName: string, run: () => Promise): void { this.actions.unshift({ @@ -114,7 +173,9 @@ export class Wizard extends ModalButton { } /** - * Add action at the end of the list + * Add action at the end of the list. + * + * @todo: Is this used? */ addActionAfter(displayName: string, run: () => Promise): void { this.actions.push({ @@ -123,17 +184,60 @@ export class Wizard extends ModalButton { }); } + /** + * Reset the wizard to it's initial state. + */ + reset = () => { + this.open = false; + + this.querySelectorAll("[data-wizardmanaged=true]").forEach((el) => { + el.remove(); + }); + + for (const step of this.steps) { + const stepElement = this.getStepElementByName(step); + + stepElement?.reset?.(); + } + + this.steps = this._initialSteps; + this.actions = []; + this.state = {}; + this.activeStepElement = undefined; + this.canBack = true; + this.canCancel = true; + }; + + //#endregion + + //#region Rendering + renderModalInner(): TemplateResult { - const firstPage = this.querySelector(`[slot=${this.steps[0]}]`); - if (!this.currentStep && firstPage) { - this.currentStep = firstPage; + const firstPage = this.getStepElementByIndex(0); + + if (!this.activeStepElement && firstPage) { + this.activeStepElement = firstPage; } - const currentIndex = this.currentStep ? this.steps.indexOf(this.currentStep.slot) : 0; - let lastPage = currentIndex === this.steps.length - 1; + + const activeStepIndex = this.activeStepElement + ? this.steps.indexOf(this.activeStepElement.slot) + : 0; + + let lastPage = activeStepIndex === this.steps.length - 1; + if (lastPage && !this.steps.includes("ak-wizard-page-action") && this.actions.length > 0) { this.steps = this.steps.concat("ak-wizard-page-action"); - lastPage = currentIndex === this.steps.length - 1; + lastPage = activeStepIndex === this.steps.length - 1; } + + const navigateToPreviousStep = () => { + const prevPage = this.getStepElementByIndex(activeStepIndex - 1); + + if (prevPage) { + this.activeStepElement = prevPage; + } + }; + return html`
${this.canCancel @@ -141,13 +245,11 @@ export class Wizard extends ModalButton { class="pf-c-button pf-m-plain pf-c-wizard__close" type="button" aria-label="${msg("Close")}" - @click=${() => { - this.reset(); - }} + @click=${this.reset} > ` - : html``} + : nothing}

${this.header}

${this.description}

@@ -156,28 +258,25 @@ export class Wizard extends ModalButton {
- +
@@ -196,44 +295,38 @@ export class Wizard extends ModalButton { type="submit" ?disabled=${!this.isValid} @click=${async () => { - const cb = await this.currentStep?.nextCallback(); - if (!cb) { - return; - } + const completedStep = await this.activeStepElement?.nextCallback(); + if (!completedStep) return; + if (lastPage) { await this.finalHandler(); this.reset(); - } else { - const nextPage = this.querySelector( - `[slot=${this.steps[currentIndex + 1]}]`, - ); - if (nextPage) { - this.currentStep = nextPage; - } + + return; + } + + const nextPage = this.getStepElementByIndex(activeStepIndex + 1); + + if (nextPage) { + this.activeStepElement = nextPage; } }} > ${lastPage ? msg("Finish") : msg("Next")} - ${(this.currentStep ? this.steps.indexOf(this.currentStep.slot) : 0) > 0 && - this.canBack + ${(this.activeStepElement + ? this.steps.indexOf(this.activeStepElement.slot) + : 0) > 0 && this.canBack ? html` ` - : html``} + : nothing} ${this.canCancel ? html`
` - : html``} + : nothing}
`; } - reset(): void { - this.open = false; - this.querySelectorAll("[data-wizardmanaged=true]").forEach((el) => { - el.remove(); - }); - this.steps = this._initialSteps; - this.actions = []; - this.state = {}; - this.currentStep = undefined; - this.canBack = true; - this.canCancel = true; - } + //#endregion } declare global { diff --git a/web/src/elements/wizard/WizardFormPage.ts b/web/src/elements/wizard/WizardFormPage.ts index 9fc2fd2c7a04..408f652db4f6 100644 --- a/web/src/elements/wizard/WizardFormPage.ts +++ b/web/src/elements/wizard/WizardFormPage.ts @@ -39,24 +39,24 @@ export class WizardFormPage extends WizardPage { inputCallback(): void { const form = this.shadowRoot?.querySelector("form"); - if (!form) { - return; - } + + if (!form) return; + const state = form.checkValidity(); this.host.isValid = state; } nextCallback = async (): Promise => { const form = this.shadowRoot?.querySelector("ak-wizard-form"); + if (!form) { console.warn("authentik/wizard: could not find form element"); return false; } + const response = await form.submit(); - if (response === undefined) { - return false; - } - return response; + + return Boolean(response); }; nextDataCallback: (data: KeyUnknown) => Promise = async (): Promise => { diff --git a/web/src/elements/wizard/WizardPage.ts b/web/src/elements/wizard/WizardPage.ts index 6725cd303ea9..a2b559155af2 100644 --- a/web/src/elements/wizard/WizardPage.ts +++ b/web/src/elements/wizard/WizardPage.ts @@ -6,14 +6,32 @@ import { customElement, property } from "lit/decorators.js"; import PFBase from "@patternfly/patternfly/patternfly-base.css"; +/** + * Callback for when the page is brought into view. + */ +export type WizardPageActiveCallback = () => void | Promise; + +/** + * Callback for when the next button is pressed. + * + * @returns `true` if the wizard can proceed to the next page, `false` otherwise. + */ +export type WizardPageNextCallback = () => boolean | Promise; + @customElement("ak-wizard-page") export class WizardPage extends AKElement { static get styles(): CSSResult[] { return [PFBase]; } + /** + * The label to display in the sidebar for this page. + * + * Override this to provide a custom label. + * @todo: Should this be a getter or static property? + */ @property() - sidebarLabel: () => string = () => { + sidebarLabel = (): string => { return "UNNAMED"; }; @@ -22,9 +40,18 @@ export class WizardPage extends AKElement { } /** - * Called when this is the page brought into view + * Reset the page to its initial state. + * + * @abstract + */ + public reset(): void | Promise { + console.debug(`authentik/wizard ${this.localName}: reset)`); + } + + /** + * Called when this is the page brought into view. */ - activeCallback: () => Promise = async () => { + activeCallback: WizardPageActiveCallback = () => { this.host.isValid = false; }; @@ -32,9 +59,11 @@ export class WizardPage extends AKElement { * Called when the `next` button on the wizard is pressed. For forms, results in the submission * of the current form to the back-end before being allowed to proceed to the next page. This is * sub-optimal if we want to collect multiple bits of data before finishing the whole course. + * + * @returns `true` if the wizard can proceed to the next page, `false` otherwise. */ - nextCallback: () => Promise = async () => { - return true; + nextCallback: WizardPageNextCallback = () => { + return Promise.resolve(true); }; requestUpdate(