From df16dc3088110fa0b5a8e93889d8c7d7280b7ebe Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Thu, 3 Aug 2023 14:43:31 -0700 Subject: [PATCH 1/4] web: Replace ad-hoc toggle control with ak-toggle-group This commit replaces various ad-hoc implementations of the Patternfly Toggle Group HTML with a web component that encapsulates all of the needed behavior and exposes a single API with a single event handler, return the value of the option clicked. The results are: Lots of visual clutter is eliminated. A single link of: ```
``` Now looks like: ``` ``` This also means that the three pages that used the Patternfly Toggle Group could eliminate all of their Patternfly PFToggleGroup needs, as well as the `justify-content: center` extension, which also eliminated the `css` import. The savings aren't as spectacular as I'd hoped: removed 178 lines, but added 123; total savings 55 lines of code. I still count this a win: we need never write another toggle component again, and any bugs, extensions or features we may want to add can be centralized or forked without risking the whole edifice. --- web/src/admin/blueprints/BlueprintForm.ts | 69 +++----------- web/src/admin/policies/PolicyBindingForm.ts | 68 +++----------- .../providers/proxy/ProxyProviderForm.ts | 81 +++++------------ web/src/components/ak-toggle-group.ts | 90 +++++++++++++++++++ web/tsconfig.json | 1 + 5 files changed, 136 insertions(+), 173 deletions(-) create mode 100644 web/src/components/ak-toggle-group.ts diff --git a/web/src/admin/blueprints/BlueprintForm.ts b/web/src/admin/blueprints/BlueprintForm.ts index 24a55ee8a..3176ddb0e 100644 --- a/web/src/admin/blueprints/BlueprintForm.ts +++ b/web/src/admin/blueprints/BlueprintForm.ts @@ -9,12 +9,11 @@ import "@goauthentik/elements/forms/SearchSelect"; import YAML from "yaml"; import { msg } from "@lit/localize"; -import { CSSResult, TemplateResult, css, html } from "lit"; +import { CSSResult, TemplateResult, html } from "lit"; import { customElement, state } from "lit/decorators.js"; import { ifDefined } from "lit/directives/if-defined.js"; import PFContent from "@patternfly/patternfly/components/Content/content.css"; -import PFToggleGroup from "@patternfly/patternfly/components/ToggleGroup/toggle-group.css"; import { BlueprintFile, BlueprintInstance, ManagedApi } from "@goauthentik/api"; @@ -51,15 +50,7 @@ export class BlueprintForm extends ModelForm { } static get styles(): CSSResult[] { - return super.styles.concat( - PFToggleGroup, - PFContent, - css` - .pf-c-toggle-group { - justify-content: center; - } - `, - ); + return [...super.styles, PFContent]; } async send(data: BlueprintInstance): Promise { @@ -105,52 +96,16 @@ export class BlueprintForm extends ModelForm {
-
-
- -
- -
- -
- -
- -
-
+ ) => { + this.source = ev.detail.value; + }} + > + + + +
${this.source === blueprintSource.file diff --git a/web/src/admin/policies/PolicyBindingForm.ts b/web/src/admin/policies/PolicyBindingForm.ts index af98d8781..098a57546 100644 --- a/web/src/admin/policies/PolicyBindingForm.ts +++ b/web/src/admin/policies/PolicyBindingForm.ts @@ -5,12 +5,11 @@ import { ModelForm } from "@goauthentik/elements/forms/ModelForm"; import "@goauthentik/elements/forms/SearchSelect"; import { msg } from "@lit/localize"; -import { CSSResult, css } from "lit"; +import { CSSResult } from "lit"; import { TemplateResult, html } from "lit"; import { customElement, property, state } from "lit/decorators.js"; import PFContent from "@patternfly/patternfly/components/Content/content.css"; -import PFToggleGroup from "@patternfly/patternfly/components/ToggleGroup/toggle-group.css"; import { CoreApi, @@ -70,15 +69,7 @@ export class PolicyBindingForm extends ModelForm { } static get styles(): CSSResult[] { - return super.styles.concat( - PFToggleGroup, - PFContent, - css` - .pf-c-toggle-group { - justify-content: center; - } - `, - ); + return [...super.styles, PFContent]; } send(data: PolicyBinding): Promise { @@ -112,55 +103,22 @@ export class PolicyBindingForm extends ModelForm { } renderModeSelector(): TemplateResult { - return html`
- -
- -
- -
- -
- -
`; + return html` ) => { + this.policyGroupUser = ev.detail.value; + }} + > + + + + `; } renderForm(): TemplateResult { return html`
-
-
${this.renderModeSelector()}
-
+
${this.renderModeSelector()}
{ static get styles(): CSSResult[] { - return super.styles.concat( - PFToggleGroup, - PFContent, - PFList, - PFSpacing, - css` - .pf-c-toggle-group { - justify-content: center; - } - `, - ); + return [...super.styles, PFContent, PFList, PFSpacing]; } async loadInstance(pk: number): Promise { @@ -137,51 +127,22 @@ export class ProxyProviderFormPage extends ModelForm { } renderModeSelector(): TemplateResult { - return html`
- -
- -
- -
- -
- -
`; + return html` + ) => { + this.mode = ev.detail.value; + }} + > + + + + + `; } renderSettings(): TemplateResult { @@ -362,9 +323,7 @@ export class ProxyProviderFormPage extends ModelForm {
-
-
${this.renderModeSelector()}
-
+
${this.renderModeSelector()}
${this.renderSettings()}
diff --git a/web/src/components/ak-toggle-group.ts b/web/src/components/ak-toggle-group.ts new file mode 100644 index 000000000..be3382891 --- /dev/null +++ b/web/src/components/ak-toggle-group.ts @@ -0,0 +1,90 @@ +import { AKElement } from "@goauthentik/elements/Base"; +import { CustomEmitterElement } from "@goauthentik/elements/utils/eventEmitter"; + +import { css, html, nothing } from "lit"; +import { customElement, property } from "lit/decorators.js"; +import { classMap } from "lit/directives/class-map.js"; + +import PFToggleGroup from "@patternfly/patternfly/components/ToggleGroup/toggle-group.css"; + +type Pair = [string, string]; + +/** + * Toggle Group + * + * An implementation of the Patternfly Toggle Group as a LitElement + * + * @element ak-toggle-group + * + * @fires ak-toggle - Fired when someone clicks on a toggle option. Carries the value of the option. + */ + +// MYNIS: +// A 'name' property so that the event carries *which* toggle group emitted the event. + +@customElement("ak-toggle-group") +export class AkToggleGroup extends CustomEmitterElement(AKElement) { + static get styles() { + return [ + PFToggleGroup, + css` + .pf-c-toggle-group { + justify-content: center; + } + `, + ]; + } + + /* + * The value (causes highlighting, value is returned) + * + * @attr + */ + @property({ type: String, reflect: true }) + value = ""; + + get rawOptions(): HTMLOptionElement[] { + return Array.from(this.querySelectorAll("option") ?? []); + } + + get options(): Pair[] { + return Array.from(this.rawOptions).map( + (option: HTMLOptionElement): Pair => [ + option.getAttribute("value") ?? "", + option.textContent ?? "", + ], + ); + } + + render() { + const last = this.options.length - 1; + const mkClass = (v: string) => ({ + "pf-c-toggle-group__button": true, + "pf-m-selected": this.value === v, + }); + + const mkClick = (v: string) => () => { + this.dispatchCustomEvent("ak-toggle", { value: v }); + }; + + return html`
+ ${this.options.map( + ([key, label], idx) => + html`
+ +
+ ${idx < last + ? html`` + : nothing} `, + )} +
`; + } +} + +export default AkToggleGroup; diff --git a/web/tsconfig.json b/web/tsconfig.json index cd85334f8..7e9327c55 100644 --- a/web/tsconfig.json +++ b/web/tsconfig.json @@ -5,6 +5,7 @@ "@goauthentik/app/*": ["src/*"], "@goauthentik/admin/*": ["src/admin/*"], "@goauthentik/common/*": ["src/common/*"], + "@goauthentik/components/*": ["src/components/*"], "@goauthentik/docs/*": ["../website/docs/*"], "@goauthentik/elements/*": ["src/elements/*"], "@goauthentik/flow/*": ["src/flow/*"], From 6018ffe13dd144207643f7445d3271f0704a32eb Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Thu, 3 Aug 2023 14:53:05 -0700 Subject: [PATCH 2/4] web: minor code formatting issue. --- .../providers/proxy/ProxyProviderForm.ts | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/web/src/admin/providers/proxy/ProxyProviderForm.ts b/web/src/admin/providers/proxy/ProxyProviderForm.ts index a626dfe95..d352c0336 100644 --- a/web/src/admin/providers/proxy/ProxyProviderForm.ts +++ b/web/src/admin/providers/proxy/ProxyProviderForm.ts @@ -127,20 +127,16 @@ export class ProxyProviderFormPage extends ModelForm { } renderModeSelector(): TemplateResult { + const setMode = (ev: CustomEvent<{ value: ProxyMode }>) => { + this.mode = ev.detail.value; + }; + + // prettier-ignore return html` - ) => { - this.mode = ev.detail.value; - }} - > + - - + + `; } From 46926d54ca3fcfd61497da77f2470e6dd2a735d8 Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Thu, 3 Aug 2023 15:11:55 -0700 Subject: [PATCH 3/4] web: adding a storybook for the ak-toggle-group component --- .../stories/ak-toggle-group.stories.ts | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 web/src/components/stories/ak-toggle-group.stories.ts diff --git a/web/src/components/stories/ak-toggle-group.stories.ts b/web/src/components/stories/ak-toggle-group.stories.ts new file mode 100644 index 000000000..59fd1049b --- /dev/null +++ b/web/src/components/stories/ak-toggle-group.stories.ts @@ -0,0 +1,67 @@ +import "@goauthentik/elements/messages/MessageContainer"; +import { Meta } from "@storybook/web-components"; + +import { TemplateResult, html } from "lit"; + +import "../ak-toggle-group"; +import AkToggleGroup from "../ak-toggle-group"; + +const metadata: Meta = { + title: "Components / Toggle Group", + component: "ak-toggle-group", + parameters: { + docs: { + description: { + component: "A stylized toggle control", + }, + }, + }, +}; + +export default metadata; + +const LIGHT = "pf-t-light"; +function injectTheme() { + setTimeout(() => { + if (!document.body.classList.contains(LIGHT)) { + document.body.classList.add(LIGHT); + } + }); +} + +const container = (testItem: TemplateResult) => { + injectTheme(); + return html`
+ + ${testItem} +
    +
    `; +} + +const testOptions = [ + ["funky", "Option One: The Funky One"], + ["invalid", "Option Two: The Invalid One"], + ["weird", "Option Three: The Weird One"], +]; + +export const ToggleGroup = () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const displayChange = (ev: any) => { + document.getElementById( + "toggle-message-pad" + )!.innerText = `Value selected: ${ev.detail.value}`; + }; + + return container( + html` + ${testOptions.map(([key, label]) => html``)} + ` + ); +}; From 134c6976fa1cf18a80f55b8e39c1407a2ddd082b Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Thu, 3 Aug 2023 15:39:01 -0700 Subject: [PATCH 4/4] Bugs found by CI/CD. --- web/src/components/stories/ak-toggle-group.stories.ts | 6 +++--- website/developer-docs/docs/templates/index.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/web/src/components/stories/ak-toggle-group.stories.ts b/web/src/components/stories/ak-toggle-group.stories.ts index 59fd1049b..31b2e6c7f 100644 --- a/web/src/components/stories/ak-toggle-group.stories.ts +++ b/web/src/components/stories/ak-toggle-group.stories.ts @@ -43,7 +43,7 @@ const container = (testItem: TemplateResult) => { ${testItem}
      `; -} +}; const testOptions = [ ["funky", "Option One: The Funky One"], @@ -55,13 +55,13 @@ export const ToggleGroup = () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const displayChange = (ev: any) => { document.getElementById( - "toggle-message-pad" + "toggle-message-pad", )!.innerText = `Value selected: ${ev.detail.value}`; }; return container( html` ${testOptions.map(([key, label]) => html``)} - ` + `, ); }; diff --git a/website/developer-docs/docs/templates/index.md b/website/developer-docs/docs/templates/index.md index 7ae832890..9645e706d 100644 --- a/website/developer-docs/docs/templates/index.md +++ b/website/developer-docs/docs/templates/index.md @@ -8,7 +8,7 @@ The most common types are: - [**Procedural**](./procedural.md): these are How To docs, the HOW information, with step-by-step instructions for accomplishing a task. This is what most people are looking for when they open the docs... and best practice is to separate the procedural docs from long, lengthy conceptual or reference docs. -- **Conceptual**: these docs provide the WHY information, and explain when to use a feature (or when not to!), and general concepts behind the fature or functioanlity. +- **Conceptual**: these docs provide the WHY information, and explain when to use a feature (or when not to!), and general concepts behind the feature or functioanlity. - **Reference**: this is typically tables or lists of reference information, such as configuration values, or most commmonly APIs.