fix(ui): a picker whose list failed says so, and takes no pick while one is being written
- Combobox gains `error` (said in the list, not "No matches") and a soft `disabled` (keeps the focus, announced, opens and changes nothing). - UserPicker passes both: the share dialog's recipient picker is off while a share is pending, and the share and quota pickers say when the user list failed to load. - ChannelPicker and the Plex playlist/collection pickers say a failed load too. - The channel menu's Tags row says loading / failed instead of "no tags yet".
This commit is contained in:
@@ -40,6 +40,7 @@ export default function ChannelPicker({
|
|||||||
selected={filters.channelIds}
|
selected={filters.channelIds}
|
||||||
onChange={(channelIds) => setFilters({ ...filters, channelIds })}
|
onChange={(channelIds) => setFilters({ ...filters, channelIds })}
|
||||||
loading={channelsQuery.isLoading}
|
loading={channelsQuery.isLoading}
|
||||||
|
error={channelsQuery.isError && !channelsQuery.data ? t("common.loadError") : undefined}
|
||||||
emptyText={t("sidebar.noChannelMatch")}
|
emptyText={t("sidebar.noChannelMatch")}
|
||||||
selectedLabels={selectedLabels}
|
selectedLabels={selectedLabels}
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -329,6 +329,7 @@ export default function Channels() {
|
|||||||
onView,
|
onView,
|
||||||
onOpenWizard,
|
onOpenWizard,
|
||||||
tags: userTags,
|
tags: userTags,
|
||||||
|
tagsState: tagsQuery.isPending ? "loading" : tagsQuery.isError ? "error" : "ready",
|
||||||
onToggleTag: (c, tagId) => toggleTag(c.id, tagId, c.tag_ids.includes(tagId)),
|
onToggleTag: (c, tagId) => toggleTag(c.id, tagId, c.tag_ids.includes(tagId)),
|
||||||
onPickPriority: pickPriority,
|
onPickPriority: pickPriority,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -371,6 +371,8 @@ function PlaylistPicker() {
|
|||||||
query={query}
|
query={query}
|
||||||
onQueryChange={setQuery}
|
onQueryChange={setQuery}
|
||||||
placeholder={t("plex.playlist.pickerPlaceholder")}
|
placeholder={t("plex.playlist.pickerPlaceholder")}
|
||||||
|
loading={playlistsQ.isLoading}
|
||||||
|
error={playlistsQ.isError && !playlistsQ.data ? t("common.loadError") : undefined}
|
||||||
emptyText={t("plex.playlist.none")}
|
emptyText={t("plex.playlist.none")}
|
||||||
renderOption={(o) =>
|
renderOption={(o) =>
|
||||||
o.value === CREATE_PLAYLIST ? (
|
o.value === CREATE_PLAYLIST ? (
|
||||||
@@ -454,6 +456,7 @@ function CollectionPicker({
|
|||||||
onQueryChange={setSearch}
|
onQueryChange={setSearch}
|
||||||
onOpenChange={setOpen}
|
onOpenChange={setOpen}
|
||||||
loading={q.isFetching}
|
loading={q.isFetching}
|
||||||
|
error={q.isError ? t("common.loadError") : undefined}
|
||||||
placeholder={t("plex.filter.collectionSearch")}
|
placeholder={t("plex.filter.collectionSearch")}
|
||||||
emptyText={t("plex.filter.collectionEmpty")}
|
emptyText={t("plex.filter.collectionEmpty")}
|
||||||
onChange={(next) => {
|
onChange={(next) => {
|
||||||
|
|||||||
@@ -46,6 +46,8 @@ function UserShare({ job }: { job: DownloadJob }) {
|
|||||||
<UserPicker
|
<UserPicker
|
||||||
users={list}
|
users={list}
|
||||||
loading={recipients.isLoading}
|
loading={recipients.isLoading}
|
||||||
|
error={recipients.isError}
|
||||||
|
disabled={share.isPending}
|
||||||
onPick={(u) => {
|
onPick={(u) => {
|
||||||
if (!share.isPending) share.mutate(u.email);
|
if (!share.isPending) share.mutate(u.email);
|
||||||
}}
|
}}
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
import { useTranslation } from "react-i18next";
|
||||||
import Combobox from "./ui/Combobox";
|
import Combobox from "./ui/Combobox";
|
||||||
|
|
||||||
// Pick one registered user by name or email: the download quota's "any user" picker and the share
|
// Pick one registered user by name or email: the download quota's "any user" picker and the share
|
||||||
@@ -17,13 +18,20 @@ export default function UserPicker<U extends PickableUser>({
|
|||||||
label,
|
label,
|
||||||
placeholder,
|
placeholder,
|
||||||
loading,
|
loading,
|
||||||
|
error,
|
||||||
|
disabled,
|
||||||
}: {
|
}: {
|
||||||
users: readonly U[];
|
users: readonly U[];
|
||||||
onPick: (u: U) => void;
|
onPick: (u: U) => void;
|
||||||
label: string;
|
label: string;
|
||||||
placeholder: string;
|
placeholder: string;
|
||||||
loading?: boolean;
|
loading?: boolean;
|
||||||
|
/** The user list failed to load: the picker says so instead of "No matches". */
|
||||||
|
error?: boolean;
|
||||||
|
/** The last pick is still being acted on: no second one meanwhile. */
|
||||||
|
disabled?: boolean;
|
||||||
}) {
|
}) {
|
||||||
|
const { t } = useTranslation();
|
||||||
const byId = new Map(users.map((u) => [String(u.id), u]));
|
const byId = new Map(users.map((u) => [String(u.id), u]));
|
||||||
return (
|
return (
|
||||||
<Combobox
|
<Combobox
|
||||||
@@ -31,6 +39,8 @@ export default function UserPicker<U extends PickableUser>({
|
|||||||
placeholder={placeholder}
|
placeholder={placeholder}
|
||||||
multiple={false}
|
multiple={false}
|
||||||
loading={loading}
|
loading={loading}
|
||||||
|
error={error ? t("common.loadError") : undefined}
|
||||||
|
disabled={disabled}
|
||||||
selected={[]}
|
selected={[]}
|
||||||
// The label carries both, so the client filter matches a name OR an email.
|
// The label carries both, so the client filter matches a name OR an email.
|
||||||
options={users.map((u) => ({
|
options={users.map((u) => ({
|
||||||
|
|||||||
@@ -149,6 +149,7 @@ function QuotaUserPicker({ onPick }: { onPick: (u: { id: number; email: string }
|
|||||||
<UserPicker
|
<UserPicker
|
||||||
users={list}
|
users={list}
|
||||||
loading={users.isLoading}
|
loading={users.isLoading}
|
||||||
|
error={users.isError}
|
||||||
onPick={(u) => onPick({ id: u.id, email: u.email })}
|
onPick={(u) => onPick({ id: u.id, email: u.email })}
|
||||||
label={t("downloads.admin.quotaPickPlaceholder")}
|
label={t("downloads.admin.quotaPickPlaceholder")}
|
||||||
placeholder={t("downloads.admin.quotaPickPlaceholder")}
|
placeholder={t("downloads.admin.quotaPickPlaceholder")}
|
||||||
|
|||||||
@@ -276,6 +276,33 @@ describe("Combobox", () => {
|
|||||||
// PageUp/PageDown (E11, owner request): the sidebar's channel filter and the Plex pickers run to
|
// PageUp/PageDown (E11, owner request): the sidebar's channel filter and the Plex pickers run to
|
||||||
// hundreds of rows, and floating-ui's list navigation has only the arrows. jsdom has no layout, so a
|
// hundreds of rows, and floating-ui's list navigation has only the arrows. jsdom has no layout, so a
|
||||||
// page is the fixed fallback of 10 rows; the measured page is pinned in lib/listPaging.
|
// page is the fixed fallback of 10 rows; the measured page is pinned in lib/listPaging.
|
||||||
|
// E28 review J1 + J2: a failed list said "No matches" (an error dressed as an empty result), and a
|
||||||
|
// picker whose pick was still being written took another pick.
|
||||||
|
describe("Combobox · honest states", () => {
|
||||||
|
it("says the list failed to load, not that nothing matched", () => {
|
||||||
|
wrap(
|
||||||
|
<Combobox label="User" options={[]} selected={[]} onChange={vi.fn()} error="Load failed" />,
|
||||||
|
);
|
||||||
|
fireEvent.focus(screen.getByRole("combobox"));
|
||||||
|
expect(screen.getByText("Load failed")).toBeTruthy();
|
||||||
|
expect(screen.queryByText("common.noResults")).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("disabled: keeps the focus, opens nothing and picks nothing", () => {
|
||||||
|
const onChange = vi.fn();
|
||||||
|
wrap(<Combobox label="Genre" options={OPTIONS} selected={[]} onChange={onChange} disabled />);
|
||||||
|
const field = screen.getByRole("combobox");
|
||||||
|
field.focus();
|
||||||
|
fireEvent.focus(field);
|
||||||
|
expect(screen.queryByRole("option", { hidden: true })).toBeNull();
|
||||||
|
fireEvent.keyDown(field, { key: "ArrowDown" });
|
||||||
|
fireEvent.keyDown(field, { key: "Enter" });
|
||||||
|
expect(onChange).not.toHaveBeenCalled();
|
||||||
|
expect(field.getAttribute("aria-disabled")).toBe("true");
|
||||||
|
expect(document.activeElement).toBe(field); // not natively disabled: the focus stays put
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("Combobox · paging", () => {
|
describe("Combobox · paging", () => {
|
||||||
const rows = (disabled: number[] = []) =>
|
const rows = (disabled: number[] = []) =>
|
||||||
Array.from({ length: 25 }, (_, i) => ({ value: `Row ${i}`, disabled: disabled.includes(i) }));
|
Array.from({ length: 25 }, (_, i) => ({ value: `Row ${i}`, disabled: disabled.includes(i) }));
|
||||||
|
|||||||
@@ -52,6 +52,12 @@ export interface ComboboxProps {
|
|||||||
* the list server-side (then `options` arrives pre-filtered). */
|
* the list server-side (then `options` arrives pre-filtered). */
|
||||||
clientFilter?: boolean;
|
clientFilter?: boolean;
|
||||||
loading?: boolean;
|
loading?: boolean;
|
||||||
|
/** The options failed to load: said in the list instead of "No matches" (an error is not an empty
|
||||||
|
* result — E28 review J1). */
|
||||||
|
error?: string;
|
||||||
|
/** Takes no pick (e.g. the last one is still being written, J2). Soft, like `FocusSafeButton`: the
|
||||||
|
* field keeps the focus (a natively disabled one drops it to <body>), announced as unavailable. */
|
||||||
|
disabled?: boolean;
|
||||||
/** Shown when there are no options to list. */
|
/** Shown when there are no options to list. */
|
||||||
emptyText?: string;
|
emptyText?: string;
|
||||||
/** Cap on rendered rows for a huge vocabulary; the rest hide behind the search. Default 50. */
|
/** Cap on rendered rows for a huge vocabulary; the rest hide behind the search. Default 50. */
|
||||||
@@ -75,6 +81,8 @@ export default function Combobox({
|
|||||||
onQueryChange,
|
onQueryChange,
|
||||||
clientFilter = true,
|
clientFilter = true,
|
||||||
loading = false,
|
loading = false,
|
||||||
|
error,
|
||||||
|
disabled = false,
|
||||||
emptyText,
|
emptyText,
|
||||||
maxVisible = 50,
|
maxVisible = 50,
|
||||||
renderOption,
|
renderOption,
|
||||||
@@ -102,6 +110,7 @@ export default function Combobox({
|
|||||||
const labelFor = (v: string) => selectedLabels?.[v] ?? labelCache.current[v] ?? v;
|
const labelFor = (v: string) => selectedLabels?.[v] ?? labelCache.current[v] ?? v;
|
||||||
|
|
||||||
const setOpenAnd = (next: boolean) => {
|
const setOpenAnd = (next: boolean) => {
|
||||||
|
if (next && disabled) return;
|
||||||
setOpen(next);
|
setOpen(next);
|
||||||
onOpenChange?.(next);
|
onOpenChange?.(next);
|
||||||
// Closing discards the typed query: the list is for "find and pick", and a leftover filter would
|
// Closing discards the typed query: the list is for "find and pick", and a leftover filter would
|
||||||
@@ -145,6 +154,7 @@ export default function Combobox({
|
|||||||
const hiddenCount = filtered.length - visible.length;
|
const hiddenCount = filtered.length - visible.length;
|
||||||
|
|
||||||
const commit = (value: string) => {
|
const commit = (value: string) => {
|
||||||
|
if (disabled) return;
|
||||||
if (multiple) {
|
if (multiple) {
|
||||||
onChange(
|
onChange(
|
||||||
selected.includes(value) ? selected.filter((x) => x !== value) : [...selected, value],
|
selected.includes(value) ? selected.filter((x) => x !== value) : [...selected, value],
|
||||||
@@ -183,13 +193,14 @@ export default function Combobox({
|
|||||||
className={clsx(
|
className={clsx(
|
||||||
fieldClass({ size: "free" }),
|
fieldClass({ size: "free" }),
|
||||||
"flex min-h-8 w-full cursor-text flex-wrap items-center gap-1 px-1.5 py-0.5 text-sm",
|
"flex min-h-8 w-full cursor-text flex-wrap items-center gap-1 px-1.5 py-0.5 text-sm",
|
||||||
|
disabled && "cursor-not-allowed opacity-50",
|
||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
{selected.map((v) => (
|
{selected.map((v) => (
|
||||||
<DismissibleChip
|
<DismissibleChip
|
||||||
key={v}
|
key={v}
|
||||||
className="max-w-40"
|
className="max-w-40"
|
||||||
onDismiss={() => onChange(selected.filter((x) => x !== v))}
|
onDismiss={() => !disabled && onChange(selected.filter((x) => x !== v))}
|
||||||
dismissLabel={t("common.removeNamed", { name: labelFor(v) })}
|
dismissLabel={t("common.removeNamed", { name: labelFor(v) })}
|
||||||
>
|
>
|
||||||
{labelFor(v)}
|
{labelFor(v)}
|
||||||
@@ -209,13 +220,20 @@ export default function Combobox({
|
|||||||
if (e.key === "Enter" && open && idx != null && visible[idx]) {
|
if (e.key === "Enter" && open && idx != null && visible[idx]) {
|
||||||
e.preventDefault(); // select the highlighted option, don't submit a surrounding form
|
e.preventDefault(); // select the highlighted option, don't submit a surrounding form
|
||||||
if (!visible[idx].disabled) commit(visible[idx].value);
|
if (!visible[idx].disabled) commit(visible[idx].value);
|
||||||
} else if (e.key === "Backspace" && query === "" && selected.length > 0) {
|
} else if (
|
||||||
|
e.key === "Backspace" &&
|
||||||
|
!disabled &&
|
||||||
|
query === "" &&
|
||||||
|
selected.length > 0
|
||||||
|
) {
|
||||||
// Backspace on an empty field removes the last chip (react-select behaviour).
|
// Backspace on an empty field removes the last chip (react-select behaviour).
|
||||||
onChange(selected.slice(0, -1));
|
onChange(selected.slice(0, -1));
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
})}
|
})}
|
||||||
value={query}
|
value={query}
|
||||||
|
readOnly={disabled}
|
||||||
|
aria-disabled={disabled || undefined}
|
||||||
aria-label={label}
|
aria-label={label}
|
||||||
placeholder={
|
placeholder={
|
||||||
selected.length === 0
|
selected.length === 0
|
||||||
@@ -237,7 +255,11 @@ export default function Combobox({
|
|||||||
>
|
>
|
||||||
{/* The scroll box owns the height cap AND the scroll-fade mask (ref+style on the same element). */}
|
{/* The scroll box owns the height cap AND the scroll-fade mask (ref+style on the same element). */}
|
||||||
<div ref={fade.ref} style={fade.style} className="max-h-64 overflow-y-auto no-scrollbar">
|
<div ref={fade.ref} style={fade.style} className="max-h-64 overflow-y-auto no-scrollbar">
|
||||||
{loading && filtered.length === 0 ? (
|
{error && filtered.length === 0 ? (
|
||||||
|
<p role="alert" className="px-2 py-1.5 text-xs text-danger">
|
||||||
|
{error}
|
||||||
|
</p>
|
||||||
|
) : loading && filtered.length === 0 ? (
|
||||||
<p className="px-2 py-1.5 text-xs text-muted">{t("common.loading")}</p>
|
<p className="px-2 py-1.5 text-xs text-muted">{t("common.loading")}</p>
|
||||||
) : filtered.length === 0 ? (
|
) : filtered.length === 0 ? (
|
||||||
<p className="px-2 py-1.5 text-xs text-muted">
|
<p className="px-2 py-1.5 text-xs text-muted">
|
||||||
|
|||||||
@@ -40,6 +40,7 @@ afterEach(() => {
|
|||||||
h.role = "user";
|
h.role = "user";
|
||||||
h.canWrite = true;
|
h.canWrite = true;
|
||||||
tags = [];
|
tags = [];
|
||||||
|
tagsState = "ready";
|
||||||
onToggleTag.mockReset();
|
onToggleTag.mockReset();
|
||||||
onPickPriority.mockReset();
|
onPickPriority.mockReset();
|
||||||
});
|
});
|
||||||
@@ -70,6 +71,7 @@ const onView = vi.fn();
|
|||||||
const onToggleTag = vi.fn();
|
const onToggleTag = vi.fn();
|
||||||
const onPickPriority = vi.fn();
|
const onPickPriority = vi.fn();
|
||||||
let tags: Tag[] = [];
|
let tags: Tag[] = [];
|
||||||
|
let tagsState: "loading" | "error" | "ready" = "ready";
|
||||||
const tag = (id: number, name: string) => ({ id, name }) as Tag;
|
const tag = (id: number, name: string) => ({ id, name }) as Tag;
|
||||||
function mount() {
|
function mount() {
|
||||||
const qc = new QueryClient({ defaultOptions: { mutations: { retry: false } } });
|
const qc = new QueryClient({ defaultOptions: { mutations: { retry: false } } });
|
||||||
@@ -77,7 +79,15 @@ function mount() {
|
|||||||
<QueryClientProvider client={qc}>{children}</QueryClientProvider>
|
<QueryClientProvider client={qc}>{children}</QueryClientProvider>
|
||||||
);
|
);
|
||||||
return renderHook(
|
return renderHook(
|
||||||
() => useChannelActions({ onView, onOpenWizard: () => {}, tags, onToggleTag, onPickPriority }),
|
() =>
|
||||||
|
useChannelActions({
|
||||||
|
onView,
|
||||||
|
onOpenWizard: () => {},
|
||||||
|
tags,
|
||||||
|
tagsState,
|
||||||
|
onToggleTag,
|
||||||
|
onPickPriority,
|
||||||
|
}),
|
||||||
{ wrapper },
|
{ wrapper },
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -220,6 +230,14 @@ describe("useChannelActions — the Tags submenu", () => {
|
|||||||
expect(onToggleTag).toHaveBeenCalledWith(expect.objectContaining({ id: "UC1" }), 2);
|
expect(onToggleTag).toHaveBeenCalledWith(expect.objectContaining({ id: "UC1" }), 2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// E28 review J3: a failed /tags read said "no tags yet" — an error dressed as an empty list.
|
||||||
|
it("says the tags are loading, or failed to load, instead of 'no tags yet'", () => {
|
||||||
|
tagsState = "loading";
|
||||||
|
expect(row(ch(), "tags")).toMatchObject({ disabled: true, note: "common.loading" });
|
||||||
|
tagsState = "error";
|
||||||
|
expect(row(ch(), "tags")).toMatchObject({ disabled: true, note: "common.loadError" });
|
||||||
|
});
|
||||||
|
|
||||||
it("stays in place, off, saying where tags are made, with no tag yet", () => {
|
it("stays in place, off, saying where tags are made, with no tag yet", () => {
|
||||||
expect(row(ch(), "tags")).toMatchObject({ disabled: true, note: "channels.row.noTagsYet" });
|
expect(row(ch(), "tags")).toMatchObject({ disabled: true, note: "channels.row.noTagsYet" });
|
||||||
expect(row(ch(), "tags").items).toBeUndefined();
|
expect(row(ch(), "tags").items).toBeUndefined();
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ export function useChannelActions({
|
|||||||
onView,
|
onView,
|
||||||
onOpenWizard,
|
onOpenWizard,
|
||||||
tags,
|
tags,
|
||||||
|
tagsState = "ready",
|
||||||
onToggleTag,
|
onToggleTag,
|
||||||
onPickPriority,
|
onPickPriority,
|
||||||
}: {
|
}: {
|
||||||
@@ -30,6 +31,9 @@ export function useChannelActions({
|
|||||||
onOpenWizard: () => void;
|
onOpenWizard: () => void;
|
||||||
/** The reader's own tags (no system tags) — the Tags submenu lists them A–Z. */
|
/** The reader's own tags (no system tags) — the Tags submenu lists them A–Z. */
|
||||||
tags: readonly Tag[];
|
tags: readonly Tag[];
|
||||||
|
/** Whether `tags` is the answer yet: loading or failed is said as such, never as "no tags yet"
|
||||||
|
* (E28 review J3). */
|
||||||
|
tagsState?: "loading" | "error" | "ready";
|
||||||
onToggleTag: (c: ManagedChannel, tagId: number) => void;
|
onToggleTag: (c: ManagedChannel, tagId: number) => void;
|
||||||
/** A priority step picked: the caller re-sorts its list and follows the row. */
|
/** A priority step picked: the caller re-sorts its list and follows the row. */
|
||||||
onPickPriority: (c: ManagedChannel, priority: number) => void;
|
onPickPriority: (c: ManagedChannel, priority: number) => void;
|
||||||
@@ -149,8 +153,15 @@ export function useChannelActions({
|
|||||||
label: t("channels.tags.label"),
|
label: t("channels.tags.label"),
|
||||||
icon: <Tags className="w-4 h-4" />,
|
icon: <Tags className="w-4 h-4" />,
|
||||||
};
|
};
|
||||||
if (tags.length === 0)
|
if (tags.length === 0) {
|
||||||
return { ...base, disabled: true, note: t("channels.row.noTagsYet"), onSelect: () => {} };
|
const note =
|
||||||
|
tagsState === "loading"
|
||||||
|
? t("common.loading")
|
||||||
|
: tagsState === "error"
|
||||||
|
? t("common.loadError")
|
||||||
|
: t("channels.row.noTagsYet");
|
||||||
|
return { ...base, disabled: true, note, onSelect: () => {} };
|
||||||
|
}
|
||||||
const on = new Set(c.tag_ids);
|
const on = new Set(c.tag_ids);
|
||||||
return {
|
return {
|
||||||
...base,
|
...base,
|
||||||
|
|||||||
Reference in New Issue
Block a user