diff --git a/e2e/specs/07-checklists.e2e.ts b/e2e/specs/07-checklists.e2e.ts index f51558d..8b034fc 100644 --- a/e2e/specs/07-checklists.e2e.ts +++ b/e2e/specs/07-checklists.e2e.ts @@ -2,8 +2,8 @@ import { browser, expect } from '@wdio/globals'; import { canvas } from '../pageobjects/canvas.page.js'; import { editor } from '../pageobjects/editor.page.js'; -import { clipboardText } from '../support/app.js'; -import { bridge, query } from '../support/bridge.js'; +import { clipboardText, reloadCanvas } from '../support/app.js'; +import { bridge, draft, homeSpaceId, query } from '../support/bridge.js'; /** * A todo list has items and no body: `note_items` is keyed by position, so every write @@ -134,4 +134,67 @@ describe('Todo lists', () => { expect(progress).toContain('1'); expect(progress).toContain('2'); }); + + /** + * A card shows two items of a list. Found by a third, it used to show the first two and + * explain nothing — the branch that renders a search excerpt sits behind `isChecklist()` + * and a todo list never reached it. + */ + describe('found by an item the card does not show', () => { + const long = 'Deep list'; + + before(async () => { + await bridge.createNote( + draft({ + spaceId: await homeSpaceId(), + title: long, + kind: 'checklist', + items: [ + { text: 'first step', done: false }, + { text: 'second step', done: false }, + { text: 'third step', done: false }, + { text: 'rotate the kubeconfig', done: false }, + ], + }), + ); + await reloadCanvas(); + }); + + after(async () => { + await canvas.clearSearch(); + }); + + it('slides its window to the item that matched', async () => { + await canvas.search('kubeconfig'); + const card = await canvas.cardWithTitle(long); + const texts = await card.$$('[data-testid="note-card-item"]').map((item) => item.getText()); + + expect(texts.join(' ')).toContain('rotate the kubeconfig'); + expect(texts.join(' ')).not.toContain('first step'); + }); + + /** + * ⚠️ The one that would have corrupted data: the template counts within the window, + * the position in the note is what gets written. Ticking the first visible box must + * not tick the first box of the list. + */ + it('ticks the box it shows, not the one at the same place in the list', async () => { + await canvas.search('kubeconfig'); + const card = await canvas.cardWithTitle(long); + const boxes = await card.$$('[data-testid="note-card-item"]').getElements(); + // The second visible box, which is the last item of the list. + await boxes[1]!.click(); + + await browser.waitUntil( + async () => { + const view = await bridge.queryNotes(query({ search: long })); + return view.sections[0]?.notes[0]?.items?.at(-1)?.done === true; + }, + { timeout: 10_000, timeoutMsg: 'the matching item never came back ticked' }, + ); + + const items = (await bridge.queryNotes(query({ search: long }))).sections[0]?.notes[0]?.items; + expect(items?.map((item) => item.done)).toEqual([false, false, false, true]); + }); + }); }); diff --git a/src/app/notes/canvas/note-section/note-card/note-card.component.spec.ts b/src/app/notes/canvas/note-section/note-card/note-card.component.spec.ts index 17f56c1..8157c49 100644 --- a/src/app/notes/canvas/note-section/note-card/note-card.component.spec.ts +++ b/src/app/notes/canvas/note-section/note-card/note-card.component.spec.ts @@ -141,6 +141,110 @@ describe('NoteCardComponent', () => { expect(text('[data-testid="note-card-hit"]')).toBe('Push the tag'); }); + /** + * A todo list has no body, so the branch that renders the excerpt sits behind + * `isChecklist()` and was never reached: a list found by its fifth item showed its + * first two and `+3 more`, explaining nothing. + */ + describe('on a todo list, which has no body to quote into', () => { + const items = [ + { text: 'Version bumped', done: true }, + { text: 'Lockfiles agree', done: true }, + { text: 'Changelog written', done: false }, + { text: 'Dry run of the release workflow', done: false }, + { text: 'Tag pushed', done: false }, + ]; + + function itemTexts(): string[] { + return fixture.debugElement + .queryAll(By.css('[data-testid="note-card-item"] .item-text')) + .map((node) => node.nativeElement.textContent.trim()); + } + + it('slides its window to the item that matched', async () => { + fixture.componentRef.setInput( + 'note', + createNote({ + kind: 'checklist', + items, + searchHit: { field: 'item', excerpt: 'Dry run of the release workflow' }, + }), + ); + await fixture.whenStable(); + + expect(itemTexts()).toEqual(['Dry run of the release workflow', 'Tag pushed']); + }); + + it('leaves the window at the head when the match is already in it', async () => { + fixture.componentRef.setInput( + 'note', + createNote({ kind: 'checklist', items, searchHit: { field: 'item', excerpt: 'Version bumped' } }), + ); + await fixture.whenStable(); + + expect(itemTexts()).toEqual(['Version bumped', 'Lockfiles agree']); + }); + + /** + * ⚠️ The excerpt is clipped at 160 characters, so a long item comes back with a + * trailing `…` and never equals its own text. + */ + it('finds the item behind a clipped excerpt', async () => { + const long = 'x'.repeat(200); + fixture.componentRef.setInput( + 'note', + createNote({ + kind: 'checklist', + items: [...items, { text: long, done: false }], + searchHit: { field: 'item', excerpt: `${'x'.repeat(160)}…` }, + }), + ); + await fixture.whenStable(); + + expect(itemTexts()).toEqual(['Tag pushed', long]); + }); + + /** + * ⚠️ The template counts within the window; the position in the note is what gets + * written. Without the offset, ticking the first visible box edits the first box + * of the list — a card silently changing the wrong line. + */ + it('ticks the item it shows, not the one at the same place in the list', async () => { + const store = TestBed.inject(NotesStore); + const setChecklist = vi.spyOn(store, 'setChecklist').mockResolvedValue(undefined); + fixture.componentRef.setInput( + 'note', + createNote({ + id: 'note-42', + kind: 'checklist', + items, + searchHit: { field: 'item', excerpt: 'Dry run of the release workflow' }, + }), + ); + await fixture.whenStable(); + + fixture.debugElement.queryAll(By.css('[data-testid="note-card-item"]'))[0].nativeElement.click(); + await fixture.whenStable(); + + const written = setChecklist.mock.calls[0][1]; + expect(written.map((item) => item.done)).toEqual([true, true, false, true, false]); + }); + }); + + it('slides the tags it shows to the one that matched', async () => { + fixture.componentRef.setInput( + 'note', + createNote({ + tags: ['angular', 'ci', 'urgent'], + searchHit: { field: 'tag', excerpt: 'urgent' }, + }), + ); + await fixture.whenStable(); + + const tags = fixture.debugElement.queryAll(By.css('.card-tags span')); + expect(tags.map((tag) => tag.nativeElement.textContent)).toEqual(['#ci', '#urgent']); + }); + it('keeps the head of the body when the back end quoted nothing', async () => { fixture.componentRef.setInput('note', createNote({ content: 'one\ntwo', searchHit: null })); await fixture.whenStable(); diff --git a/src/app/notes/canvas/note-section/note-card/note-card.component.ts b/src/app/notes/canvas/note-section/note-card/note-card.component.ts index 7c16ea2..973ab2e 100644 --- a/src/app/notes/canvas/note-section/note-card/note-card.component.ts +++ b/src/app/notes/canvas/note-section/note-card/note-card.component.ts @@ -114,11 +114,63 @@ export class NoteCardComponent { return !hit || hit.field === 'body'; }); - protected readonly displayedTags = computed(() => this.note().tags.slice(0, MAX_VISIBLE_TAGS)); + /** + * Where a short list has to start for the thing a search found to be in it. + * + * ⚠️ A **window**, not a filter: the list keeps its order and its length, so what the + * reader sees is the card scrolled to the right place rather than a different card. + */ + private windowStart(length: number, at: number, size: number): number { + if (at < size) return 0; + return Math.min(at, Math.max(0, length - size)); + } + + /** + * ⚠️ The excerpt is clipped at 160 characters, so an item found by a long line comes + * back with a trailing `…` and never equals its own text. Compared by prefix. + */ + private indexOfHit(texts: readonly string[], excerpt: string): number { + const needle = excerpt.endsWith('…') ? excerpt.slice(0, -1) : excerpt; + return texts.findIndex((text) => text.startsWith(needle)); + } + + private readonly tagWindowStart = computed(() => { + const hit = this.searchHit(); + if (hit?.field !== 'tag') return 0; + + const tags = this.note().tags; + return this.windowStart(tags.length, this.indexOfHit(tags, hit.excerpt), MAX_VISIBLE_TAGS); + }); + + protected readonly displayedTags = computed(() => { + const from = this.tagWindowStart(); + return this.note().tags.slice(from, from + MAX_VISIBLE_TAGS); + }); protected readonly isChecklist = computed(() => this.note().kind === 'checklist'); protected readonly progress = computed(() => checklistProgress(this.note().items)); - protected readonly visibleItems = computed(() => this.note().items.slice(0, MAX_VISIBLE_ITEMS)); + /** + * A todo list has no body, so `searchHit` never reached its card: the branch that + * renders the excerpt is unreachable behind `isChecklist()`. A list found by its fifth + * item showed its first two and `+3 more`, explaining nothing. + * + * ⚠️ Replacing the layer with the excerpt was not an option: these are real checkboxes + * a card can be ticked from. The window slides to the matching item instead, and the + * boxes keep working. + */ + private readonly itemWindowStart = computed(() => { + const hit = this.searchHit(); + if (hit?.field !== 'item') return 0; + + const items = this.note().items; + const texts = items.map((item) => item.text); + return this.windowStart(items.length, this.indexOfHit(texts, hit.excerpt), MAX_VISIBLE_ITEMS); + }); + + protected readonly visibleItems = computed(() => { + const from = this.itemWindowStart(); + return this.note().items.slice(from, from + MAX_VISIBLE_ITEMS); + }); protected readonly hiddenItemCount = computed(() => Math.max(0, this.note().items.length - MAX_VISIBLE_ITEMS), ); @@ -161,9 +213,18 @@ export class NoteCardComponent { this.selection.toggleChecked(this.note().id); } - /** Ticking from the card without opening the note: the whole list is written back. */ - protected onItemToggle(event: MouseEvent, index: number): void { + /** + * Ticking from the card without opening the note: the whole list is written back. + * + * ⚠️ The template counts within the **window**, and the position in the note is what + * gets written. They were the same number while the window always started at zero; + * now that a search slides it, ticking the first visible box would have ticked the + * first box of the list instead — a card silently editing the wrong line. + */ + protected onItemToggle(event: MouseEvent, indexInWindow: number): void { event.stopPropagation(); + const index = this.itemWindowStart() + indexInWindow; + void this.notes.setChecklist( this.note().id, this.note().items.map((item, at) => (at === index ? { ...item, done: !item.done } : { ...item })),