Commit Graph

5 Commits

Author SHA1 Message Date
79c01ca473 web: update to ESLint 9 (#10812)
* web: update to ESLint 9

ESLint 9 has been out for awhile now, and all of the plug-ins that we use have caught up, so it is
time to bite the bullet and upgrade.  This commit:

- upgrades to ESLint 9, and upgrades all associated plugins
- Replaces the `.eslintrc` and `.eslintignore` files with the new, "flat" configuration file,
  "eslint.config.mjs".
- Places the previous "precommit" and "nightmare" rules in `./scripts/eslint.precommit.mjs` and
  `./scripts/eslint.nightmare.mjs`, respectively
- Replaces the scripted wrappers for eslint (`eslint`, `eslint-precommit`) with a single executable
  that takes the arguments `--precommit`, which applies a stricter set of rules, and `--nightmare`,
  which applies an even more terrifyingly strict set of rules.
- Provides the scripted wrapper `./scripts/eslint.mjs` so that eslint can be run from `bun`, if one
  so chooses.
- Fixes *all* of the lint `eslint.config.mjs` now finds, including removing all of the `eslint`
  styling rules and overrides because Eslint now proudly leaves that entirely up to Prettier.

To shut Dependabot up about ESLint.

* Added explanation for no-console removal.

* web: did not need the old and unmaintained nightmare mode; it can be configured directly.
2024-08-07 15:04:18 -07:00
d0a459076b web: enhance search select with portal, overflow, and keyboard controls (#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.
2024-07-15 17:54:06 -07:00
ee58cf0c1c web: add HTMLTagNameElementMaps to everything to activate lit analyzer (#10217)
* 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: add more linting

* A reliable test for the extra code needed in analyzer, passing shellcheck

* web: re-enable custom-element-manifest and enable component checking in Typescript

This commit includes a monkeypatch to allow custom-element-manifest (CEM) to work correctly again
despite our rich collection of mixins, reactive controllers, symbol-oriented event handlers, and the
like. With that monkeypatch in place, we can now create the CEM manifest file and then exploit it so
that IDEs and the Typescript compilation pass can tell when a component is being used incorrectly;
when the wrong types are being passed to it, or when a required attribute is not initialized.

* Added building the manifest to the build process, rather than storing it.  It is not appreciably slow.

* web: the most boring PR in the universe: Add HTMLTagNameElementMap to everyhing

This commit adds HTMLTagNameElementMap entries to every web component in the front end. Activating
and associating the HTMLTagNamElementMap with its class has enabled
[LitAnalyzer](https://github.com/runem/lit-analyzer/tree/master/packages/lit-analyzer) to reveal a
*lot* of basic problems within the UI, the most popular of which is "missing import." We usually get
away with it because the object being imported was already registered with the browser elsewhere,
but it still surprises me that we haven't gotten any complaints over things like:

```
./src/flow/stages/base.ts
Missing import for <ak-form-static>
96:  <ak-form-static
no-missing-import
```

Given how early and fundamental that seems to be in our code, I'd have expected to hear _something_
about it.

I have not enabled most of the possible checks because, well, there are just a ton of warnings when
I do.  I'd like to get in and fix those.

Aside from this, I have also _removed_ `customElement` declarations from anything declared as an
`abstract class`. It makes no sense to try and instantiate something that cannot, by definition, be
instantiated.  If the class is capable of running on its own, it's not abstract, it just needs to be
overridden in child classes.  Before removing the declaration I did check to make sure no other
piece of code was even *trying* to instantiate it, and so far I have detected no failures.  Those
elements were:

- elements/forms/Form.ts
- element-/wizard/WizardFormPage.ts

The one that blows my mind, though, is this:

```
src/elements/forms/ProxyForm.ts
6-@customElement("ak-proxy-form")
7:export abstract class ProxyForm extends Form<unknown> {
```

Which, despite being `abstract`, is somehow instantiable?

```
src/admin/outposts/ServiceConnectionListPage.ts:    <ak-proxy-form
src/admin/providers/ProviderListPage.ts:    <ak-proxy-form
src/admin/sources/SourceWizard.ts:    <ak-proxy-form
src/admin/sources/SourceListPage.ts:    <ak-proxy-form
src/admin/providers/ProviderWizard.ts:    <ak-proxy-form type=${type.component}></ak-proxy-form>
src/admin/stages/StageListPage.ts:    <ak-proxy-form
```

I've made a note to investigate.

I've started a new folder where all of my one-off tools for *how* a certain PR was run.  It has a
README describing what it's for, and the first tool, `add-htmlelementtagnamemaps-to-everything`, is
its first entry.  That tool is also documented internally.

``` Gilbert & Sullivan

I've got a little list,
I've got a little list,
Of all the code that would never be missed,
The duplicate code of cute-and-paste,
The weak abstractions that lead to waste,
The embedded templates-- you get the gist,
There ain't none of 'em that will ever be missed,
And that's why I've got them on my list!

```
2024-07-15 10:54:22 -07:00
ef2a40ed7d 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.
2024-05-10 09:50:07 -07:00
fcf752905b web: ak-checkbox-group for short, static, multi-select events (#9138)
* 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: ak-checkbox-group for short, static, multi-select events

Implements a checkbox groups web component, wholly independent of the API
(although it does implement the 'data-ak-control' protocol, including the
`json()` method that makes it easier to send the data to the Form handler).  The
controller works much like multi-select: `value` returns an array of strings,
the `name` attribute associated with whatever it is you're asking about.

The `required` property only works if you give the whole item a name, as if it
were an input.  Otherwise, it does nothing.

Giving it a `name` also activates the browser standard `formAssociated`
protocol; it works just fine for ordinary HTML forms, and presents to that
protocol the `FormValue` type, so any form using it will automagically convert
it into the CGI (Common Gateway Interface) format of, to use the example from
Storybook:

```
ak-test-checkgroup-input=funky&ak-test-checkgroup-input=invalid
```

Note that the classic CGI format is not automatically key/value; keys can appear
multiple times, and indicate that the value is an array of strings.  Most modern
appservers understand this format. Some do not.

There's a full and complete JSDOC-like comment documenting the component.  I
have even provided CSSPart sections for everything: the wrapper, each line, the
input and its associated label.  The brave or foolhardy can mangle the CSS to
their hearts' content without having to know a thing about Patternfly.

* fix styling alignment with top line

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

---------

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
Co-authored-by: Jens Langhammer <jens@goauthentik.io>
2024-04-05 09:47:38 -07:00