From 270e056c34d092a7ad610a78f4bedea23844102b Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 30 Apr 2024 11:32:18 -0700 Subject: [PATCH] Fix add to collection dropdown (#1766) Fixes https://github.com/webrecorder/browsertrix/issues/1638 ### Changes - Fixes collection selection during upload and in workflow settings - Refactors `btrix-collections-add` to `TailwindComponent` with lit `Task` usage ### Manual testing 1. Log in as crawler 2. Go to "Archived Items" 3. Click "Upload WACZ" 4. Select wacz file to upload 5. Search for collection in "Add to Collection". Verify you're able to select a search result 6. Save. Verify collection saves as expected. 7. Go to "Crawling" 8. Create a new workflow 9. Go to "Metadata" 10. Repeat 5-6. --- frontend/src/components/ui/combobox.ts | 18 +- .../features/collections/collections-add.ts | 188 ++++++++++-------- 2 files changed, 126 insertions(+), 80 deletions(-) diff --git a/frontend/src/components/ui/combobox.ts b/frontend/src/components/ui/combobox.ts index fb2b918d..a8de1bfa 100644 --- a/frontend/src/components/ui/combobox.ts +++ b/frontend/src/components/ui/combobox.ts @@ -53,7 +53,8 @@ export class Combobox extends LitElement { @queryAssignedElements({ slot: "menu-item", - selector: "sl-menu-item:not([disabled])", + selector: "sl-menu-item", + flatten: true, }) private readonly menuItems?: SlMenuItem[]; @@ -122,7 +123,12 @@ export class Combobox extends LitElement { private onKeydown(e: KeyboardEvent) { if (this.open && e.key === "ArrowDown") { - if (this.menu && this.menuItems?.length && !this.menu.getCurrentItem()) { + if ( + this.menu && + this.menuItems?.length && + !this.menu.getCurrentItem() && + !this.menuItems[0].disabled + ) { // Focus on first menu item e.preventDefault(); const menuItem = this.menuItems[0]; @@ -159,4 +165,12 @@ export class Combobox extends LitElement { private closeDropdown() { this.dropdown?.classList.add("animateHide"); } + + public show() { + this.open = true; + } + + public hide() { + this.open = false; + } } diff --git a/frontend/src/features/collections/collections-add.ts b/frontend/src/features/collections/collections-add.ts index 7b98ee85..17cd5ce0 100644 --- a/frontend/src/features/collections/collections-add.ts +++ b/frontend/src/features/collections/collections-add.ts @@ -1,10 +1,16 @@ import { localized, msg, str } from "@lit/localize"; +import { Task } from "@lit/task"; import type { SlInput, SlMenuItem } from "@shoelace-style/shoelace"; -import { customElement, property, state } from "lit/decorators.js"; +import { html } from "lit"; +import { customElement, property, query, state } from "lit/decorators.js"; import { when } from "lit/directives/when.js"; import debounce from "lodash/fp/debounce"; import queryString from "query-string"; +import { TailwindElement } from "@/classes/TailwindElement"; +import type { Combobox } from "@/components/ui/combobox"; +import { APIController } from "@/controllers/api"; +import { NotifyController } from "@/controllers/notify"; import type { APIPaginatedList, APIPaginationQuery, @@ -13,7 +19,6 @@ import type { import type { Collection } from "@/types/collection"; import type { UnderlyingFunction } from "@/types/utils"; import type { AuthState } from "@/utils/AuthService"; -import LiteElement, { html } from "@/utils/LiteElement"; const INITIAL_PAGE_SIZE = 10; const MIN_SEARCH_LENGTH = 2; @@ -37,7 +42,7 @@ export type CollectionsChangeEvent = CustomEvent<{ */ @localized() @customElement("btrix-collections-add") -export class CollectionsAdd extends LiteElement { +export class CollectionsAdd extends TailwindElement { @property({ type: Object }) authState!: AuthState; @@ -63,18 +68,35 @@ export class CollectionsAdd extends LiteElement { @state() private collectionIds: string[] = []; - @state() - private searchByValue = ""; + @query("sl-input") + private readonly input?: SlInput | null; - @state() - private searchResults: Collection[] = []; + @query("btrix-combobox") + private readonly combobox?: Combobox | null; + + private readonly api = new APIController(this); + private readonly notify = new NotifyController(this); + + private get searchByValue() { + return this.input ? this.input.value.trim() : ""; + } private get hasSearchStr() { return this.searchByValue.length >= MIN_SEARCH_LENGTH; } - @state() - private searchResultsOpen = false; + private readonly searchResultsTask = new Task(this, { + task: async ([searchByValue, hasSearchStr], { signal }) => { + if (!hasSearchStr) return []; + const data = await this.fetchCollectionsByPrefix(searchByValue, signal); + let searchResults: Collection[] = []; + if (data?.items.length) { + searchResults = this.filterOutSelectedCollections(data.items); + } + return searchResults; + }, + args: () => [this.searchByValue, this.hasSearchStr] as const, + }); connectedCallback() { if (this.initialCollections) { @@ -85,7 +107,6 @@ export class CollectionsAdd extends LiteElement { } disconnectedCallback() { - this.onSearchInput.cancel(); super.disconnectedCallback(); } @@ -121,17 +142,16 @@ export class CollectionsAdd extends LiteElement { private renderSearch() { return html` { - this.searchResultsOpen = false; - this.searchByValue = ""; + this.combobox?.hide(); + if (this.input) this.input.value = ""; }} @sl-select=${async (e: CustomEvent<{ item: SlMenuItem }>) => { - this.searchResultsOpen = false; + this.combobox?.hide(); const item = e.detail.item; const collId = item.dataset["key"]; if (collId && this.collectionIds.indexOf(collId) === -1) { - const coll = this.searchResults.find( + const coll = this.searchResultsTask.value?.find( (collection) => collection.id === collId, ); if (coll) { @@ -152,10 +172,13 @@ export class CollectionsAdd extends LiteElement { size="small" placeholder=${msg("Search by Collection name")} clearable - value=${this.searchByValue} @sl-clear=${() => { - this.searchResultsOpen = false; - this.onSearchInput.cancel(); + this.combobox?.hide(); + }} + @keyup=${() => { + if (this.combobox && !this.combobox.open && this.hasSearchStr) { + this.combobox.show(); + } }} @sl-input=${this.onSearchInput as UnderlyingFunction< typeof this.onSearchInput @@ -169,43 +192,51 @@ export class CollectionsAdd extends LiteElement { } private renderSearchResults() { - if (!this.hasSearchStr) { - return html` - ${msg("Start typing to search Collections.")} - `; - } + return this.searchResultsTask.render({ + pending: () => html` + + + + `, + complete: (searchResults) => { + if (!this.hasSearchStr) { + return html` + + ${msg("Start typing to search Collections.")} + + `; + } - // Filter out stale search results from last debounce invocation - const searchResults = this.searchResults.filter((res) => - new RegExp(`^${this.searchByValue}`, "i").test(res.name), - ); + // Filter out stale search results from last debounce invocation + const results = searchResults.filter((res) => + new RegExp(`^${this.searchByValue}`, "i").test(res.name), + ); - if (!searchResults.length) { - return html` - ${msg("No matching Collections found.")} - `; - } + if (!results.length) { + return html` + + ${msg("No matching Collections found.")} + + `; + } - return html` - ${searchResults.map((item: Collection) => { return html` - -
-
${item.name}
-
- ${msg(str`${item.crawlCount} items`)} -
-
- + ${results.map((item: Collection) => { + return html` + + ${item.name} +
+ ${msg(str`${item.crawlCount} items`)} +
+ + `; + })} `; - })} - `; + }, + }); } private renderCollectionItem(id: string) { @@ -249,19 +280,8 @@ export class CollectionsAdd extends LiteElement { } } - private readonly onSearchInput = debounce(200)(async (e: Event) => { - this.searchByValue = (e.target as SlInput).value.trim(); - - if (!this.searchResultsOpen && this.hasSearchStr) { - this.searchResultsOpen = true; - } - - const data = await this.fetchCollectionsByPrefix(this.searchByValue); - let searchResults: Collection[] = []; - if (data?.items.length) { - searchResults = this.filterOutSelectedCollections(data.items); - } - this.searchResults = searchResults; + private readonly onSearchInput = debounce(400)(() => { + void this.searchResultsTask.run(); }); private filterOutSelectedCollections(results: Collection[]) { @@ -270,21 +290,31 @@ export class CollectionsAdd extends LiteElement { }); } - private async fetchCollectionsByPrefix(namePrefix: string) { + private async fetchCollectionsByPrefix( + namePrefix: string, + signal?: AbortSignal, + ) { try { - const results = await this.getCollections({ - oid: this.orgId, - namePrefix: namePrefix, - sortBy: "name", - pageSize: INITIAL_PAGE_SIZE, - }); + const results = await this.getCollections( + { + oid: this.orgId, + namePrefix: namePrefix, + sortBy: "name", + pageSize: INITIAL_PAGE_SIZE, + }, + signal, + ); return results; - } catch { - this.notify({ - message: msg("Sorry, couldn't retrieve Collections at this time."), - variant: "danger", - icon: "exclamation-octagon", - }); + } catch (e) { + if ((e as Error).name === "AbortError") { + console.debug("Fetch aborted to throttle"); + } else { + this.notify.toast({ + message: msg("Sorry, couldn't retrieve Collections at this time."), + variant: "danger", + icon: "exclamation-octagon", + }); + } } } @@ -295,13 +325,15 @@ export class CollectionsAdd extends LiteElement { }> & APIPaginationQuery & APISortQuery, + signal?: AbortSignal, ) { const query = queryString.stringify(params || {}, { arrayFormat: "comma", }); - const data = await this.apiFetch>( + const data = await this.api.fetch>( `/orgs/${this.orgId}/collections?${query}`, this.authState!, + { signal }, ); return data; @@ -322,7 +354,7 @@ export class CollectionsAdd extends LiteElement { private readonly getCollection = async ( collId: string, ): Promise => { - return this.apiFetch( + return this.api.fetch( `/orgs/${this.orgId}/collections/${collId}`, this.authState!, );