Skip to content

Commit

Permalink
web: small fixes for elements and forms (goauthentik#11546)
Browse files Browse the repository at this point in the history
* web: small fixes for wdio and lint

- Roll back another dependabot breaking change, this time to WebdriverIO
- Remove the redundant scripts wrapping ESLint for Precommit mode. Access to those modes is
  available through the flags to the `./web/scripts/eslint.mjs` script.
- Remove SonarJS checks until SonarJS is ESLint 9 compatible.
- Minor nitpicking.

* package-lock.json update

* web: small fixes for wdio and lint

**PLEASE** Stop trying to upgrade WebdriverIO following Dependabot's instructions. The changes
between wdio8 and wdio9 are extensive enough to require a lot more manual intervention. The unit
tests fail in wdio 9, with the testbed driver Wdio uses to compile content to push to the browser
([vite](https://vitejs.dev) complaining:

```
2024-09-27T15:30:03.672Z WARN @wdio/browser-runner:vite: warning: Unrecognized default export in file /Users/ken/projects/dev/web/node_modules/@patternfly/patternfly/components/Dropdown/dropdown.css
  Plugin: postcss-lit
  File: /Users/ken/projects/dev/web/node_modules/@patternfly/patternfly/components/Dropdown/dropdown.css
[0-6] 2024-09-27T15:30:04.083Z INFO webdriver: BIDI COMMAND script.callFunction {"functionDeclaration":"<Function[976 bytes]>","awaitPromise":true,"arguments":[],"target":{"context":"8E608E6D13E355DFFC28112C236B73AF"}}
[0-6]  Error:  Test failed due to following error(s):
  - ak-search-select.test.ts: The requested module '/src/common/styles/authentik.css' does not provide an export named 'default': SyntaxError: The requested module '/src/common/styles/authentik.css' does not provide an export named 'default'

```

So until we can figure out why the Vite installation isn't liking our CSS import scheme, we'll
have to soldier on with what we have.  At least with Wdio 8, we get:

```
Spec Files:      7 passed, 7 total (100% completed) in 00:00:19
```

* Forgot to run prettier.

* web: small fixes for elements and forms

- provides a new utility, `_isSlug_`, used to verify a user input
- extends the ak-horizontal-component wrapper to have a stronger identity and available value
- updates the types that use the wrapper to be typed more strongly
  - (Why) The above are used in the wizard to get and store values
- fixes a bug in SearchSelectEZ that broke the display if the user didn't supply a `groupBy` field.
- Adds `@wdio/types` to the package file so eslint is satisfied wdio builds correctly
- updates the end-to-end test to understand the revised button identities on the login page
  - Running the end-to-end tests verifies that changes to the components listed above did not break
    the semantics of those components.

* Removing SonarJS comments.

* Reverting to log level  for tests.
  • Loading branch information
kensternberg-authentik authored Oct 3, 2024
1 parent 22a77a7 commit 5257370
Show file tree
Hide file tree
Showing 15 changed files with 67 additions and 43 deletions.
12 changes: 6 additions & 6 deletions web/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions web/src/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ export function convertToSlug(text: string): string {
.replace(/[^\w-]+/g, "");
}

export function isSlug(text: string): boolean {
const lowered = text.toLowerCase();
const forbidden = /([^\w-]|\s)/.test(lowered);
return lowered === text && !forbidden;
}

/**
* Truncate a string based on maximum word count
*/
Expand Down
6 changes: 5 additions & 1 deletion web/src/components/HorizontalLightComponent.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { AKElement } from "@goauthentik/elements/Base";
import "@goauthentik/elements/forms/HorizontalFormElement.js";

import { TemplateResult, html, nothing } from "lit";
import { property } from "lit/decorators.js";

type HelpType = TemplateResult | typeof nothing;

export class HorizontalLightComponent extends AKElement {
export class HorizontalLightComponent<T> extends AKElement {
// Render into the lightDOM. This effectively erases the shadowDOM nature of this component, but
// we're not actually using that and, for the meantime, we need the form handlers to be able to
// find the children of this component.
Expand Down Expand Up @@ -41,6 +42,9 @@ export class HorizontalLightComponent extends AKElement {
@property({ attribute: false })
errorMessages: string[] = [];

@property({ attribute: false })
value?: T;

renderControl() {
throw new Error("Must be implemented in a subclass");
}
Expand Down
10 changes: 8 additions & 2 deletions web/src/components/ak-number-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@ import { ifDefined } from "lit/directives/if-defined.js";
import { HorizontalLightComponent } from "./HorizontalLightComponent";

@customElement("ak-number-input")
export class AkNumberInput extends HorizontalLightComponent {
export class AkNumberInput extends HorizontalLightComponent<number> {
@property({ type: Number, reflect: true })
value = 0;
value = NaN;

renderControl() {
const setValue = (ev: InputEvent) => {
const value = (ev.target as HTMLInputElement).value;
this.value = value.trim() === "" ? NaN : parseInt(value, 10);
};

return html`<input
type="number"
@input=${setValue}
value=${ifDefined(this.value)}
class="pf-c-form-control"
?required=${this.required}
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/ak-radio-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { customElement, property } from "lit/decorators.js";
import { HorizontalLightComponent } from "./HorizontalLightComponent";

@customElement("ak-radio-input")
export class AkRadioInput<T> extends HorizontalLightComponent {
export class AkRadioInput<T> extends HorizontalLightComponent<T> {
@property({ type: Object })
value!: T;

Expand Down
2 changes: 1 addition & 1 deletion web/src/components/ak-slug-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { ifDefined } from "lit/directives/if-defined.js";
import { HorizontalLightComponent } from "./HorizontalLightComponent";

@customElement("ak-slug-input")
export class AkSlugInput extends HorizontalLightComponent {
export class AkSlugInput extends HorizontalLightComponent<string> {
@property({ type: String, reflect: true })
value = "";

Expand Down
7 changes: 6 additions & 1 deletion web/src/components/ak-text-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,18 @@ import { ifDefined } from "lit/directives/if-defined.js";
import { HorizontalLightComponent } from "./HorizontalLightComponent";

@customElement("ak-text-input")
export class AkTextInput extends HorizontalLightComponent {
export class AkTextInput extends HorizontalLightComponent<string> {
@property({ type: String, reflect: true })
value = "";

renderControl() {
const setValue = (ev: InputEvent) => {
this.value = (ev.target as HTMLInputElement).value;
};

return html` <input
type="text"
@input=${setValue}
value=${ifDefined(this.value)}
class="pf-c-form-control"
?required=${this.required}
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/ak-textarea-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { customElement, property } from "lit/decorators.js";
import { HorizontalLightComponent } from "./HorizontalLightComponent";

@customElement("ak-textarea-input")
export class AkTextareaInput extends HorizontalLightComponent {
export class AkTextareaInput extends HorizontalLightComponent<string> {
@property({ type: String, reflect: true })
value = "";

Expand Down
3 changes: 2 additions & 1 deletion web/src/elements/cards/tests/QuickActionCard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ describe("ak-quick-actions-card", () => {
></ak-quick-actions-card>`,
);
const component = await $("ak-quick-actions-card");
const items = await component.$$(">>>.pf-c-list li").getElements();
const items = await component.$$(">>>.pf-c-list li");
// @ts-expect-error "Another ChainablePromise mistake"
await expect(Array.from(items).length).toEqual(5);
await expect(await component.$(">>>.pf-c-list li:nth-of-type(4)")).toHaveText(
"Manage users",
Expand Down
6 changes: 4 additions & 2 deletions web/src/elements/forms/SearchSelect/ak-search-select-ez.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export interface ISearchSelectApi<T> {
renderDescription?: (element: T) => string | TemplateResult;
value: (element: T | undefined) => unknown;
selected?: (element: T, elements: T[]) => boolean;
groupBy: (items: T[]) => [string, T[]][];
groupBy?: (items: T[]) => [string, T[]][];
}

export interface ISearchSelectEz<T> extends ISearchSelectBase<T> {
Expand Down Expand Up @@ -58,7 +58,9 @@ export class SearchSelectEz<T> extends SearchSelectBase<T> implements ISearchSel
this.renderDescription = this.config.renderDescription;
this.value = this.config.value;
this.selected = this.config.selected;
this.groupBy = this.config.groupBy;
if (this.config.groupBy !== undefined) {
this.groupBy = this.config.groupBy;
}
super.connectedCallback();
}
}
Expand Down
19 changes: 6 additions & 13 deletions web/src/elements/forms/SearchSelect/ak-search-select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@ export interface ISearchSelect<T> extends ISearchSelectBase<T> {
* The API layer of ak-search-select
*
* - @prop fetchObjects (Function): The function by which objects are retrieved by the API.
* - @prop renderElement (Function | string): Either a function that can retrieve the string
* "label" of the element, or the name of the field from which the label can be retrieved.¹
* - @prop renderDescription (Function | string): Either a function that can retrieve the string
* or TemplateResult "description" of the element, or the name of the field from which the
* description can be retrieved.¹
* - @prop value (Function | string): Either a function that can retrieve the value (the current
* API object's primary key) selected or the name of the field from which the value can be
* retrieved.¹
* - @prop renderElement (Function): A function that can retrieve the string
* "label" of the element
* - @prop renderDescription (Function): A function that can retrieve the string
* or TemplateResult "description" of the element
* - @prop value (Function | string): A function that can retrieve the value (the current
* API object's primary key) selected.
* - @prop selected (Function): A function that retrieves the current "live" value from the
list of objects fetched by the function above.
* - @prop groupBy (Function): A function that can group the objects fetched from the API by
Expand All @@ -41,11 +39,6 @@ export interface ISearchSelect<T> extends ISearchSelectBase<T> {
* shown if `blankable`
* - @attr selectedObject (Object<T>): The current object, or undefined, selected
*
* ¹ Due to a limitation in the parsing of properties-vs-attributes, these must be defined as
* properties, not attributes. As a consequence, they must be declared in property syntax.
* Example:
*
* `.renderElement=${"name"}`
*
* - @fires ak-change - When a value from the collection has been positively chosen, either as a
* consequence of the user typing or when selecting from the list.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export class AkSearchSelectViewDriver {
);
}
const id = await element.getAttribute("data-ouia-component-id");
const menu = await $(`[data-ouia-component-id="menu-${id}"]`).getElement();
const menu = await $(`[data-ouia-component-id="menu-${id}"]`);
// @ts-expect-error "Another ChainablePromise mistake"
return new AkSearchSelectViewDriver(element, menu);
}

Expand All @@ -52,7 +53,7 @@ export class AkSearchSelectViewDriver {
}

async listElements() {
return await this.menu.$$(">>>li").getElements();
return await this.menu.$$(">>>li");
}

async focusOnInput() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ describe("Search select: Test Input Field", () => {
html`<ak-search-select-view .options=${longGoodForYouPairs}> </ak-search-select-view>`,
document.body,
);
select = await AkSearchSelectViewDriver.build(
await $("ak-search-select-view").getElement(),
);
// @ts-expect-error "Another ChainablePromise mistake"
select = await AkSearchSelectViewDriver.build(await $("ak-search-select-view"));
});

it("should open the menu when the input is clicked", async () => {
Expand Down Expand Up @@ -58,9 +57,8 @@ describe("Search select: Test Input Field", () => {
expect(await select.open).toBe(false);
expect(await select.menuIsVisible()).toBe(false);
await browser.keys("A");
select = await AkSearchSelectViewDriver.build(
await $("ak-search-select-view").getElement(),
);
// @ts-expect-error "Another ChainablePromise mistake"
select = await AkSearchSelectViewDriver.build(await $("ak-search-select-view"));
expect(await select.open).toBe(true);
expect(await select.menuIsVisible()).toBe(true);
});
Expand All @@ -69,6 +67,7 @@ describe("Search select: Test Input Field", () => {
await select.focusOnInput();
await browser.keys("Ap");
await expect(await select.menuIsVisible()).toBe(true);
// @ts-expect-error "Another ChainablePromise mistake"
const elements = Array.from(await select.listElements());
await expect(elements.length).toBe(2);
});
Expand All @@ -77,6 +76,7 @@ describe("Search select: Test Input Field", () => {
await select.focusOnInput();
await browser.keys("Ap");
await expect(await select.menuIsVisible()).toBe(true);
// @ts-expect-error "Another ChainablePromise mistake"
const elements = Array.from(await select.listElements());
await expect(elements.length).toBe(2);
await browser.keys(Key.Tab);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,8 @@ describe("Search select: event driven startup", () => {
mock?.dispatchEvent(new Event("resolve"));
});
expect(await $(">>>ak-search-select-loading-indicator")).not.toBeDisplayed();
select = await AkSearchSelectViewDriver.build(
await $(">>>ak-search-select-view").getElement(),
);
// @ts-expect-error "Another ChainablePromise mistake"
select = await AkSearchSelectViewDriver.build(await $(">>>ak-search-select-view"));
expect(await select).toBeExisting();
});

Expand Down
13 changes: 10 additions & 3 deletions web/wdio.conf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,14 @@ export const config: WebdriverIO.Config = {
},
],

tsConfigPath: "./tsconfig.json",
// @ts-expect-error TS2353: The types are not up-to-date with Wdio9.
autoCompileOpts: {
autoCompile: true,
tsNodeOpts: {
project: "./tsconfig.json",
transpileOnly: true,
},
},

//
// ==================
Expand Down Expand Up @@ -141,11 +148,11 @@ export const config: WebdriverIO.Config = {
// baseUrl: 'http://localhost:8080',
//
// Default timeout for all waitFor* commands.
waitforTimeout: 10000,
waitforTimeout: 12000,
//
// Default timeout in milliseconds for request
// if browser driver or grid doesn't send response
connectionRetryTimeout: 120000,
connectionRetryTimeout: 12000,
//
// Default request retries count
connectionRetryCount: 3,
Expand Down

0 comments on commit 5257370

Please sign in to comment.