habsi/authentik
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

web: fix flash of unstructured content, add tests for it (#11013)

Browse Source 
* web: fix Flash of Unstructured Content while SearchSelect is loading from the backend

Provide an alternative, readonly, disabled, unindexed input object with the text "Loading...", to be
replaced with the _real_ input element after the content is loaded.

This provides the correct appearance and spacing so the content doesn't jiggle about between the
start of loading and the SearchSelect element being finalized.  It was visually distracting and
unappealing.

* web: comment on state management in API layer, move file to point to correct component under test.

* web: test for flash of unstructured content

- Add a unit test to ensure the "Loading..." element is displayed correctly before data arrives
- Demo how to mock a `fetchObjects()` call in testing. Very cool.
- Make distinguishing rule sets for code, tests, and scripts in nightmare mode
- In SearchSelect, Move the `styles()` declaration to the top of the class for consistency.

- To test for the FLOUC issue in SearchSelect.

This is both an exercise in mocking @beryju's `fetchObjects()` protocol, and shows how we can unit
test generic components that render API objects.
This commit is contained in:
Ken Sternberg
2024-08-22 02:17:30 -07:00
committed by GitHub
parent 40b93e9b10
commit 85eb104966
8 changed files with 430 additions and 235 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
web/src/elements/forms/SearchSelect/SearchSelect.ts
View File

@ -8,7 +8,7 @@ import { CustomEmitterElement } from "@goauthentik/elements/utils/eventEmitter";
import { randomId } from "@goauthentik/elements/utils/randomId.js";
import { msg } from "@lit/localize";
import { TemplateResult, html } from "lit";
import { PropertyValues, TemplateResult, html } from "lit";
import { property, state } from "lit/decorators.js";
import { ifDefined } from "lit/directives/if-defined.js";
@ -16,6 +16,7 @@ import PFBase from "@patternfly/patternfly/patternfly-base.css";
import { ResponseError } from "@goauthentik/api";
import "./ak-search-select-loading-indicator.js";
import "./ak-search-select-view.js";
import { SearchSelectView } from "./ak-search-select-view.js";
@ -120,6 +121,7 @@ export class SearchSelectBase<T>
return Promise.resolve();
}
this.isFetchingData = true;
this.dispatchEvent(new Event("loading"));
return this.fetchObjects(this.query)
.then((objects) => {
objects.forEach((obj) => {
@ -228,8 +230,15 @@ export class SearchSelectBase<T>
return html`<em>${msg("Failed to fetch objects: ")} ${this.error.detail}</em>`;
}
// `this.objects` is both a container and a sigil; if it is in the `undefined` state, it's a
// marker that this component has not yet completed a *first* load. After that, it should
// never be empty. The only state that allows it to be empty after a successful retrieval is
// a subsequent retrieval failure, in which case `this.error` above will be populated and
// displayed before this.
if (!this.objects) {
return html`${msg("Loading...")}`;
return html`<ak-search-select-loading-indicator
tabindex="-1"
></ak-search-select-loading-indicator>`;
}
const options = this.getGroupedItems();
@ -248,7 +257,10 @@ export class SearchSelectBase<T>
></ak-search-select-view> `;
}
public override updated() {
public override updated(changed: PropertyValues<this>) {
if (!this.isFetchingData && changed.has("objects")) {
this.dispatchEvent(new Event("ready"));
}
// It is not safe for automated tests to interact with this component while it is fetching
// data.
if (!this.isFetchingData) {

4
web/src/elements/forms/SearchSelect/ak-search-select-ez.ts
View File

@ -1,8 +1,6 @@
import { TemplateResult } from "lit";
import { customElement, property } from "lit/decorators.js";
import PFBase from "@patternfly/patternfly/patternfly-base.css";
import { type ISearchSelectBase, SearchSelectBase } from "./SearchSelect.js";
export interface ISearchSelectApi<T> {
@ -48,7 +46,7 @@ export interface ISearchSelectEz<T> extends ISearchSelectBase<T> {
@customElement("ak-search-select-ez")
export class SearchSelectEz<T> extends SearchSelectBase<T> implements ISearchSelectEz<T> {
static get styles() {
return [PFBase];
return [...SearchSelectBase.styles];
}
@property({ type: Object, attribute: false })

64
web/src/elements/forms/SearchSelect/ak-search-select-loading-indicator.ts Normal file
View File

@ -0,0 +1,64 @@
import { AKElement } from "@goauthentik/elements/Base.js";
import { randomId } from "@goauthentik/elements/utils/randomId.js";
import { msg } from "@lit/localize";
import { html } from "lit";
import { customElement } from "lit/decorators.js";
import PFFormControl from "@patternfly/patternfly/components/FormControl/form-control.css";
import PFSelect from "@patternfly/patternfly/components/Select/select.css";
import PFBase from "@patternfly/patternfly/patternfly-base.css";
/**
* @class SearchSelectLoadingIndicator
* @element ak-search-select-loading-indicator
*
* Just a loading indicator to fill in while we wait for the view to settle
*
* ## Available CSS `part::`
*
* - @part ak-search-select: The main Patternfly div
* - @part ak-search-select-toggle: The Patternfly inner div
* - @part ak-search-select-wrapper: Yet another Patternfly inner div
* - @part ak-search-select-loading-indicator: The input object that hosts the "Loading..." message
*/
@customElement("ak-search-select-loading-indicator")
export class SearchSelectLoadingIndicator extends AKElement {
static get styles() {
return [PFBase, PFFormControl, PFSelect];
}
connectedCallback() {
super.connectedCallback();
this.setAttribute("data-ouia-component-type", "ak-search-select-loading-indicator");
this.setAttribute("data-ouia-component-id", this.getAttribute("id") || randomId());
this.setAttribute("data-ouia-component-safe", "true");
}
render() {
return html`
<div class="pf-c-select" part="ak-search-select">
<div class="pf-c-select__toggle pf-m-typeahead" part="ak-search-select-toggle">
<div class="pf-c-select__toggle-wrapper" part="ak-search-select-wrapper">
<input
class="pf-c-form-control pf-c-select__toggle-typeahead"
part="ak-search-select-loading-indicator"
type="text"
disabled
readonly
tabindex="-1"
value=${msg("Loading...")}
/>
</div>
</div>
</div>
`;
}
}
declare global {
interface HTMLElementTagNameMap {
"ak-search-select-loading-indicator": SearchSelectLoadingIndicator;
}
}

8
web/src/elements/forms/SearchSelect/ak-search-select-view.ts
View File

@ -69,6 +69,10 @@ export interface ISearchSelectView {
*/
@customElement("ak-search-select-view")
export class SearchSelectView extends AKElement implements ISearchSelectView {
static get styles() {
return [PFBase, PFForm, PFFormControl, PFSelect];
}
/**
* The options collection. The simplest variant is just [key, label, optional<description>]. See
* the `./types.ts` file for variants and how to use them.
@ -186,10 +190,6 @@ export class SearchSelectView extends AKElement implements ISearchSelectView {
*/
flatOptions: [string, SelectOption][] = [];
static get styles() {
return [PFBase, PFForm, PFFormControl, PFSelect];
}
connectedCallback() {
super.connectedCallback();
this.setAttribute("data-ouia-component-type", "ak-search-select-view");

4
web/src/elements/forms/SearchSelect/ak-search-select.ts
View File

@ -3,8 +3,6 @@ import { groupBy } from "@goauthentik/common/utils";
import { TemplateResult } from "lit";
import { customElement, property } from "lit/decorators.js";
import PFBase from "@patternfly/patternfly/patternfly-base.css";
import { type ISearchSelectBase, SearchSelectBase } from "./SearchSelect.js";
export interface ISearchSelect<T> extends ISearchSelectBase<T> {
@ -57,7 +55,7 @@ export interface ISearchSelect<T> extends ISearchSelectBase<T> {
@customElement("ak-search-select")
export class SearchSelect<T> extends SearchSelectBase<T> implements ISearchSelect<T> {
static get styles() {
return [PFBase];
return [...SearchSelectBase.styles];
}
// A function which takes the query state object (accepting that it may be empty) and returns a

104
web/src/elements/forms/SearchSelect/tests/ak-search-select-view.test.ts Normal file
View File

@ -0,0 +1,104 @@
import { $, browser } from "@wdio/globals";
import { slug } from "github-slugger";
import { Key } from "webdriverio";
import { html, render } from "lit";
import "../ak-search-select-view.js";
import { sampleData } from "../stories/sampleData.js";
import { AkSearchSelectViewDriver } from "./ak-search-select-view.comp.js";
const longGoodForYouPairs = {
grouped: false,
options: sampleData.map(({ produce }) => [slug(produce), produce]),
};
describe("Search select: Test Input Field", () => {
let select: AkSearchSelectViewDriver;
beforeEach(async () => {
await render(
html`<ak-search-select-view .options=${longGoodForYouPairs}> </ak-search-select-view>`,
document.body,
);
// @ts-ignore
select = await AkSearchSelectViewDriver.build(await $("ak-search-select-view"));
});
it("should open the menu when the input is clicked", async () => {
expect(await select.open).toBe(false);
expect(await select.menuIsVisible()).toBe(false);
await select.clickInput();
expect(await select.open).toBe(true);
// expect(await select.menuIsVisible()).toBe(true);
});
it("should not open the menu when the input is focused", async () => {
expect(await select.open).toBe(false);
await select.focusOnInput();
expect(await select.open).toBe(false);
expect(await select.menuIsVisible()).toBe(false);
});
it("should close the menu when the input is clicked a second time", async () => {
expect(await select.open).toBe(false);
expect(await select.menuIsVisible()).toBe(false);
await select.clickInput();
expect(await select.menuIsVisible()).toBe(true);
expect(await select.open).toBe(true);
await select.clickInput();
expect(await select.open).toBe(false);
expect(await select.open).toBe(false);
});
it("should open the menu from a focused but closed input when a search is begun", async () => {
expect(await select.open).toBe(false);
await select.focusOnInput();
expect(await select.open).toBe(false);
expect(await select.menuIsVisible()).toBe(false);
await browser.keys("A");
expect(await select.open).toBe(true);
expect(await select.menuIsVisible()).toBe(true);
});
it("should update the list as the user types", async () => {
await select.focusOnInput();
await browser.keys("Ap");
expect(await select.menuIsVisible()).toBe(true);
const elements = Array.from(await select.listElements());
expect(elements.length).toBe(2);
});
it("set the value when a match is close", async () => {
await select.focusOnInput();
await browser.keys("Ap");
expect(await select.menuIsVisible()).toBe(true);
const elements = Array.from(await select.listElements());
expect(elements.length).toBe(2);
await browser.keys(Key.Tab);
expect(await (await select.input()).getValue()).toBe("Apples");
});
it("should close the menu when the user clicks away", async () => {
document.body.insertAdjacentHTML(
"afterbegin",
'<input id="a-separate-component" type="text" />',
);
const input = await browser.$("#a-separate-component");
await select.clickInput();
expect(await select.open).toBe(true);
await input.click();
expect(await select.open).toBe(false);
});
afterEach(async () => {
await document.body.querySelector("#a-separate-component")?.remove();
await document.body.querySelector("ak-search-select-view")?.remove();
// @ts-expect-error expression of type '"_$litPart$"' is added by Lit
if (document.body["_$litPart$"]) {
// @ts-expect-error expression of type '"_$litPart$"' is added by Lit
delete document.body["_$litPart$"];
}
});
});

168
web/src/elements/forms/SearchSelect/tests/ak-search-select.test.ts
View File

@ -1,100 +1,108 @@
/* eslint-env jest */
import { AKElement } from "@goauthentik/elements/Base";
import { bound } from "@goauthentik/elements/decorators/bound.js";
import { CustomListenerElement } from "@goauthentik/elements/utils/eventEmitter";
import { $, browser } from "@wdio/globals";
import { slug } from "github-slugger";
import { Key } from "webdriverio";
import { html, render } from "lit";
import { customElement } from "lit/decorators.js";
import { property, query } from "lit/decorators.js";
import "../ak-search-select-view.js";
import { sampleData } from "../stories/sampleData.js";
import "../ak-search-select.js";
import { SearchSelect } from "../ak-search-select.js";
import { type ViewSample, sampleData } from "../stories/sampleData.js";
import { AkSearchSelectViewDriver } from "./ak-search-select-view.comp.js";
const longGoodForYouPairs = {
grouped: false,
options: sampleData.map(({ produce }) => [slug(produce), produce]),
};
const renderElement = (fruit: ViewSample) => fruit.produce;
describe("Search select: Test Input Field", () => {
const renderDescription = (fruit: ViewSample) => html`${fruit.desc}`;
const renderValue = (fruit: ViewSample | undefined) => slug(fruit?.produce ?? "");
@customElement("ak-mock-search-group")
export class MockSearch extends CustomListenerElement(AKElement) {
/**
* The current fruit
*
* @attr
*/
@property({ type: String, reflect: true })
fruit?: string;
@query("ak-search-select")
search!: SearchSelect<ViewSample>;
selectedFruit?: ViewSample;
get value() {
return this.selectedFruit ? renderValue(this.selectedFruit) : undefined;
}
@bound
handleSearchUpdate(ev: CustomEvent) {
ev.stopPropagation();
this.selectedFruit = ev.detail.value;
this.dispatchEvent(new InputEvent("input", { bubbles: true, composed: true }));
}
@bound
selected(fruit: ViewSample) {
return this.fruit === slug(fruit.produce);
}
@bound
fetchObjects() {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const resolver = (resolve: any) => {
this.addEventListener("resolve", () => {
resolve(sampleData);
});
};
return new Promise(resolver);
}
render() {
return html`
<ak-search-select
.fetchObjects=${this.fetchObjects}
.renderElement=${renderElement}
.renderDescription=${renderDescription}
.value=${renderValue}
.selected=${this.selected}
managed
@ak-change=${this.handleSearchUpdate}
?blankable=${true}
>
</ak-search-select>
`;
}
}
describe("Search select: event driven startup", () => {
let select: AkSearchSelectViewDriver;
let wrapper: SearchSelect<ViewSample>;
beforeEach(async () => {
await render(
html`<ak-search-select-view .options=${longGoodForYouPairs}> </ak-search-select-view>`,
document.body,
);
await render(html`<ak-mock-search-group></ak-mock-search-group>`, document.body);
// @ts-ignore
select = await AkSearchSelectViewDriver.build(await $("ak-search-select-view"));
wrapper = await $(">>>ak-search-select");
});
it("should open the menu when the input is clicked", async () => {
expect(await select.open).toBe(false);
expect(await select.menuIsVisible()).toBe(false);
await select.clickInput();
expect(await select.open).toBe(true);
// expect(await select.menuIsVisible()).toBe(true);
});
it("should not open the menu when the input is focused", async () => {
expect(await select.open).toBe(false);
await select.focusOnInput();
expect(await select.open).toBe(false);
expect(await select.menuIsVisible()).toBe(false);
});
it("should close the menu when the input is clicked a second time", async () => {
expect(await select.open).toBe(false);
expect(await select.menuIsVisible()).toBe(false);
await select.clickInput();
expect(await select.menuIsVisible()).toBe(true);
expect(await select.open).toBe(true);
await select.clickInput();
expect(await select.open).toBe(false);
expect(await select.open).toBe(false);
});
it("should open the menu from a focused but closed input when a search is begun", async () => {
expect(await select.open).toBe(false);
await select.focusOnInput();
expect(await select.open).toBe(false);
expect(await select.menuIsVisible()).toBe(false);
await browser.keys("A");
expect(await select.open).toBe(true);
expect(await select.menuIsVisible()).toBe(true);
});
it("should update the list as the user types", async () => {
await select.focusOnInput();
await browser.keys("Ap");
expect(await select.menuIsVisible()).toBe(true);
const elements = Array.from(await select.listElements());
expect(elements.length).toBe(2);
});
it("set the value when a match is close", async () => {
await select.focusOnInput();
await browser.keys("Ap");
expect(await select.menuIsVisible()).toBe(true);
const elements = Array.from(await select.listElements());
expect(elements.length).toBe(2);
await browser.keys(Key.Tab);
expect(await (await select.input()).getValue()).toBe("Apples");
});
it("should close the menu when the user clicks away", async () => {
document.body.insertAdjacentHTML(
"afterbegin",
'<input id="a-separate-component" type="text" />',
);
const input = await browser.$("#a-separate-component");
await select.clickInput();
expect(await select.open).toBe(true);
await input.click();
expect(await select.open).toBe(false);
it("should shift from the loading indicator to search select view on fetch event completed", async () => {
expect(await wrapper).toBeExisting();
expect(await $(">>>ak-search-select-loading-indicator")).toBeDisplayed();
await browser.execute(() => {
const mock = document.querySelector("ak-mock-search-group");
mock?.dispatchEvent(new Event("resolve"));
});
expect(await $(">>>ak-search-select-loading-indicator")).not.toBeDisplayed();
select = await AkSearchSelectViewDriver.build(await $(">>>ak-search-select-view"));
expect(await select).toBeExisting();
});
afterEach(async () => {
await document.body.querySelector("#a-separate-component")?.remove();
await document.body.querySelector("ak-search-select-view")?.remove();
await document.body.querySelector("ak-mock-search-group")?.remove();
// @ts-expect-error expression of type '"_$litPart$"' is added by Lit
if (document.body["_$litPart$"]) {
// @ts-expect-error expression of type '"_$litPart$"' is added by Lit