From 8fa2bf4713370e145d77e639c6daaacc2ef0854e Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Thu, 4 Jan 2024 13:21:13 -0800 Subject: [PATCH] web: code quality pass, extra comments, pre-commit check. --- .../ak-dual-select/ak-dual-select-provider.ts | 64 +++++++++---------- .../elements/ak-dual-select/ak-dual-select.ts | 28 ++++---- .../ak-dual-select-available-pane.ts | 8 +-- .../ak-dual-select-selected-pane.ts | 18 +++--- .../components/ak-search-bar.ts | 10 ++- web/src/elements/ak-dual-select/types.ts | 2 +- 6 files changed, 68 insertions(+), 62 deletions(-) diff --git a/web/src/elements/ak-dual-select/ak-dual-select-provider.ts b/web/src/elements/ak-dual-select/ak-dual-select-provider.ts index d46883538..90e9cd3e4 100644 --- a/web/src/elements/ak-dual-select/ak-dual-select-provider.ts +++ b/web/src/elements/ak-dual-select/ak-dual-select-provider.ts @@ -1,4 +1,5 @@ import { AKElement } from "@goauthentik/elements/Base"; +import { debounce } from "@goauthentik/elements/utils/debounce"; import { CustomListenerElement } from "@goauthentik/elements/utils/eventEmitter"; import { msg } from "@lit/localize"; @@ -6,7 +7,6 @@ import { PropertyValues, html } from "lit"; import { customElement, property, state } from "lit/decorators.js"; import { createRef, ref } from "lit/directives/ref.js"; import type { Ref } from "lit/directives/ref.js"; -import { debounce } from "@goauthentik/elements/utils/debounce"; import type { Pagination } from "@goauthentik/api"; @@ -45,7 +45,7 @@ export class AkDualSelectProvider extends CustomListenerElement(AKElement) { /** The remote lists are debounced by definition. This is the interval for the debounce. */ @property({ attribute: "search-delay", type: Number }) searchDelay = 250; - + @state() private options: DualSelectPair[] = []; @@ -55,10 +55,6 @@ export class AkDualSelectProvider extends CustomListenerElement(AKElement) { private pagination?: Pagination; - get value() { - return this.dualSelector.value!.selected.map(([k, _]) => k); - } - selectedMap: WeakMap = new WeakMap(); constructor() { @@ -74,32 +70,6 @@ export class AkDualSelectProvider extends CustomListenerElement(AKElement) { this.addCustomListener("ak-dual-select-search", this.onSearch); } - onNav(event: Event) { - if (!(event instanceof CustomEvent)) { - throw new Error(`Expecting a CustomEvent for navigation, received ${event} instead`); - } - this.fetch(event.detail); - } - - onChange(event: Event) { - if (!(event instanceof CustomEvent)) { - throw new Error(`Expecting a CustomEvent for change, received ${event} instead`); - } - this.selected = event.detail.value; - } - - doSearch(search: string) { - this.pagination = undefined; - this.fetch(undefined, search); - } - - onSearch(event: Event) { - if (!(event instanceof CustomEvent)) { - throw new Error(`Expecting a CustomEvent for change, received ${event} instead`); - } - this.doSearch(event.detail); - } - willUpdate(changedProperties: PropertyValues) { if (changedProperties.has("searchDelay")) { this.doSearch = debounce(this.doSearch.bind(this), this.searchDelay); @@ -127,6 +97,36 @@ export class AkDualSelectProvider extends CustomListenerElement(AKElement) { this.isLoading = false; } + onNav(event: Event) { + if (!(event instanceof CustomEvent)) { + throw new Error(`Expecting a CustomEvent for navigation, received ${event} instead`); + } + this.fetch(event.detail); + } + + onChange(event: Event) { + if (!(event instanceof CustomEvent)) { + throw new Error(`Expecting a CustomEvent for change, received ${event} instead`); + } + this.selected = event.detail.value; + } + + onSearch(event: Event) { + if (!(event instanceof CustomEvent)) { + throw new Error(`Expecting a CustomEvent for change, received ${event} instead`); + } + this.doSearch(event.detail); + } + + doSearch(search: string) { + this.pagination = undefined; + this.fetch(undefined, search); + } + + get value() { + return this.dualSelector.value!.selected.map(([k, _]) => k); + } + render() { return html`) { if (changedProperties.has("selected")) { this.selectedKeys = new Set(this.selected.map(([key, _]) => key)); @@ -246,6 +242,10 @@ export class AkDualSelect extends CustomEmitterElement(CustomListenerElement(AKE this.selectedPane.value!.clearMove(); } + get value() { + return this.selected; + } + get canAddAll() { // False unless any visible option cannot be found in the selected list, so can still be // added. @@ -268,15 +268,17 @@ export class AkDualSelect extends CustomEmitterElement(CustomListenerElement(AKE } render() { - const selected = this.selectedFilter === "" ? this.selected : - this.selected.filter(([_k, v, s]) => { - const value = s !== undefined ? s : v; - if (typeof value !== "string") { - throw new Error("Filter only works when there's a string comparator"); - } - return value.toLowerCase().includes(this.selectedFilter.toLowerCase()) - }); - + const selected = + this.selectedFilter === "" + ? this.selected + : this.selected.filter(([_k, v, s]) => { + const value = s !== undefined ? s : v; + if (typeof value !== "string") { + throw new Error("Filter only works when there's a string comparator"); + } + return value.toLowerCase().includes(this.selectedFilter.toLowerCase()); + }); + const availableCount = this.availablePane.value?.toMove.size ?? 0; const selectedCount = this.selectedPane.value?.toMove.size ?? 0; const selectedTotal = selected.length; diff --git a/web/src/elements/ak-dual-select/components/ak-dual-select-available-pane.ts b/web/src/elements/ak-dual-select/components/ak-dual-select-available-pane.ts index 06f724ea5..b8f58aab9 100644 --- a/web/src/elements/ak-dual-select/components/ak-dual-select-available-pane.ts +++ b/web/src/elements/ak-dual-select/components/ak-dual-select-available-pane.ts @@ -80,10 +80,6 @@ export class AkDualSelectAvailablePane extends CustomEmitterElement(AKElement) { }); } - get moveable() { - return Array.from(this.toMove.values()); - } - clearMove() { this.toMove = new Set(); } @@ -112,6 +108,10 @@ export class AkDualSelectAvailablePane extends CustomEmitterElement(AKElement) { this.requestUpdate(); } + get moveable() { + return Array.from(this.toMove.values()); + } + // DO NOT use `Array.map()` instead of Lit's `map()` function. Lit's `map()` is object-aware and // will not re-arrange or reconstruct the list automatically if the actual sources do not // change; this allows the available pane to illustrate selected items with the checkmark diff --git a/web/src/elements/ak-dual-select/components/ak-dual-select-selected-pane.ts b/web/src/elements/ak-dual-select/components/ak-dual-select-selected-pane.ts index 6325f1533..bd8a1d58c 100644 --- a/web/src/elements/ak-dual-select/components/ak-dual-select-selected-pane.ts +++ b/web/src/elements/ak-dual-select/components/ak-dual-select-selected-pane.ts @@ -63,8 +63,13 @@ export class AkDualSelectSelectedPane extends CustomEmitterElement(AKElement) { this.onMove = this.onMove.bind(this); } - get moveable() { - return Array.from(this.toMove.values()); + connectedCallback() { + super.connectedCallback(); + hostAttributes.forEach(([attr, value]) => { + if (!this.hasAttribute(attr)) { + this.setAttribute(attr, value); + } + }); } clearMove() { @@ -92,13 +97,8 @@ export class AkDualSelectSelectedPane extends CustomEmitterElement(AKElement) { this.requestUpdate(); } - connectedCallback() { - super.connectedCallback(); - hostAttributes.forEach(([attr, value]) => { - if (!this.hasAttribute(attr)) { - this.setAttribute(attr, value); - } - }); + get moveable() { + return Array.from(this.toMove.values()); } render() { diff --git a/web/src/elements/ak-dual-select/components/ak-search-bar.ts b/web/src/elements/ak-dual-select/components/ak-search-bar.ts index 247f34b84..dafc14994 100644 --- a/web/src/elements/ak-dual-select/components/ak-search-bar.ts +++ b/web/src/elements/ak-dual-select/components/ak-search-bar.ts @@ -19,14 +19,18 @@ export class AkSearchbar extends CustomEmitterElement(AKElement) { return styles; } - input: Ref = createRef(); - @property({ type: String, reflect: true }) value = ""; + /** + * If you're using more than one search, this token can help listeners distinguishing between + * those searches. Lit's own helpers sometimes erase the source and current targets. + */ @property({ type: String }) name = ""; + input: Ref = createRef(); + constructor() { super(); this.onChange = this.onChange.bind(this); @@ -38,7 +42,7 @@ export class AkSearchbar extends CustomEmitterElement(AKElement) { } this.dispatchCustomEvent("ak-search", { source: this.name, - value: this.value + value: this.value, }); } diff --git a/web/src/elements/ak-dual-select/types.ts b/web/src/elements/ak-dual-select/types.ts index 92afe1bf3..2e7cea13a 100644 --- a/web/src/elements/ak-dual-select/types.ts +++ b/web/src/elements/ak-dual-select/types.ts @@ -22,5 +22,5 @@ export interface SearchbarEvent extends CustomEvent { detail: { source: string; value: string; - } + }; }