From b8244da2e59f77d956d35e6e5daed2f01949ba82 Mon Sep 17 00:00:00 2001 From: Craig Williams Date: Mon, 19 Oct 2026 09:47:49 +0100 Subject: [PATCH] Added a "Hide all visible" button It hides the opportunities the filters show that aren't hidden already, after asking for confirmation with their count. Those the filters leave out keep their marks, so clearing the filters shows them as before. The backend has no bulk route, so each opportunity is hidden with its own request. The rows go at once; every mark change is now saved at most six at a time (lib/concurrency.ts), so hundreds of requests aren't sent together. Any that fail are shown again, with one toast for them all. --- web/src/App.tsx | 17 ++++ web/src/components/FilterPanel.tsx | 12 +++ web/src/lib/concurrency.test.ts | 77 +++++++++++++++++++ web/src/lib/concurrency.ts | 29 +++++++ web/src/lib/format.test.ts | 7 +- web/src/lib/format.ts | 7 ++ .../createOpportunityAnnotations.ts | 66 +++++++++++++--- 7 files changed, 205 insertions(+), 10 deletions(-) create mode 100644 web/src/lib/concurrency.test.ts create mode 100644 web/src/lib/concurrency.ts 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));