441b6ca6b0facde20d693227bae7202d2140b41d
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| 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!
```
|
|||
| bfd0fb66b3 |
web/admin: fix ak-toggle-group for policy and blueprint uses (#6687)
* web/admin: fix ak-toggle-group for policy and blueprint uses Signed-off-by: Jens Langhammer <jens@goauthentik.io> * fix and re-enable lit-analyse Signed-off-by: Jens Langhammer <jens@goauthentik.io> --------- Signed-off-by: Jens Langhammer <jens@goauthentik.io> |
|||
| f5394da9f7 |
web: Replace ad-hoc toggle control with ak-toggle-group (#6470)
* web: Replace ad-hoc toggle control with ak-toggle-group
This commit replaces various ad-hoc implementations of the Patternfly Toggle Group HTML with a web
component that encapsulates all of the needed behavior and exposes a single API with a single event
handler, return the value of the option clicked.
The results are: Lots of visual clutter is eliminated. A single link of:
```
<div class="pf-c-toggle-group__item">
<button
class="pf-c-toggle-group__button ${this.mode === ProxyMode.Proxy
? "pf-m-selected"
: ""}"
type="button"
@click=${() => {
this.mode = ProxyMode.Proxy;
}}>
<span class="pf-c-toggle-group__text">${msg("Proxy")}</span>
</button>
</div>
<div class="pf-c-divider pf-m-vertical" role="separator"></div>
```
Now looks like:
```
<option value=${ProxyMode.Proxy}>${msg("Proxy")}</option>
```
This also means that the three pages that used the Patternfly Toggle Group could eliminate all of
their Patternfly PFToggleGroup needs, as well as the `justify-content: center` extension, which also
eliminated the `css` import.
The savings aren't as spectacular as I'd hoped: removed 178 lines, but added 123; total savings 55
lines of code. I still count this a win: we need never write another toggle component again, and
any bugs, extensions or features we may want to add can be centralized or forked without risking the
whole edifice.
* web: minor code formatting issue.
* web: adding a storybook for the ak-toggle-group component
* Bugs found by CI/CD.
* web: Replace ad-hoc search for CryptoCertificateKeyPairs with crypto-certificate-search (#6475)
* web: Replace ad-hoc search for CryptoCertificateKeyPairs with ak-crypto-certeficate-search
This commit replaces various ad-hoc implementations of `search-select` for CryptoCertificateKeyPairs
with a web component that encapsulates all of the needed behavior and exposes a single API.
The results are: Lots of visual clutter is eliminated. A single search of:
```HTML
<ak-search-select
.fetchObjects=${async (query?: string): Promise<CertificateKeyPair[]> => {
const args: CryptoCertificatekeypairsListRequest = {
ordering: "name",
hasKey: true,
includeDetails: false,
};
if (query !== undefined) {
args.search = query;
}
const certificates = await new CryptoApi(
DEFAULT_CONFIG,
).cryptoCertificatekeypairsList(args);
return certificates.results;
}}
.renderElement=${(item: CertificateKeyPair): string => {
return item.name;
}}
.value=${(item: CertificateKeyPair | undefined): string | undefined => {
return item?.pk;
}}
.selected=${(item: CertificateKeyPair): boolean => {
return this.instance?.tlsVerification === item.pk;
}}
?blankable=${true}
>
</ak-search-select>
```
Now looks like:
```HTML
<ak-crypto-certificate-search certificate=${this.instance?.tlsVerification}>
</ak-crypto-certificate-search>
```
There are three searches that do not require there to be a valid key with the certificate; these are
supported with the boolean property `nokey`; likewise, there is one search (in SAMLProviderForm)
that states that if there is no current certificate in the SAMLProvider and only one certificate can
be found in the Authentik database, use that one; this is supported with the boolean property
`singleton`.
These changes replace 382 lines of object-oriented invocations with 36 lines of declarative
configuration, and 98 lines for the class. Overall, the code for "find a crypto certificate" has
been reduced by 46%.
Suggestions for a better word than `singleton` are welcome!
* web: display tests for CryptoCertificateKeypair search
This adds a Storybook for the CryptoCertificateKeypair search, including
a mock fetch of the data. In the course of running the tests, we discovered
that including the SearchSelect _class_ won't include the customElement declaration
unless you include the whole file! Other bugs found: including the CSS from
Storybook is different from that of LitElement native, so much so that the
adapter needed to be included. FlowSearch had a similar bug. The problem
only manifests when building via Webpack (which Storybook uses) and not
Rollup, but we should support both in distribution.
|
