Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
web: enhance search select with portal, overflow, and keyboard contro…
…ls (goauthentik#9517) * 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: enhance search select Patternfly doesn't even *have* a setting for "selected but not hovered," so I had to invent one. I borrowed a trick from MUI and used the light blue "info" color, making it darker on "selected+hovered." This commit starts the revision process for search select. The goal is to have it broken down into four major components: The inline-DOM component, which just shows the current value (or placeholder, if none) and creates the portal for the floating component, then have a higher-level component for the SearchSelect behavior, and a sidecar to manage the keyboard interactivity. This is the portaled component: the actual list. * web: enhance search select. Break menu and Input items into separate handlers. * web: search select: added keyboard controller. * web: search select - the isolation continues This commit brings us to the position of having an independently rendered menu that listens for click events on its contents, records the value internally *and* sends it upstream as an event. This commit also includes a KeyboardController reactor that listens for keyboard events on a list of objects, moving the focus up and down and sending a both a "selected" event when the user presses Enter or Space, and a "close" event when the user presses Escape. A lot of this is just infrastructure. None of these *do* very much; they're just tools for making SearchSelect better. AkSearchSelectView is next: it's responsible for rendering the input and menu items, and then for forwarding the `value` component up to whoever cares. `ak-search-select` will ultimately be responsible for fetching the data and mapping the string tokens from AkSearchSelectView back into the objects that Authentik cares about. * web: search select - a functioning search select So search select is now separated into the following components: - SearchSelectView: Takes the renderables and the selected() Value and draws the Value in a box, then forwards the Options to a portaled drop-down component. - SearchSelectMenuPosition: A web component that renders the Menu into the <BODY> element and positions it with respect to an anchor provided by SearchSelectView. - SearchSelectMenu: Renders the Menu and listens for events indicating an Item has been selected. Sends events through a reference to the View. - SearchKeyboardController: A specialized listener that keeps an independent list of indices and tabstops, and listens for keyboard events to move the index forward or backward, as well as for Event or Space for "select" and Escape for "close". Doesn't actually _do_ these things; they're just semantics implied by the event names, it just sends an event up to the host, which can do what it wants with them. What's not done: - SearchSelect: The interface with the API. Maps to and from API values to renderable Options. One thing of note: of the 35 uses of SearchSelect in our product, 28 of them have `renderElement` annotations of a single field. Six of them use the same annotation (renderFlow), and only one (in EventMatcherPolicyForm) is at all complex. The 28 are: - 7: group.name; - 1: item.label; - 5: item.name; - 1: policy.name; - 1: role.name; - 1: source.name; - 3: stage.name; - 9: user.username; I propose to modify `.renderElement` to take a string of `keyof T`, where T is the type passed to the SearchSelect; it will simply look that up in the object passed in and use that as the Label. `.renderDescription` is more or less similar, except it has _no_ special cases: - 6: html`${flow.name}`; - 1: html`${source.verboseName}`; - 9: html`${user.name}`; - 2: html`${flow.slug}`; Given that, it makes sense to modify this as well to take a field key as a look up and render it, making all that function calling somewhat moot. Selected has a similar issue; passing it a value that is _not_ a function would be a signal to find this specific element in the corresponding 'pk'. Or we could pass a tuple of [keyof T] and value, so we didn't have to hard-code 'pk' into the thing. - 1 return item.pk === this.instance?.createUsersGroup; - 1 return item.pk === this.instance?.filterGroup; - 2 return item.pk === this.instance?.group; - 1 return item.pk === this.instance?.parent; - 1 return item.pk === this.instance?.searchGroup; - 1 return item.pk === this.instance?.syncParentGroup; - 1 return item.pk === this.instance?.policy; - 1 return item.pk === this.instance?.source; - 1 return item.pk === this.instance?.passwordStage; - 1 return item.pk === this.instance?.stage; - 1 return item.pk === this.instance?.user; - 2 return item.pk === this.previewUser?.pk; - 5 return item.pk === this.instance?.configureFlow; - 1 return item.pk === this.instance?.mapping; - 1 return item.pk === this.instance?.nameIdMapping; - 1 return item.pk === this.instance?.user; - 1 return item.pk === this.instance?.webhookMapping; - 1 return item.component === this.instance?.action; - 1 return item.path === this.instance?.path; - 1 return item.name === this.instance?.model; - 1 return item.name === this.instance?.app; - 1 return user.pk.toString() === this.request?.toString(); - 2 return this.request?.user.toString() === user.pk.toString(); And of course, `.value` kinda sorta has the same thing going on: - 6: flow?.pk; - 3: group ? group.pk : undefined; - 4: group?.pk; - 1: item?.component; - 2: item?.name; - 1: item?.path; - 4: item?.pk; - 1: policy?.pk; - 1: role?.pk; - 1: source?.pk; - 3: stage?.pk; - 8: user?.pk; - 1: user?.username; All in all, the _protocol_ for SearchSelect could be streamlined. A _lot_. And still retain the existing power. * Old take; not keeping. * Didn't need this either. * web: search select - a functioning search select with API interface So many edge cases! Because the propagation here is sometimes KeyboardEvent -> MenuEvent -> SearchSelectEvent, I had to rename some of the events to prevent them from creating infinite loops of event handling. This resulted in having to define separate events for Input, Close, and Select. I struggled like heck to get the `<input>` object to show the value after updating. Ultimately, I had to special case the `updated()` method to make sure it was showing the currently chosen display value. Looking through Stack Overflow, there's a lot of contention about the meaning of the `value` field on HTMLInputElements. The API layer distinguishes between a "search" event, which triggers the query to run, and the "select" event, which triggers the component to pick an object as _the_ `.value`. The API layer handles the conversion to GroupedItems and makes sure that the View receives either FlatSelect or GroupedSelect options collections (see ./types, but in practice users should never care too much about this.) * web: completed the search select update * web: search-select reveals a weakness in our plans While testing SearchSelect, I realized that the protocol for our "custom input elements" was neither specified nor documented. I have attempted to fix that, and am finding edge cases and buggy implementations that required addressing. I've described the protocol by creating a class that implements it: AkControlElement. It extends the constructor to always provide the "this is an data-ak-control element," and provides a `json()` method that throws an exception in the base class, so it must always be overriden and never called as super(). I've also fixed ak-dual-select so it carries its name properly into the Forms parser. * web: search select (and friends) This commit finalizes the search select quest! Headline: Search Select is now keyboard-friendly *and* CSS friendly; the styling needed for position is small enough to fit in a `styleMap`, and the styling for the menu itself can be safely locked into a web component. Primarily, I was forgetting to map the value to its displayValue whenever the value was changed from an external source. It should have been an easy catch, but I missed it the first dozen times through. * Not using this yet. ESLint-9 experiment that was loosely left here for some reason. * Added lots of comments. * Added new comments, fixed error message. * Removing a console.log * Fixed an incorrect comment. * Added comments about workaround. * web: focus fixes. Fixes several issues with the drop-down, including primarily how "loss of focus" does not result in the pop-up being banished. Also, the type definition for the attribute `hidden` is inconsistent between Typescript, the attribute, and the related property; I've chosen to route around that problem by using a custom attribute and setting `hidden` in the template, where `lit-analyze` has a workable definition and allows it to pass. Finally, on `open` the focus is passed to the current value, if any.
- Loading branch information
1 parent
49e2eab
commit 5edafa9