web: fix value handling inside controlled components (#9648) * web: fix esbuild issue with style sheets Getting ESBuild, Lit, and Storybook to all agree on how to read and parse stylesheets is a serious pain. This fix better identifies the value types (instances) being passed from various sources in the repo to the three *different* kinds of style processors we're using (the native one, the polyfill one, and whatever the heck Storybook does internally). Falling back to using older CSS instantiating techniques one era at a time seems to do the trick. It's ugly, but in the face of the aggressive styling we use to avoid Flashes of Unstyled Content (FLoUC), it's the logic with which we're left. In standard mode, the following warning appears on the console when running a Flow: ``` Autofocus processing was blocked because a document already has a focused element. ``` In compatibility mode, the following **error** appears on the console when running a Flow: ``` crawler-inject.js:1106 Uncaught TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'. at initDomMutationObservers (crawler-inject.js:1106:18) at crawler-inject.js:1114:24 at Array.forEach (<anonymous>) at initDomMutationObservers (crawler-inject.js:1114:10) at crawler-inject.js:1549:1 initDomMutationObservers @ crawler-inject.js:1106 (anonymous) @ crawler-inject.js:1114 initDomMutationObservers @ crawler-inject.js:1114 (anonymous) @ crawler-inject.js:1549 ``` Despite this error, nothing seems to be broken and flows work as anticipated. * web: fix value handling inside controlled components This is one of those stupid bugs that drive web developers crazy. The basics are straightforward: when you cause a higher-level component to have a "big enough re-render," for some unknown definition of "big enough," it will re-render the sub-components. In traditional web interaction, those components should never be re-rendered while the user is interacting with the form, but in frameworks where there's dynamic re-arrangement, part or all of the form could get re-rendered at any mmoment. Since neither the form nor any of its intermediaries is tracking the values as they're changed, it's up to the components themselves to keep the user's input-- and to be hardened against property changes coming from the outside world. So static memoization of the initial value passed in, and aggressively walling off the values the customer generates from that field, are needed to protect the user's work from any framework's dynamic DOM management. I remember struggling with this in React; I had hoped Lit was better, but in this case, not better enough. The protocol for "is it an ak-data-control" is "it has a `json()` method that returns the data ready to be sent to the authentik server." I missed that in one place, so that's on me. * Eslint had opinions. * Added comments to explain something. Co-authored-by: Ken Sternberg <133134217+kensternberg-authentik@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
9240fa1037
commit
ade1f08c89
@ -2,8 +2,9 @@ import { AKElement } from "@goauthentik/elements/Base";
|
|||||||
import { CustomEmitterElement } from "@goauthentik/elements/utils/eventEmitter";
|
import { CustomEmitterElement } from "@goauthentik/elements/utils/eventEmitter";
|
||||||
|
|
||||||
import { msg } from "@lit/localize";
|
import { msg } from "@lit/localize";
|
||||||
|
import { PropertyValues } from "@lit/reactive-element/reactive-element";
|
||||||
import { TemplateResult, css, html } from "lit";
|
import { TemplateResult, css, html } from "lit";
|
||||||
import { customElement, property, queryAll } from "lit/decorators.js";
|
import { customElement, property, queryAll, state } from "lit/decorators.js";
|
||||||
import { map } from "lit/directives/map.js";
|
import { map } from "lit/directives/map.js";
|
||||||
|
|
||||||
import PFCheck from "@patternfly/patternfly/components/Check/check.css";
|
import PFCheck from "@patternfly/patternfly/components/Check/check.css";
|
||||||
@ -112,10 +113,14 @@ export class CheckboxGroup extends AkElementWithCustomEvents {
|
|||||||
@queryAll('input[type="checkbox"]')
|
@queryAll('input[type="checkbox"]')
|
||||||
checkboxes!: NodeListOf<HTMLInputElement>;
|
checkboxes!: NodeListOf<HTMLInputElement>;
|
||||||
|
|
||||||
internals?: ElementInternals;
|
@state()
|
||||||
|
values: string[] = [];
|
||||||
|
|
||||||
get json() {
|
internals?: ElementInternals;
|
||||||
return this.value;
|
doneFirstUpdate = false;
|
||||||
|
|
||||||
|
json() {
|
||||||
|
return this.values;
|
||||||
}
|
}
|
||||||
|
|
||||||
private get formValue() {
|
private get formValue() {
|
||||||
@ -124,7 +129,7 @@ export class CheckboxGroup extends AkElementWithCustomEvents {
|
|||||||
}
|
}
|
||||||
const name = this.name;
|
const name = this.name;
|
||||||
const entries = new FormData();
|
const entries = new FormData();
|
||||||
this.value.forEach((v) => entries.append(name, v));
|
this.values.forEach((v) => entries.append(name, v));
|
||||||
return entries;
|
return entries;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -136,14 +141,14 @@ export class CheckboxGroup extends AkElementWithCustomEvents {
|
|||||||
|
|
||||||
onClick(ev: Event) {
|
onClick(ev: Event) {
|
||||||
ev.stopPropagation();
|
ev.stopPropagation();
|
||||||
this.value = Array.from(this.checkboxes)
|
this.values = Array.from(this.checkboxes)
|
||||||
.filter((checkbox) => checkbox.checked)
|
.filter((checkbox) => checkbox.checked)
|
||||||
.map((checkbox) => checkbox.name);
|
.map((checkbox) => checkbox.name);
|
||||||
this.dispatchCustomEvent("change", this.value);
|
this.dispatchCustomEvent("change", this.values);
|
||||||
this.dispatchCustomEvent("input", this.value);
|
this.dispatchCustomEvent("input", this.values);
|
||||||
if (this.internals) {
|
if (this.internals) {
|
||||||
this.internals.setValidity({});
|
this.internals.setValidity({});
|
||||||
if (this.required && this.value.length === 0) {
|
if (this.required && this.values.length === 0) {
|
||||||
this.internals.setValidity(
|
this.internals.setValidity(
|
||||||
{
|
{
|
||||||
valueMissing: true,
|
valueMissing: true,
|
||||||
@ -154,6 +159,16 @@ export class CheckboxGroup extends AkElementWithCustomEvents {
|
|||||||
}
|
}
|
||||||
this.internals.setFormValue(this.formValue);
|
this.internals.setFormValue(this.formValue);
|
||||||
}
|
}
|
||||||
|
// Doing a write-back so anyone examining the checkbox.value field will get something
|
||||||
|
// meaningful. Doesn't do anything for anyone, usually, but it's nice to have.
|
||||||
|
this.value = this.values;
|
||||||
|
}
|
||||||
|
|
||||||
|
willUpdate(changed: PropertyValues<this>) {
|
||||||
|
if (changed.has("value") && !this.doneFirstUpdate) {
|
||||||
|
this.doneFirstUpdate = true;
|
||||||
|
this.values = this.value;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
connectedCallback() {
|
connectedCallback() {
|
||||||
@ -183,7 +198,7 @@ export class CheckboxGroup extends AkElementWithCustomEvents {
|
|||||||
|
|
||||||
render() {
|
render() {
|
||||||
const renderOne = ([name, label]: CheckboxPr) => {
|
const renderOne = ([name, label]: CheckboxPr) => {
|
||||||
const selected = this.value.includes(name);
|
const selected = this.values.includes(name);
|
||||||
const blockFwd = (e: Event) => {
|
const blockFwd = (e: Event) => {
|
||||||
e.stopImmediatePropagation();
|
e.stopImmediatePropagation();
|
||||||
};
|
};
|
||||||
|
|||||||
@ -53,6 +53,9 @@ export class AkDualSelectProvider extends CustomListenerElement(AKElement) {
|
|||||||
|
|
||||||
private isLoading = false;
|
private isLoading = false;
|
||||||
|
|
||||||
|
private doneFirstUpdate = false;
|
||||||
|
private internalSelected: DualSelectPair[] = [];
|
||||||
|
|
||||||
private pagination?: Pagination;
|
private pagination?: Pagination;
|
||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
@ -69,6 +72,11 @@ export class AkDualSelectProvider extends CustomListenerElement(AKElement) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
willUpdate(changedProperties: PropertyValues<this>) {
|
willUpdate(changedProperties: PropertyValues<this>) {
|
||||||
|
if (changedProperties.has("selected") && !this.doneFirstUpdate) {
|
||||||
|
this.doneFirstUpdate = true;
|
||||||
|
this.internalSelected = this.selected;
|
||||||
|
}
|
||||||
|
|
||||||
if (changedProperties.has("searchDelay")) {
|
if (changedProperties.has("searchDelay")) {
|
||||||
this.doSearch = debounce(
|
this.doSearch = debounce(
|
||||||
AkDualSelectProvider.prototype.doSearch.bind(this),
|
AkDualSelectProvider.prototype.doSearch.bind(this),
|
||||||
@ -105,7 +113,8 @@ export class AkDualSelectProvider extends CustomListenerElement(AKElement) {
|
|||||||
if (!(event instanceof CustomEvent)) {
|
if (!(event instanceof CustomEvent)) {
|
||||||
throw new Error(`Expecting a CustomEvent for change, received ${event} instead`);
|
throw new Error(`Expecting a CustomEvent for change, received ${event} instead`);
|
||||||
}
|
}
|
||||||
this.selected = event.detail.value;
|
this.internalSelected = event.detail.value;
|
||||||
|
this.selected = this.internalSelected;
|
||||||
}
|
}
|
||||||
|
|
||||||
onSearch(event: Event) {
|
onSearch(event: Event) {
|
||||||
@ -124,12 +133,16 @@ export class AkDualSelectProvider extends CustomListenerElement(AKElement) {
|
|||||||
return this.dualSelector.value!.selected.map(([k, _]) => k);
|
return this.dualSelector.value!.selected.map(([k, _]) => k);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
json() {
|
||||||
|
return this.value;
|
||||||
|
}
|
||||||
|
|
||||||
render() {
|
render() {
|
||||||
return html`<ak-dual-select
|
return html`<ak-dual-select
|
||||||
${ref(this.dualSelector)}
|
${ref(this.dualSelector)}
|
||||||
.options=${this.options}
|
.options=${this.options}
|
||||||
.pages=${this.pagination}
|
.pages=${this.pagination}
|
||||||
.selected=${this.selected}
|
.selected=${this.internalSelected}
|
||||||
available-label=${this.availableLabel}
|
available-label=${this.availableLabel}
|
||||||
selected-label=${this.selectedLabel}
|
selected-label=${this.selectedLabel}
|
||||||
></ak-dual-select>`;
|
></ak-dual-select>`;
|
||||||
|
|||||||
@ -80,7 +80,7 @@ export function serializeForm<T extends KeyUnknown>(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if ("akControl" in inputElement.dataset) {
|
if ("akControl" in inputElement.dataset) {
|
||||||
assignValue(element, inputElement.value, json);
|
assignValue(element, (inputElement as unknown as AkControlElement).json(), json);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user