diff --git a/web/src/App.tsx b/web/src/App.tsx index 91970f0..7ceac28 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -8,6 +8,7 @@ import { ThemeToggle } from '@/components/ThemeToggle'; import { ToastList } from '@/components/ToastList'; import type { OpportunityFilters } from '@/lib/filterOpportunities'; import { emptyFilters, filterOpportunities, statusOptions } from '@/lib/filterOpportunities'; +import { formatCount } from '@/lib/format'; import { createLiveUpdates } from '@/primitives/createLiveUpdates'; import { createClassifications, createOpportunities } from '@/primitives/createOpportunities'; import { createOpportunityAnnotations } from '@/primitives/createOpportunityAnnotations'; @@ -35,6 +36,20 @@ export function App(): JSX.Element { setFilters(emptyFilters()); }; + // Only the rows shown are hidden: those a filter leaves out keep their + // marks, and come back as they were when it's cleared. + const hideable = createMemo(() => + rows() + .map((opportunity) => opportunity.uuid) + .filter((uuid) => !annotations.get(uuid).hidden), + ); + const hideAllVisible = (): void => { + const uuids = hideable(); + const count = + uuids.length === 1 ? 'this opportunity' : `these ${formatCount(uuids.length)} opportunities`; + if (window.confirm(`Hide ${count}?`)) annotations.hideAll(uuids); + }; + // A failed load is shown where the list would be, with a retry, and as a // toast, so it's seen wherever the user is looking. createEffect(() => { @@ -64,6 +79,8 @@ export function App(): JSX.Element { onRetryClassifications={classifications.reload} shown={rows().length} total={opportunities.items().length} + hideable={hideable().length} + onHideAll={hideAllVisible} />
diff --git a/web/src/components/FilterPanel.tsx b/web/src/components/FilterPanel.tsx index 502de94..2525cf5 100644 --- a/web/src/components/FilterPanel.tsx +++ b/web/src/components/FilterPanel.tsx @@ -26,6 +26,10 @@ interface Props { onRetryClassifications: () => void; shown: number; total: number; + /** How many of the opportunities shown aren't hidden already. */ + hideable: number; + /** Hides every opportunity shown. */ + onHideAll: () => void; } const toggleChip = @@ -98,6 +102,14 @@ export function FilterPanel(props: Props): JSX.Element { > Clear filters + {/* Animating grid rows from 0fr to 1fr collapses to the content's real height. */} diff --git a/web/src/lib/concurrency.test.ts b/web/src/lib/concurrency.test.ts new file mode 100644 index 0000000..4892f30 --- /dev/null +++ b/web/src/lib/concurrency.test.ts @@ -0,0 +1,77 @@ +import { expect, test } from 'vitest'; + +import { limitConcurrency } from './concurrency'; + +/** A task that runs until it's finished by hand, recording when it starts. */ +function manualTask(started: string[], name: string) { + let finish!: (value: string) => void; + let fail!: (err: Error) => void; + const task = () => { + started.push(name); + return new Promise((resolve, reject) => { + finish = resolve; + fail = reject; + }); + }; + return { task, finish: () => finish(name), fail: () => fail(new Error(name)) }; +} + +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + +test('runs at most max tasks at once, in order', async () => { + const limited = limitConcurrency(2); + const started: string[] = []; + const tasks = ['a', 'b', 'c', 'd'].map((name) => manualTask(started, name)); + const results = tasks.map((t) => limited(t.task)); + + await settle(); + expect(started).toEqual(['a', 'b']); + + tasks[1]?.finish(); + await settle(); + expect(started).toEqual(['a', 'b', 'c']); + + tasks[0]?.finish(); + tasks[2]?.finish(); + await settle(); + tasks[3]?.finish(); + await expect(Promise.all(results)).resolves.toEqual(['a', 'b', 'c', 'd']); +}); + +test('a failed task makes room for the next', async () => { + const limited = limitConcurrency(1); + const started: string[] = []; + const first = manualTask(started, 'a'); + const second = manualTask(started, 'b'); + const firstResult = limited(first.task); + const secondResult = limited(second.task); + + first.fail(); + await expect(firstResult).rejects.toThrow('a'); + await settle(); + expect(started).toEqual(['a', 'b']); + + second.finish(); + await expect(secondResult).resolves.toBe('b'); +}); + +test('a task given a finished task’s place still counts towards max', async () => { + const limited = limitConcurrency(1); + const started: string[] = []; + const first = manualTask(started, 'a'); + const second = manualTask(started, 'b'); + const third = manualTask(started, 'c'); + void limited(first.task); + void limited(second.task); + + first.finish(); + await settle(); + void limited(third.task); + await settle(); + expect(started).toEqual(['a', 'b']); + + second.finish(); + await settle(); + expect(started).toEqual(['a', 'b', 'c']); + third.finish(); +}); diff --git a/web/src/lib/concurrency.ts b/web/src/lib/concurrency.ts new file mode 100644 index 0000000..3fa12e7 --- /dev/null +++ b/web/src/lib/concurrency.ts @@ -0,0 +1,29 @@ +/** Runs a task when there's room for it, and returns its result. */ +export type Limited = (task: () => Promise) => Promise; + +/** + * Runs tasks at most max at a time, in the order they're given; the rest + * wait until one finishes, whether it succeeds or fails. + */ +export function limitConcurrency(max: number): Limited { + let running = 0; + const waiting: (() => void)[] = []; + + // A finished task hands its place to the first waiting one, if there is + // one, so running only falls when nothing is waiting. + const next = (): void => { + const resume = waiting.shift(); + if (resume) resume(); + else running -= 1; + }; + + return async (task: () => Promise): Promise => { + if (running >= max) await new Promise((resolve) => waiting.push(resolve)); + else running += 1; + try { + return await task(); + } finally { + next(); + } + }; +} diff --git a/web/src/lib/format.test.ts b/web/src/lib/format.test.ts index 24c96a4..6939164 100644 --- a/web/src/lib/format.test.ts +++ b/web/src/lib/format.test.ts @@ -1,6 +1,6 @@ import { expect, test } from 'vitest'; -import { formatDate, formatMoney, parseDate } from './format'; +import { formatCount, formatDate, formatMoney, parseDate } from './format'; test.each([ { name: 'null', value: null }, @@ -26,3 +26,8 @@ test('money is shown in whole pounds, and nothing as a dash', () => { expect(formatMoney(0)).toBe('—'); expect(formatMoney(-5)).toBe('—'); }); + +test('formatCount groups thousands', () => { + expect(formatCount(4854)).toBe('4,854'); + expect(formatCount(7)).toBe('7'); +}); diff --git a/web/src/lib/format.ts b/web/src/lib/format.ts index 8e64b40..2653529 100644 --- a/web/src/lib/format.ts +++ b/web/src/lib/format.ts @@ -14,6 +14,8 @@ const moneyFormat = new Intl.NumberFormat('en-GB', { maximumFractionDigits: 0, }); +const countFormat = new Intl.NumberFormat('en-GB'); + /** Treats null and Go's zero time.Time as "no date". */ export function parseDate(value: DateTimeString | null | undefined): Date | null { if (!value || value.startsWith(ZERO_TIME_PREFIX)) return null; @@ -31,3 +33,8 @@ export function formatDate(value: DateTimeString | null | undefined): string { export function formatMoney(value: number): string { return value > 0 ? moneyFormat.format(value) : '—'; } + +/** A count for display, e.g. "4,854". */ +export function formatCount(value: number): string { + return countFormat.format(value); +} diff --git a/web/src/primitives/createOpportunityAnnotations.ts b/web/src/primitives/createOpportunityAnnotations.ts index 135ec36..8e36a78 100644 --- a/web/src/primitives/createOpportunityAnnotations.ts +++ b/web/src/primitives/createOpportunityAnnotations.ts @@ -1,7 +1,8 @@ -import { createEffect } from 'solid-js'; +import { batch, createEffect } from 'solid-js'; import { createStore, produce, reconcile } from 'solid-js/store'; import { api } from '@/api/client'; +import { limitConcurrency } from '@/lib/concurrency'; import { errorText } from '@/lib/errors'; import type { Annotation, MarkName, OpportunityMark } from '@/types/Annotations'; import { annotationFrom, MARK_LABELS, normaliseMarks } from '@/types/Annotations'; @@ -11,6 +12,13 @@ type AnnotationMap = Record; const blank = (uuid: string): Annotation => ({ uuid, marks: [], hidden: false }); +/** + * The most marks saved at once. Hiding every opportunity shown can mean + * hundreds of requests; the rest wait their turn rather than all being sent + * together. + */ +const MAX_SAVES = 6; + /** The marks on every opportunity, and the actions that change them. */ export interface OpportunityAnnotations { /** Reactive: reading inside a tracking scope re-runs when it changes. */ @@ -19,6 +27,12 @@ export interface OpportunityAnnotations { toggleMark: (uuid: string, mark: OpportunityMark) => void; /** Hides or shows an opportunity. The UI updates at once, and reverts if the request fails. */ setHidden: (uuid: string, hidden: boolean) => void; + /** + * Hides each of the opportunities that isn't hidden already. The UI + * updates at once; those whose requests fail are shown again, with one + * message for them all. + */ + hideAll: (uuids: readonly string[]) => void; isSaving: (uuid: string) => boolean; /** * Sets an opportunity's marks to those the backend reports, as an event @@ -39,6 +53,7 @@ export function createOpportunityAnnotations( ): OpportunityAnnotations { const [byUuid, setByUuid] = createStore({}); const [saving, setSaving] = createStore>({}); + const limited = limitConcurrency(MAX_SAVES); // A (re)load of the opportunities brings the marks the backend has. createEffect(() => { @@ -53,20 +68,23 @@ export function createOpportunityAnnotations( if (!byUuid[uuid]) setByUuid(uuid, blank(uuid)); }; - /** Applies a change now, saves it, and undoes it if the save fails. */ + /** + * Applies a change now, saves it, and undoes it if the save fails, giving + * onFailure the error. + */ async function save( uuid: string, change: { apply: () => void; undo: () => void; mark: MarkName; isSet: boolean }, - failure: string, + onFailure: (err: unknown) => void, ): Promise { ensure(uuid); change.apply(); setSaving(uuid, (count) => (count ?? 0) + 1); try { - await api.setMark(uuid, change.mark, change.isSet); + await limited(() => api.setMark(uuid, change.mark, change.isSet)); } catch (err) { change.undo(); - onError(`${failure}: ${errorText(err)}`); + onFailure(err); } finally { setSaving( produce((counts) => { @@ -88,6 +106,7 @@ export function createOpportunityAnnotations( const toggleMark = (uuid: string, mark: OpportunityMark): void => { const isSet = !(byUuid[uuid]?.marks ?? []).includes(mark); const label = MARK_LABELS[mark]; + const failure = isSet ? `"${label}" wasn't added` : `"${label}" wasn't removed`; void save( uuid, { @@ -96,13 +115,17 @@ export function createOpportunityAnnotations( mark, isSet, }, - isSet ? `"${label}" wasn't added` : `"${label}" wasn't removed`, + (err) => onError(`${failure}: ${errorText(err)}`), ); }; - const setHidden = (uuid: string, hidden: boolean): void => { + const saveHidden = ( + uuid: string, + hidden: boolean, + onFailure: (err: unknown) => void, + ): Promise => { const previous = byUuid[uuid]?.hidden ?? false; - void save( + return save( uuid, { apply: () => setByUuid(uuid, 'hidden', hidden), @@ -110,14 +133,39 @@ export function createOpportunityAnnotations( mark: 'hidden', isSet: hidden, }, - hidden ? "The opportunity wasn't hidden" : "The opportunity wasn't shown again", + onFailure, ); }; + const setHidden = (uuid: string, hidden: boolean): void => { + const failure = hidden ? "The opportunity wasn't hidden" : "The opportunity wasn't shown again"; + void saveHidden(uuid, hidden, (err) => onError(`${failure}: ${errorText(err)}`)); + }; + + const hideAll = async (uuids: readonly string[]): Promise => { + const toHide = uuids.filter((uuid) => !(byUuid[uuid]?.hidden ?? false)); + let failed = 0; + let firstError: unknown; + const onFailure = (err: unknown): void => { + failed += 1; + firstError ??= err; + }; + // Batched, so the rows are filtered once for them all rather than once + // for each: the changes are applied before save's first await. + const saves = batch(() => toHide.map((uuid) => saveHidden(uuid, true, onFailure))); + await Promise.all(saves); + if (failed > 0) { + onError( + `${failed} of ${toHide.length} opportunities weren't hidden: ${errorText(firstError)}`, + ); + } + }; + return { get: (uuid) => byUuid[uuid] ?? blank(uuid), toggleMark, setHidden, + hideAll: (uuids) => void hideAll(uuids), isSaving: (uuid) => Boolean(saving[uuid]), applyMarks: (marks) => { if (!saving[marks.uuid]) setByUuid(marks.uuid, annotationFrom(marks));