3f02534eb1
* web: weightloss program, part 1: FlowSearch This commit extracts the multiple uses of SearchSelect for Flow lookups in the `providers` collection and replaces them with a slightly more legible format, from: ```HTML <ak-search-select .fetchObjects=${async (query?: string): Promise<Flow[]> => { const args: FlowsInstancesListRequest = { ordering: "slug", designation: FlowsInstancesListDesignationEnum.Authentication, }; if (query !== undefined) { args.search = query; } const flows = await new FlowsApi(DEFAULT_CONFIG).flowsInstancesList(args); return flows.results; }} .renderElement=${(flow: Flow): string => { return RenderFlowOption(flow); }} .renderDescription=${(flow: Flow): TemplateResult => { return html`${flow.name}`; }} .value=${(flow: Flow | undefined): string | undefined => { return flow?.pk; }} .selected=${(flow: Flow): boolean => { return flow.pk === this.instance?.authenticationFlow; }} > </ak-search-select> ``` ... to: ```HTML <ak-flow-search flowType=${FlowsInstancesListDesignationEnum.Authentication} .currentFlow=${this.instance?.authenticationFlow} required ></ak-flow-search> ``` All of those middle methods, like `renderElement`, `renderDescription`, etc, are *completely the same* for *all* of the searches, and there are something like 25 of them; this commit only covers the 8 in `providers`, but the next commit should carry all of them. The topmost example has been extracted into its own Web Component, `ak-flow-search`, that takes only two arguments: the type of `FlowInstanceListDesignation` and the current instance of the flow. The static methods for `renderElement`, `renderDescription` and `value` (which are all the same in all 25 instances of `FlowInstancesListRequest`) have been made into standalone functions. `fetchObjects` has been made into a method that takes the parameter from the `designation` property, and `selected` has been turned into a method that takes the comparator instance from the `currentFlow` property. That's it. That's the whole of it. `SearchSelect` now emits an event whenever the user changes the field, and `ak-flow-search` intercepts that event to mirror the value locally. `Form` has been adapted to recognize the `ak-flow-search` element and extract the current value. There are a number of legibility issues remaining, even with this fix. The Authentik Form manager is dependent upon a component named `ak-form-element-horizontal`, which is a container for a single displayed element in a form: ```HTML <ak-form-element-horizontal label=${msg("Authorization flow")} ?required=${true} name="authorizationFlow" > <ak-flow-search flowType=${FlowsInstancesListDesignationEnum.Authorization} .currentFlow=${this.instance?.authorizationFlow} required ></ak-flow-search> <p class="pf-c-form__helper-text"> ${msg("Flow used when authorizing this provider.")} </p> </ak-form-element-horizontal> ``` Imagine, instead, if we could write: ```HTML <ak-form-element-flow-search flowType=${FlowsInstancesListDesignationEnum.Authorization} .currentFlow=${this.instance?.authorizationFlow} required name="authorizationFlow"> <label slot="label">${msg("Authorization flow")}</label> <span slot="help">${msg("Flow used when authorizing this provider.")}</span> <ak-form-element-flow-search> ``` Starting with a superclass that understands the need for `label` and `help` slots, it would automatically configure the input object that would be used. We've already specified multiple identical copies of this thing in multiple different places; centralizing their definition and then re-using them would be classic code re-use. Even better, since the Authorization flow is used 10 times in the whole of our code base, and the Authentication flow 8 times, and they are *all identical*, it would be fitting if we just created wrappers: ```HTML <ak-form-element-flow-search flowType=${FlowsInstancesListDesignationEnum.Authorization}> <ak-form-element-flow-search> ``` That's really all that's needed. There are *hundreds* (about 470 total) cases where nine or more lines of repetitious HTML could be replaced with a one-liner like the above. A "narrow waist" design is one that allows for a system to communicate between two different components through a small but consistent collection of calls. The Form manager needs to be narrowed hard. The `ak-form-element-horizontal` is a wrapper around an input object, and it has this at its core for extracting that information. This forwards the name component to the containing input object so that when the input object generates an event, we can identify the field it's associated with. ```Javascript this.querySelectorAll("*").forEach((input) => { switch (input.tagName.toLowerCase()) { case "input": case "textarea": case "select": case "ak-codemirror": case "ak-chip-group": case "ak-search-select": case "ak-radio": input.setAttribute("name", this.name); break; default: return; } ``` A *temporary* variant of this is in the `ak-flow-search` component, to support this API without having to modify `ak-form-element-horizontal`. And then `ak-form` itself has this: ```Javascript if ( inputElement.tagName.toLowerCase() === "select" && "multiple" in inputElement.attributes ) { const selectElement = inputElement as unknown as HTMLSelectElement; json[element.name] = Array.from(selectElement.selectedOptions).map((v) => v.value); } else if ( inputElement.tagName.toLowerCase() === "input" && inputElement.type === "date" ) { json[element.name] = inputElement.valueAsDate; } else if ( inputElement.tagName.toLowerCase() === "input" && inputElement.type === "datetime-local" ) { json[element.name] = new Date(inputElement.valueAsNumber); } // ... another 20 lines removed ``` This ought to read: ```Javascript const json = elements.filter((element => element instanceof AkFormComponent) .reduce((acc, element) => ({ ...acc, [element.name]: element.value] }); ``` Where, instead of hand-writing all the different input objects for date and datetime and checkbox into our forms, and then having to craft custom value extractors for each and every one of them, just write *one* version of each with all the wrappers and bells and whistles already attached, and have each one of them have a `value` getter descriptor that returns the value expected by our form handler. A back-of-the-envelope estimation is that there's about four *thousand* lines that could disappear if we did this right. More importantly, it would be possible to create new `AkFormComponent`s without having to register them or define them for `ak-form`; as long as they conformed to the AkFormComponent's expectations for "what is a source of values for a Form", `ak-form` would understand how to handle it. Ultimately, what I want is to be able to do this: ``` HTML <ak-input-form itemtype="ak-search" itemid="ak-authentication" itemprop=${this.instance}></ak-inputform> ``` And it will (1) go out and find the right kind of search to put there, (2) conduct the right kind of fetch to fill that search, (3) pre-configure it with the user's current choice in that locale. I don't think this is possible-- for one thing, it would be very expensive in terms of development, and it may break the "narrow waist" ideal by require that the `ak-input-form` object know all the different kinds of searches that are available. The old Midgardian dream was that the object would have *just* the identity triple (A table, a row of that table, a field of that row), and the Javascript would go out and, using the identity, *find* the right object for CRUD (Creating, Retrieving, Updating, and Deleting) it. But that inspiration, as unreachable as it is, is where I'm headed. Where our objects are both *smart* and *standalone*. Where they're polite citizens in an ordered universe, capable of independence sufficient to be tested and validated and trusted, but working in concert to achieve our aims. * web: unravel the search-select for flows completely. This commit removes *all* instances of the search-select for flows, classifying them into four different categories: - a search with no default - a search with a default - a search with a default and a fallback to a static default if non specified - a search with a default and a fallback to the tenant's preferred default if this is a new instance and no flow specified. It's not humanly possible to test all the instances where this has been committed, but the linters are very happy with the results, and I'm going to eyeball every one of them in the github presentation before I move this out of draft. * web: several were declared 'required' that were not. * web: I can't believe this was rejected because of a misspelling in a code comment. Well done\! * web: another codespell fix for a comment. * web: adding 'codespell' to the pre-commit command. Fixed spelling error in eventEmitter. |
||
---|---|---|
.. | ||
admin | ||
assets/images | ||
common | ||
elements | ||
flow | ||
locales | ||
polyfill | ||
standalone | ||
stories | ||
user | ||
custom.css | ||
global.d.ts | ||
locale-codes.ts |