Skip to content

Commit

Permalink
Fix add to collection dropdown (#1766)
Browse files Browse the repository at this point in the history 
Fixes #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.
  • Loading branch information
@SuaYoo
SuaYoo authored Apr 30, 2024
1 parent fe65ccf commit 270e056
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 82 deletions.
18 changes: 16 additions & 2 deletions frontend/src/components/ui/combobox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[];

Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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;
}
}
192 changes: 112 additions & 80 deletions frontend/src/features/collections/collections-add.ts
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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;
Expand All @@ -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;

Expand All @@ -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) {
Expand All @@ -85,7 +107,6 @@ export class CollectionsAdd extends LiteElement {
}

disconnectedCallback() {
this.onSearchInput.cancel();
super.disconnectedCallback();
}

Expand Down Expand Up @@ -121,17 +142,16 @@ export class CollectionsAdd extends LiteElement {
private renderSearch() {
return html`
<btrix-combobox
?open=${this.searchResultsOpen}
@request-close=${() => {
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) {
Expand All @@ -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
Expand All @@ -169,43 +192,51 @@ export class CollectionsAdd extends LiteElement {
}

private renderSearchResults() {
if (!this.hasSearchStr) {
return html`
<sl-menu-item slot="menu-item" disabled
>${msg("Start typing to search Collections.")}</sl-menu-item
>
`;
}

// Filter out stale search results from last debounce invocation
const searchResults = this.searchResults.filter((res) =>
new RegExp(`^${this.searchByValue}`, "i").test(res.name),
);
return this.searchResultsTask.render({
pending: () => html`
<sl-menu-item slot="menu-item" disabled>
<sl-spinner></sl-spinner>
</sl-menu-item>
`,
complete: (searchResults) => {
if (!this.hasSearchStr) {
return html`
<sl-menu-item slot="menu-item" disabled>
${msg("Start typing to search Collections.")}
</sl-menu-item>
`;
}

// Filter out stale search results from last debounce invocation
const results = searchResults.filter((res) =>
new RegExp(`^${this.searchByValue}`, "i").test(res.name),
);

if (!results.length) {
return html`
<sl-menu-item slot="menu-item" disabled>
${msg("No matching Collections found.")}
</sl-menu-item>
`;
}

if (!searchResults.length) {
return html`
<sl-menu-item slot="menu-item" disabled
>${msg("No matching Collections found.")}</sl-menu-item
>
`;
}

return html`
${searchResults.map((item: Collection) => {
return html`
<sl-menu-item class="w-full" slot="menu-item" data-key=${item.id}>
<div class="flex w-full items-center gap-2">
<div class="grow justify-self-stretch truncate">${item.name}</div>
<div
class="font-monostyle flex-auto text-right text-xs text-neutral-500"
>
${msg(str`${item.crawlCount} items`)}
</div>
</div>
</sl-menu-item>
${results.map((item: Collection) => {
return html`
<sl-menu-item slot="menu-item" data-key=${item.id}>
${item.name}
<div
slot="suffix"
class="font-monostyle flex-auto text-right text-xs text-neutral-500"
>
${msg(str`${item.crawlCount} items`)}
</div>
</sl-menu-item>
`;
})}
`;
})}
`;
},
});
}

private renderCollectionItem(id: string) {
Expand Down Expand Up @@ -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[]) {
Expand All @@ -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",
});
}
}
}

Expand All @@ -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<APIPaginatedList<Collection>>(
const data = await this.api.fetch<APIPaginatedList<Collection>>(
`/orgs/${this.orgId}/collections?${query}`,
this.authState!,
{ signal },
);

return data;
Expand All @@ -322,7 +354,7 @@ export class CollectionsAdd extends LiteElement {
private readonly getCollection = async (
collId: string,
): Promise<Collection | undefined> => {
return this.apiFetch(
return this.api.fetch(
`/orgs/${this.orgId}/collections/${collId}`,
this.authState!,
);
Expand Down

0 comments on commit 270e056

Please sign in to comment.