From 48083ac380263abd01acc2a1d2dc32a477cdc0cf Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Sun, 30 Jul 2023 09:51:19 -0700 Subject: [PATCH] web: Legibility in the ApplicationForm. This is a pretty good result. By using the LightDOM setting, this provides the existing Authentik form manager with access to the ak-form-horizontal-element components without having to do any cross-border magic. It's not ideal, and it shows up just how badly we've got patternfly splattered everywhere, but the actual results are remarkable. The patterns for text, switch, radio, textarea, file, and even select are smaller and easier here. I'm still noodling on what an unspread search-select element would look like. It's just dependency injection, so it ought to be as straightforward as that. --- web/src/admin/applications/ApplicationForm.ts | 334 ++++++++---------- web/src/admin/applications/renderers.ts | 35 -- .../renderers/ak-backchannel-input.ts | 108 ++++++ .../applications/renderers/ak-file-input.ts | 85 +++++ .../renderers/ak-provider-search-input.ts | 80 +++++ .../applications/renderers/ak-radio-input.ts | 77 ++++ .../applications/renderers/ak-switch-input.ts | 84 +++++ .../applications/renderers/ak-text-input.ts | 76 ++++ .../renderers/ak-textarea-input.ts | 75 ++++ web/src/elements/forms/Form.ts | 6 +- .../elements/forms/HorizontalFormElement.ts | 4 +- 11 files changed, 734 insertions(+), 230 deletions(-) delete mode 100644 web/src/admin/applications/renderers.ts create mode 100644 web/src/admin/applications/renderers/ak-backchannel-input.ts create mode 100644 web/src/admin/applications/renderers/ak-file-input.ts create mode 100644 web/src/admin/applications/renderers/ak-provider-search-input.ts create mode 100644 web/src/admin/applications/renderers/ak-radio-input.ts create mode 100644 web/src/admin/applications/renderers/ak-switch-input.ts create mode 100644 web/src/admin/applications/renderers/ak-text-input.ts create mode 100644 web/src/admin/applications/renderers/ak-textarea-input.ts diff --git a/web/src/admin/applications/ApplicationForm.ts b/web/src/admin/applications/ApplicationForm.ts index 9a8f3e8ce..2bbb03f30 100644 --- a/web/src/admin/applications/ApplicationForm.ts +++ b/web/src/admin/applications/ApplicationForm.ts @@ -1,7 +1,7 @@ import "@goauthentik/admin/applications/ProviderSelectModal"; import { iconHelperText } from "@goauthentik/admin/helperText"; import { DEFAULT_CONFIG, config } from "@goauthentik/common/api/config"; -import { first, groupBy } from "@goauthentik/common/utils"; +import { first } from "@goauthentik/common/utils"; import { rootInterface } from "@goauthentik/elements/Base"; import "@goauthentik/elements/forms/FormGroup"; import "@goauthentik/elements/forms/HorizontalFormElement"; @@ -22,14 +22,39 @@ import { CoreApi, PolicyEngineMode, Provider, - ProvidersAllListRequest, - ProvidersApi, } from "@goauthentik/api"; -import { akText } from "./renderers"; +import "./renderers/ak-backchannel-input"; +import "./renderers/ak-file-input"; +import "./renderers/ak-provider-search-input"; +import "./renderers/ak-radio-input"; +import "./renderers/ak-switch-input"; +import "./renderers/ak-text-input"; +import "./renderers/ak-textarea-input"; + +const policyOptions = [ + { + label: "any", + value: PolicyEngineMode.Any, + default: true, + description: html`${msg("Any policy must match to grant access")}`, + }, + { + label: "all", + value: PolicyEngineMode.All, + description: html`${msg("All policies must match to grant access")}`, + }, +]; @customElement("ak-application-form") export class ApplicationForm extends ModelForm { + constructor() { + super(); + this.handleConfirmBackchannelProviders = this.handleConfirmBackchannelProviders.bind(this); + this.makeRemoveBackchannelProviderHandler = + this.makeRemoveBackchannelProviderHandler.bind(this); + } + async loadInstance(pk: string): Promise { const app = await new CoreApi(DEFAULT_CONFIG).coreApplicationsRetrieve({ slug: pk, @@ -90,200 +115,131 @@ export class ApplicationForm extends ModelForm { return app; } + handleConfirmBackchannelProviders({ items }: { items: Provider[] }) { + this.backchannelProviders = items; + this.requestUpdate(); + return Promise.resolve(); + } + + makeRemoveBackchannelProviderHandler(provider: Provider) { + return () => { + const idx = this.backchannelProviders.indexOf(provider); + this.backchannelProviders.splice(idx, 1); + this.requestUpdate(); + }; + } + + handleClearIcon(ev: Event) { + ev.stopPropagation(); + if (!(ev instanceof InputEvent) || !ev.target) { + return; + } + this.clearIcon = !!(ev.target as HTMLInputElement).checked; + } + renderForm(): TemplateResult { return html`
- ${akText({ - name: "name", - value: this.instance?.name, - label: msg("Name"), - required: true, - help: msg("Application's display Name."), - })} - ${akText({ - name: "slug", - value: this.instance?.slug, - label: msg("Slug"), - required: true, - help: msg("Internal application name, used in URLs."), - })} - ${akText({ - name: "group", - value: this.instance?.group, - label: msg("Group"), - help: msg( - "Optionally enter a group name. Applications with identical groups are shown grouped together." - ), - })} - + + + + -
- { - this.backchannelProviders = items; - this.requestUpdate(); - return Promise.resolve(); - }} - > - - -
- - ${this.backchannelProviders.map((provider) => { - return html` { - const idx = this.backchannelProviders.indexOf(provider); - this.backchannelProviders.splice(idx, 1); - this.requestUpdate(); - }} - > - ${provider.name} - `; - })} - -
-
-

- ${msg( - "Select backchannel providers which augment the functionality of the main provider." - )} -

- - - + - - - + .options=${policyOptions} + .value=${this.instance?.policyEngineMode} + > ${msg("UI settings")}
- - -

- ${msg( - "If left empty, authentik will try to extract the launch URL based on the selected provider." - )} -

- - - -

- ${msg( - "If checked, the launch URL will open in a new browser tab or window from the user's application library." - )} -

- - ${ - rootInterface()?.config?.capabilities.includes( - CapabilitiesEnum.CanSaveMedia - ) - ? html` - - ${this.instance?.metaIcon - ? html` -

- ${msg("Currently set to:")} - ${this.instance?.metaIcon} -

- ` - : html``} - - ${this.instance?.metaIcon - ? html` - - -

- ${msg("Delete currently set icon.")} -

- - ` - : html``}` - : html` - -

${iconHelperText}

- ` - } - - - - - - + + + + ${rootInterface()?.config?.capabilities.includes(CapabilitiesEnum.CanSaveMedia) + ? html` + ${this.instance?.metaIcon + ? html` + + ` + : html``}` + : html` + `} + +
`; diff --git a/web/src/admin/applications/renderers.ts b/web/src/admin/applications/renderers.ts deleted file mode 100644 index 93452f6db..000000000 --- a/web/src/admin/applications/renderers.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { msg } from "@lit/localize"; -import { html } from "lit"; -import { ifDefined } from "lit/directives/if-defined.js"; - -type AkTextInput = { - // The name of the field, snake-to-camel'd if necessary. - name: string; - // The label of the field. - label: string; - value?: any; - required: boolean; - // The help message, shown at the bottom. - help?: string; -}; - -const akTextDefaults = { - required: false, -}; - -export function akText(args: AkTextInput) { - const { name, label, value, required, help } = { - ...akTextDefaults, - ...args - } - - return html` - -

${help}

- `; -} diff --git a/web/src/admin/applications/renderers/ak-backchannel-input.ts b/web/src/admin/applications/renderers/ak-backchannel-input.ts new file mode 100644 index 000000000..d66773b52 --- /dev/null +++ b/web/src/admin/applications/renderers/ak-backchannel-input.ts @@ -0,0 +1,108 @@ +import "@goauthentik/admin/applications/ProviderSelectModal"; +import { AKElement } from "@goauthentik/elements/Base"; + +import { html, nothing } from "lit"; +import { customElement, property } from "lit/decorators.js"; +import { ifDefined } from "lit/directives/if-defined.js"; +import { map } from "lit/directives/map.js"; + +import { Provider } from "@goauthentik/api"; + +type AkBackchannelProvidersArgs = { + // The name of the field, snake-to-camel'd if necessary. + name: string; + // The label of the field. + label: string; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + value?: any; + // The help message, shown at the bottom. + help?: string; + + providers: Provider[]; + confirm: ({ items }: { items: Provider[] }) => Promise; + remove: (provider: Provider) => () => void; +}; + +const akBackchannelProvidersDefaults = { + required: false, +}; + +export function akBackchannelProvidersInput(args: AkBackchannelProvidersArgs) { + const { name, label, help, providers, confirm, remove } = { + ...akBackchannelProvidersDefaults, + ...args, + }; + + const renderOneChip = (provider: Provider) => + html`${provider.name}`; + + return html` + +
+ + + +
+ ${map(providers, renderOneChip)} +
+
+ ${help ? html`

${help}

` : nothing} + + `; +} + +@customElement("ak-backchannel-providers-input") +export class AkBackchannelProvidersInput extends AKElement { + // Render into the lightDOM. This effectively erases the shadowDOM nature of this component, but + // we're not actually using that and, for the meantime, we need the form handlers to be able to + // find the children of this component. + // + // This field is so highly specialized that it would make more sense if we put the API and the + // fetcher here. + // + // TODO: This abstraction is wrong; it's putting *more* layers in as a way of managing the + // visual clutter and legibility issues of ak-form-elemental-horizontal and patternfly in + // general. + protected createRenderRoot() { + return this; + } + + @property({ type: String }) + name!: string; + + @property({ type: String }) + label = ""; + + @property({ type: Array }) + providers: Provider[] = []; + + @property({ attribute: false, type: Object }) + confirm!: ({ items }: { items: Provider[] }) => Promise; + + @property({ attribute: false, type: Object }) + remover!: (provider: Provider) => () => void; + + @property({ type: String }) + value = ""; + + @property({ type: Boolean }) + required = false; + + @property({ type: String }) + help = ""; + + render() { + return akBackchannelProvidersInput({ + name: this.name, + label: this.label, + help: this.help.trim() !== "" ? this.help : undefined, + providers: this.providers, + confirm: this.confirm, + remove: this.remover, + }); + } +} diff --git a/web/src/admin/applications/renderers/ak-file-input.ts b/web/src/admin/applications/renderers/ak-file-input.ts new file mode 100644 index 000000000..957818bd3 --- /dev/null +++ b/web/src/admin/applications/renderers/ak-file-input.ts @@ -0,0 +1,85 @@ +import { AKElement } from "@goauthentik/elements/Base"; + +import { msg } from "@lit/localize"; +import { html, nothing } from "lit"; +import { customElement, property } from "lit/decorators.js"; + +type AkFileArgs = { + // The name of the field, snake-to-camel'd if necessary. + name: string; + // The label of the field. + label: string; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + value?: any; + required: boolean; + // The message to show next to the "current icon". + current: string; + // The help message, shown at the bottom. + help?: string; +}; + +const akFileDefaults = { + name: "", + required: false, + current: msg("Currently set to:"), +}; + +export function akFile(args: AkFileArgs) { + const { name, label, required, value, help, current } = { + ...akFileDefaults, + ...args, + }; + + const currentMsg = + value && current + ? html`

${current} ${value}

` + : nothing; + + return html` + + ${currentMsg} ${help ? html`

${help}

` : nothing} + `; +} + +@customElement("ak-file-input") +export class AkFileInput extends AKElement { + // Render into the lightDOM. This effectively erases the shadowDOM nature of this component, but + // we're not actually using that and, for the meantime, we need the form handlers to be able to + // find the children of this component. + // + // TODO: This abstraction is wrong; it's putting *more* layers in as a way of managing the + // visual clutter and legibility issues of ak-form-elemental-horizontal and patternfly in + // general. + protected createRenderRoot() { + return this; + } + + @property({ type: String }) + name!: string; + + @property({ type: String }) + label = ""; + + @property({ type: String }) + current = msg("Currently set to:"); + + @property({ type: String }) + value = ""; + + @property({ type: Boolean }) + required = false; + + @property({ type: String }) + help = ""; + + render() { + return akFile({ + name: this.name, + label: this.label, + value: this.value, + current: this.current, + required: this.required, + help: this.help.trim() !== "" ? this.help : undefined, + }); + } +} diff --git a/web/src/admin/applications/renderers/ak-provider-search-input.ts b/web/src/admin/applications/renderers/ak-provider-search-input.ts new file mode 100644 index 000000000..552cb0764 --- /dev/null +++ b/web/src/admin/applications/renderers/ak-provider-search-input.ts @@ -0,0 +1,80 @@ +import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; +import { groupBy } from "@goauthentik/common/utils"; +import { AKElement } from "@goauthentik/elements/Base"; +import "@goauthentik/elements/forms/SearchSelect"; + +import { html, nothing } from "lit"; +import { customElement, property } from "lit/decorators.js"; + +import { Provider, ProvidersAllListRequest, ProvidersApi } from "@goauthentik/api"; + +const renderElement = (item: Provider) => item.name; +const renderValue = (item: Provider | undefined) => item?.pk; +const doGroupBy = (items: Provider[]) => groupBy(items, (item) => item.verboseName); + +async function fetch(query?: string) { + const args: ProvidersAllListRequest = { + ordering: "name", + }; + if (query !== undefined) { + args.search = query; + } + const items = await new ProvidersApi(DEFAULT_CONFIG).providersAllList(args); + return items.results; +} + +@customElement("ak-provider-search-input") +export class AkProviderInput extends AKElement { + // Render into the lightDOM. This effectively erases the shadowDOM nature of this component, but + // we're not actually using that and, for the meantime, we need the form handlers to be able to + // find the children of this component. + // + // TODO: This abstraction is wrong; it's putting *more* layers in as a way of managing the + // visual clutter and legibility issues of ak-form-elemental-horizontal and patternfly in + // general. + protected createRenderRoot() { + return this; + } + + @property({ type: String }) + name!: string; + + @property({ type: String }) + label = ""; + + @property({ type: Number }) + value?: number; + + @property({ type: Boolean }) + required = false; + + @property({ type: Boolean }) + blankable = false; + + @property({ type: String }) + help = ""; + + constructor() { + super(); + this.selected = this.selected.bind(this); + } + + selected(item: Provider) { + return this.value !== undefined && this.value === item.pk; + } + + render() { + return html` + + + ${this.help ? html`

${this.help}

` : nothing} + `; + } +} diff --git a/web/src/admin/applications/renderers/ak-radio-input.ts b/web/src/admin/applications/renderers/ak-radio-input.ts new file mode 100644 index 000000000..57cfa22c6 --- /dev/null +++ b/web/src/admin/applications/renderers/ak-radio-input.ts @@ -0,0 +1,77 @@ +import { AKElement } from "@goauthentik/elements/Base"; +import { RadioOption } from "@goauthentik/elements/forms/Radio"; + +import { html, nothing } from "lit"; +import { customElement, property } from "lit/decorators.js"; + +type AkRadioArgs = { + // The name of the field, snake-to-camel'd if necessary. + name: string; + // The label of the field. + label: string; + value?: T; + required?: boolean; + options: RadioOption[]; + // The help message, shown at the bottom. + help?: string; +}; + +const akRadioDefaults = { + required: false, + options: [], +}; + +export function akRadioInput(args: AkRadioArgs) { + const { name, label, help, required, options, value } = { + ...akRadioDefaults, + ...args, + }; + + return html` + + ${help ? html`

${help}

` : nothing} + `; +} + +@customElement("ak-radio-input") +export class AkRadioInput extends AKElement { + // Render into the lightDOM. This effectively erases the shadowDOM nature of this component, but + // we're not actually using that and, for the meantime, we need the form handlers to be able to + // find the children of this component. + // + // TODO: This abstraction is wrong; it's putting *more* layers in as a way of managing the + // visual clutter and legibility issues of ak-form-elemental-horizontal and patternfly in + // general. + protected createRenderRoot() { + return this; + } + + @property({ type: String }) + name!: string; + + @property({ type: String }) + label = ""; + + @property({ type: String }) + help = ""; + + @property({ type: Boolean }) + required = false; + + @property({ type: Object }) + value!: T; + + @property({ type: Array }) + options: RadioOption[] = []; + + render() { + return akRadioInput({ + name: this.name, + label: this.label, + value: this.value, + options: this.options, + required: this.required, + help: this.help.trim() !== "" ? this.help : undefined, + }); + } +} diff --git a/web/src/admin/applications/renderers/ak-switch-input.ts b/web/src/admin/applications/renderers/ak-switch-input.ts new file mode 100644 index 000000000..6fe2fe850 --- /dev/null +++ b/web/src/admin/applications/renderers/ak-switch-input.ts @@ -0,0 +1,84 @@ +import { AKElement } from "@goauthentik/elements/Base"; + +import { html, nothing } from "lit"; +import { customElement, property, query } from "lit/decorators.js"; + +type AkSwitchArgs = { + // The name of the field, snake-to-camel'd if necessary. + name: string; + // The label of the field. + label: string; + checked: boolean; + required: boolean; + // The help message, shown at the bottom. + help?: string; +}; + +const akSwitchDefaults = { + checked: false, + required: false, +}; + +export function akSwitch(args: AkSwitchArgs) { + const { name, label, checked, required, help } = { + ...akSwitchDefaults, + ...args, + }; + + const doCheck = checked ? checked : undefined; + + return html` + + ${help ? html`

${help}

` : nothing} + `; +} + +@customElement("ak-switch-input") +export class AkSwitchInput extends AKElement { + // Render into the lightDOM. This effectively erases the shadowDOM nature of this component, but + // we're not actually using that and, for the meantime, we need the form handlers to be able to + // find the children of this component. + // + // TODO: This abstraction is wrong; it's putting *more* layers in as a way of managing the + // visual clutter and legibility issues of ak-form-elemental-horizontal and patternfly in + // general. + protected createRenderRoot() { + return this; + } + + @property({ type: String }) + name!: string; + + @property({ type: String }) + label = ""; + + @property({ type: Boolean }) + checked: boolean = false; + + @property({ type: Boolean }) + required = false; + + @property({ type: String }) + help = ""; + + @query("input.pf-c-switch__input[type=checkbox]") + checkbox!: HTMLInputElement; + + render() { + return akSwitch({ + name: this.name, + label: this.label, + checked: this.checked, + required: this.required, + help: this.help.trim() !== "" ? this.help : undefined, + }); + } +} diff --git a/web/src/admin/applications/renderers/ak-text-input.ts b/web/src/admin/applications/renderers/ak-text-input.ts new file mode 100644 index 000000000..50d85fe39 --- /dev/null +++ b/web/src/admin/applications/renderers/ak-text-input.ts @@ -0,0 +1,76 @@ +import { AKElement } from "@goauthentik/elements/Base"; + +import { html, nothing } from "lit"; +import { customElement, property } from "lit/decorators.js"; +import { ifDefined } from "lit/directives/if-defined.js"; + +type AkTextArgs = { + // The name of the field, snake-to-camel'd if necessary. + name: string; + // The label of the field. + label: string; + value?: string; + required: boolean; + // The help message, shown at the bottom. + help?: string; +}; + +const akTextDefaults = { + required: false, +}; + +export function akText(args: AkTextArgs) { + const { name, label, value, required, help } = { + ...akTextDefaults, + ...args, + }; + + return html` + + ${help ? html`

${help}

` : nothing} + `; +} + +@customElement("ak-text-input") +export class AkTextInput extends AKElement { + // Render into the lightDOM. This effectively erases the shadowDOM nature of this component, but + // we're not actually using that and, for the meantime, we need the form handlers to be able to + // find the children of this component. + // + // TODO: This abstraction is wrong; it's putting *more* layers in as a way of managing the + // visual clutter and legibility issues of ak-form-elemental-horizontal and patternfly in + // general. + protected createRenderRoot() { + return this; + } + + @property({ type: String }) + name!: string; + + @property({ type: String }) + label = ""; + + @property({ type: String }) + value = ""; + + @property({ type: Boolean }) + required = false; + + @property({ type: String }) + help = ""; + + render() { + return akText({ + name: this.name, + label: this.label, + value: this.value, + required: this.required, + help: this.help.trim() !== "" ? this.help : undefined, + }); + } +} diff --git a/web/src/admin/applications/renderers/ak-textarea-input.ts b/web/src/admin/applications/renderers/ak-textarea-input.ts new file mode 100644 index 000000000..3c8e51942 --- /dev/null +++ b/web/src/admin/applications/renderers/ak-textarea-input.ts @@ -0,0 +1,75 @@ +import { AKElement } from "@goauthentik/elements/Base"; + +import { html, nothing } from "lit"; +import { customElement, property } from "lit/decorators.js"; + +type AkTextareaArgs = { + // The name of the field, snake-to-camel'd if necessary. + name: string; + // The label of the field. + label: string; + value?: string; + required: boolean; + // The help message, shown at the bottom. + help?: string; +}; + +const akTextareaDefaults = { + required: false, +}; + +export function akTextarea(args: AkTextareaArgs) { + const { name, label, value, required, help } = { + ...akTextareaDefaults, + ...args, + }; + + // ` + ${help ? html`

${help}

` : nothing} + `; +} + +@customElement("ak-textarea-input") +export class AkTextareaInput extends AKElement { + // Render into the lightDOM. This effectively erases the shadowDOM nature of this component, but + // we're not actually using that and, for the meantime, we need the form handlers to be able to + // find the children of this component. + // + // TODO: This abstraction is wrong; it's putting *more* layers in as a way of managing the + // visual clutter and legibility issues of ak-form-elemental-horizontal and patternfly in + // general. + protected createRenderRoot() { + return this; + } + + @property({ type: String }) + name!: string; + + @property({ type: String }) + label = ""; + + @property({ type: String }) + value = ""; + + @property({ type: Boolean }) + required = false; + + @property({ type: String }) + help = ""; + + render() { + return akTextarea({ + name: this.name, + label: this.label, + value: this.value, + required: this.required, + help: this.help.trim() !== "" ? this.help : undefined, + }); + } +} diff --git a/web/src/elements/forms/Form.ts b/web/src/elements/forms/Form.ts index d148baaef..e2fbacc74 100644 --- a/web/src/elements/forms/Form.ts +++ b/web/src/elements/forms/Form.ts @@ -68,11 +68,9 @@ export interface KeyUnknown { * Consider refactoring serializeForm() so that the conversions are on * the input types, rather than here. (i.e. "Polymorphism is better than * switch.") - * - * + * + * */ - - @customElement("ak-form") export abstract class Form extends AKElement { diff --git a/web/src/elements/forms/HorizontalFormElement.ts b/web/src/elements/forms/HorizontalFormElement.ts index 4ee1b2bf7..9d00327da 100644 --- a/web/src/elements/forms/HorizontalFormElement.ts +++ b/web/src/elements/forms/HorizontalFormElement.ts @@ -16,7 +16,7 @@ import PFBase from "@patternfly/patternfly/patternfly-base.css"; * Horizontal Form Element Container. * * This element provides the interface between elements of our forms and the - * form itself. + * form itself. * @custom-element ak-form-element-horizontal */ @@ -79,7 +79,7 @@ export class HorizontalFormElement extends AKElement { /* If this property changes, we want to make sure the parent control is "opened" so * that users can see the change.[1] - */ + */ @property({ type: Boolean }) set invalid(v: boolean) { this._invalid = v;