mirror of
https://github.com/bretzel-app/crumbs.git
synced 2026-10-12 12:06:57 +02:00
fix(search): preserve unsynced hits and avoid empty-state flash
Address review feedback on the search-results preservation change: - reconcileSearchResults now keeps a result absent from the canonical notes store (a server hit not yet loaded locally, e.g. from another device) instead of dropping it, so valid matches no longer vanish mid-session. Locally deleted/left notes are still pruned at their call sites; trashed notes are still dropped. - SearchBar commits searchQuery only once its results arrive, so the view no longer flashes an empty state over the current notes while a request is in flight, and searchQuery stays consistent with searchResults on a failed request. - Expand reconcile unit tests (keep-absent, trashed-drop, mixed order, empty) and add an e2e test asserting notes stay visible during an in-flight search. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
Fred Cabassut
co-authored by
Claude Opus 4.8
parent
b802f17504
commit
460f39298b
@@ -31,9 +31,9 @@
|
||||
searchController = undefined;
|
||||
|
||||
const trimmedQuery = query.trim();
|
||||
searchQuery.set(trimmedQuery);
|
||||
|
||||
if (!trimmedQuery) {
|
||||
searchQuery.set('');
|
||||
searchResults.set([]);
|
||||
return;
|
||||
}
|
||||
@@ -48,7 +48,13 @@
|
||||
if (res.ok) {
|
||||
const results: Note[] = await res.json();
|
||||
if (searchController !== controller || query.trim() !== trimmedQuery) return;
|
||||
// Commit the query alongside its results so the view switches to the
|
||||
// (possibly empty) result set only once it's ready — this avoids
|
||||
// flashing an empty state over the current notes while the request is
|
||||
// in flight, and keeps searchQuery consistent with searchResults on
|
||||
// a failed request (neither updates).
|
||||
searchResults.set(results);
|
||||
searchQuery.set(trimmedQuery);
|
||||
}
|
||||
} catch (error) {
|
||||
if (!(error instanceof DOMException && error.name === 'AbortError')) {
|
||||
|
||||
@@ -99,15 +99,47 @@ describe('mergeNotesByVersion', () => {
|
||||
});
|
||||
|
||||
describe('reconcileSearchResults', () => {
|
||||
it('updates existing results and removes notes absent from canonical state', () => {
|
||||
const updated = makeNote({ id: 'n1', title: 'Updated', version: 2 });
|
||||
const removed = makeNote({ id: 'n2', title: 'Removed' });
|
||||
it('replaces a stale result with the canonical note', () => {
|
||||
const canonical = makeNote({ id: 'n1', title: 'Updated', version: 2 });
|
||||
|
||||
const result = reconcileSearchResults([updated], [
|
||||
makeNote({ id: 'n1', title: 'Stale', version: 1 }),
|
||||
removed
|
||||
]);
|
||||
const result = reconcileSearchResults(
|
||||
[canonical],
|
||||
[makeNote({ id: 'n1', title: 'Stale', version: 1 })]
|
||||
);
|
||||
|
||||
expect(result).toEqual([updated]);
|
||||
expect(result).toEqual([canonical]);
|
||||
});
|
||||
|
||||
it('keeps a result absent from the canonical store (hit not yet loaded locally)', () => {
|
||||
const serverHit = makeNote({ id: 'n2', title: 'From another device' });
|
||||
|
||||
const result = reconcileSearchResults([makeNote({ id: 'n1' })], [serverHit]);
|
||||
|
||||
expect(result).toEqual([serverHit]);
|
||||
});
|
||||
|
||||
it('drops a result that is trashed in the canonical store', () => {
|
||||
const trashed = makeNote({ id: 'n1', trashed: true });
|
||||
|
||||
const result = reconcileSearchResults([trashed], [makeNote({ id: 'n1', trashed: false })]);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('preserves result order while reconciling a mix of cases', () => {
|
||||
const canonical = makeNote({ id: 'n1', title: 'Canonical', version: 2 });
|
||||
const trashed = makeNote({ id: 'n2', trashed: true });
|
||||
const absent = makeNote({ id: 'n3', title: 'Absent' });
|
||||
|
||||
const result = reconcileSearchResults(
|
||||
[canonical, trashed],
|
||||
[makeNote({ id: 'n1', title: 'Stale', version: 1 }), trashed, absent]
|
||||
);
|
||||
|
||||
expect(result).toEqual([canonical, absent]);
|
||||
});
|
||||
|
||||
it('returns an empty array when there are no results', () => {
|
||||
expect(reconcileSearchResults([makeNote({ id: 'n1' })], [])).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
+15
-3
@@ -11,15 +11,27 @@ export const notesLoaded = writable(false);
|
||||
export const searchQuery = writable<string>('');
|
||||
export const searchResults = writable<Note[]>([]);
|
||||
|
||||
/**
|
||||
* Reconcile search results against the canonical notes store so local edits and
|
||||
* trashing propagate into the active result set without re-querying the server.
|
||||
*
|
||||
* A result absent from the canonical store is kept as-is — it's a server hit not
|
||||
* yet loaded locally (e.g. created on another device), and dropping it would make
|
||||
* valid matches disappear mid-session. Notes removed locally (deleted or left) are
|
||||
* pruned explicitly at their call sites; trashed notes are dropped here.
|
||||
*/
|
||||
export function reconcileSearchResults(canonicalNotes: Note[], results: Note[]): Note[] {
|
||||
const notesMap = new Map(canonicalNotes.map((note) => [note.id, note]));
|
||||
return results.flatMap((result) => {
|
||||
const updated = notesMap.get(result.id);
|
||||
return updated && !updated.trashed ? [updated] : [];
|
||||
const canonical = notesMap.get(result.id);
|
||||
if (!canonical) return [result];
|
||||
return canonical.trashed ? [] : [canonical];
|
||||
});
|
||||
}
|
||||
|
||||
// Keep searchResults in sync with note updates/deletions in notes store
|
||||
// Keep searchResults in sync with edits/trashing in the notes store. These search
|
||||
// stores (like the others in this module) are client-only state; the subscription
|
||||
// lives for the app's lifetime and is intentionally never unsubscribed.
|
||||
notes.subscribe(($notes) => {
|
||||
searchResults.update(($results) => {
|
||||
if ($results.length === 0) return $results;
|
||||
|
||||
@@ -83,4 +83,32 @@ test.describe('Search', () => {
|
||||
await expect(page.getByText('Race Fast 3812925')).toBeVisible();
|
||||
await expect(page.getByText('Race Slow 3812925')).not.toBeVisible();
|
||||
});
|
||||
|
||||
test('Scenario: Current notes stay visible while a search request is in flight', async ({ authenticatedPage: page }) => {
|
||||
// Given two notes exist
|
||||
await createNote(page, 'Flash Alpha 771903');
|
||||
await createNote(page, 'Flash Beta 771903');
|
||||
|
||||
// And the search request is held open before it can respond
|
||||
let releaseSearch: () => void = () => {};
|
||||
const searchGate = new Promise<void>((resolve) => {
|
||||
releaseSearch = resolve;
|
||||
});
|
||||
await page.route('**/api/search?*', async (route) => {
|
||||
await searchGate;
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
// When the user starts a search
|
||||
await page.getByTestId('search-input').fill('Flash Alpha 771903');
|
||||
|
||||
// Then the current notes stay on screen instead of flashing an empty state
|
||||
await expect(page.getByText('Flash Beta 771903')).toBeVisible();
|
||||
await expect(page.getByText('Flash Alpha 771903')).toBeVisible();
|
||||
|
||||
// And once the results arrive, only the match remains
|
||||
releaseSearch();
|
||||
await expect(page.getByText('Flash Alpha 771903')).toBeVisible();
|
||||
await expect(page.getByText('Flash Beta 771903')).not.toBeVisible();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user