From 2267474cd5799fcb8c6f0c3f66e8b299941cc313 Mon Sep 17 00:00:00 2001 From: Frederic Cabassut Date: Sat, 21 Mar 2026 23:45:12 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94=20a11y?= =?UTF-8?q?,=20checked=20detection,=20test=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused flushSync import - Fix removeItem single-item branch: emit change and trigger reactivity - Remove misleading role="button" + tabindex from drag handle - Use anchored regex for checked detection instead of line.includes('[x]') in NoteCard and public share page (prevents false positive on item text) - Fix NoteCard preview E2E test: scope assertion to specific card, use unique title to avoid collision - Restore arrow key navigation E2E test Co-Authored-By: Claude Opus 4.6 (1M context) --- src/lib/components/Checklist.svelte | 6 ++--- src/lib/components/NoteCard.svelte | 2 +- src/routes/(share)/s/[token]/+page.svelte | 2 +- tests/e2e/checklist.spec.ts | 32 ++++++++++++++++++++--- 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/lib/components/Checklist.svelte b/src/lib/components/Checklist.svelte index 998edb0..5122b28 100644 --- a/src/lib/components/Checklist.svelte +++ b/src/lib/components/Checklist.svelte @@ -3,7 +3,7 @@ import Plus from 'lucide-svelte/icons/plus'; import GripVertical from 'lucide-svelte/icons/grip-vertical'; import ChevronRight from 'lucide-svelte/icons/chevron-right'; - import { onDestroy, flushSync } from 'svelte'; + import { onDestroy } from 'svelte'; import { dragHandleZone, dragHandle, type DndEvent } from 'svelte-dnd-action'; import { flip } from 'svelte/animate'; import { type ChecklistItem, generateId, parseChecklist, serializeChecklist, toggleItemWithCascade, indentItem, outdentItem } from '$lib/utils/checklist.js'; @@ -113,6 +113,8 @@ function removeItem(id: string) { if (items.length <= 1) { if (items[0].parentId) items[0].parentId = null; + items = [...items]; + emitChange(); return; } items = items.map((i) => (i.parentId === id ? { ...i, parentId: null } : i)); @@ -262,8 +264,6 @@
handleDragPointerDown(e, item.id)} class="drag-handle cursor-grab max-md:opacity-100 md:opacity-0 md:group-hover:opacity-100 focus:opacity-100 transition-opacity duration-150 touch-none outline-none" data-testid="checklist-drag-handle"> diff --git a/src/lib/components/NoteCard.svelte b/src/lib/components/NoteCard.svelte index 809bafd..a87c0a9 100644 --- a/src/lib/components/NoteCard.svelte +++ b/src/lib/components/NoteCard.svelte @@ -43,7 +43,7 @@ const indented = line.startsWith(' - ['); return { text: line.replace(/^ {0,2}- \[[ x]\] /, ''), - checked: line.includes('[x]'), + checked: /^ {0,2}- \[x\] /.test(line), indented }; }) diff --git a/src/routes/(share)/s/[token]/+page.svelte b/src/routes/(share)/s/[token]/+page.svelte index 4756263..c1ef719 100644 --- a/src/routes/(share)/s/[token]/+page.svelte +++ b/src/routes/(share)/s/[token]/+page.svelte @@ -24,7 +24,7 @@ const indented = line.startsWith(' - ['); return { text: line.replace(/^ {0,2}- \[[ x]\] /, ''), - checked: line.includes('[x]'), + checked: /^ {0,2}- \[x\] /.test(line), indented }; }) diff --git a/tests/e2e/checklist.spec.ts b/tests/e2e/checklist.spec.ts index c006498..47ead8e 100644 --- a/tests/e2e/checklist.spec.ts +++ b/tests/e2e/checklist.spec.ts @@ -136,6 +136,29 @@ test.describe('Checklist', () => { await expect(page.getByTestId('checklist-done-section')).toContainText('Done task'); }); + test('Scenario: Arrow keys navigate between checklist items', async ({ authenticatedPage: page }) => { + // Given a checklist with three items + await page.getByTestId('new-note-btn').click(); + await page.getByTestId('checklist-toggle').click(); + await page.getByTestId('checklist-input').first().fill('First'); + await page.getByTestId('checklist-input').first().press('Enter'); + await page.getByTestId('checklist-input').nth(1).fill('Second'); + await page.getByTestId('checklist-input').nth(1).press('Enter'); + await page.getByTestId('checklist-input').nth(2).fill('Third'); + + // When the user presses ArrowUp from the third item + await page.getByTestId('checklist-input').nth(2).press('ArrowUp'); + + // Then focus moves to the second item + await expect(page.getByTestId('checklist-input').nth(1)).toBeFocused(); + + // When the user presses ArrowDown + await page.getByTestId('checklist-input').nth(1).press('ArrowDown'); + + // Then focus moves back to the third item + await expect(page.getByTestId('checklist-input').nth(2)).toBeFocused(); + }); + test('Scenario: Tab indents an item and Shift+Tab outdents it', async ({ authenticatedPage: page }) => { // Given a checklist with two top-level items await page.getByTestId('new-note-btn').click(); @@ -249,13 +272,14 @@ test.describe('Checklist', () => { test('Scenario: NoteCard preview shows indentation for nested items', async ({ authenticatedPage: page }) => { // Given a checklist note with nested items - await createChecklistNote(page, 'Preview Indent', ['Parent', 'Child']); - await noteCard(page, 'Preview Indent').click(); + await createChecklistNote(page, 'Indent Preview Note', ['Parent', 'Child']); + await noteCard(page, 'Indent Preview Note').click(); await page.getByTestId('checklist-input').nth(1).press('Tab'); await page.getByTestId('close-editor-btn').click(); - // Then the NoteCard preview shows the child indented - await expect(page.getByTestId('card-checklist-child')).toHaveCount(1); + // Then the NoteCard preview shows the child indented (scoped to this card) + const card = noteCard(page, 'Indent Preview Note'); + await expect(card.getByTestId('card-checklist-child')).toHaveCount(1); }); test('Scenario: Disabling checklist mode restores the rich text editor', async ({ authenticatedPage: page }) => {