Repository navigation
Add Command-F search within journal entries - #3
Conversation
|
starting review... |
|
starting review... |
![eve-code-review-imann24[bot]](https://avatars.githubusercontent.com/in/5224018?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds Cmd/Ctrl+F find-in-entry with highlighting, match navigation, and rendered and raw view support, leaving Cmd+K unchanged.
Findings: 0 blockers, 0 should-fix, 1 nit.
I read the app wiring, the find bar, the highlighting component, the rehype plugin (entry-search.ts), the rendering tests, and the Vitest config. I found no correctness problems. The literal-match escaping is valid under the u flag. Matches don't join across block, image, or <br> boundaries. Raw HTML is shown as text, not parsed. Focus is restored on close.
I did not review tests/entry-find.spec.ts or src/lib/entry-search.test.ts, because their diffs were truncated in my context. That is why this is a comment, not an approval. I also haven't run anything, since the sandbox wasn't available.
Docs checked: the docs lookup found no ticket, spec, or team conventions (no wiki is configured for this instance). The PR description and the README shortcut entry were the only references. The README addition matches the implemented behavior.
| groups.set(id, [...(groups.get(id) ?? []), mark]); | ||
| }); | ||
| setMatches([...groups.values()]); | ||
| setIndex(0); |
There was a problem hiding this comment.
nit: revision includes entry.content, so every live-update refresh of the open entry resets the active match to 1. A user who has stepped to match 7 will jump back to the start if the file changes on disk. Consider keeping the current index (clamped to the new match count) when only the content changes, and resetting only when the query, entry, or view changes.
Summary
Validation
macOS CI passed for f6f516d:
Local DOM smoke checks also passed for shortcuts, literal matching, wrapping, focus, original-text mode, entry navigation, Escape/IME handling, and highlight cleanup. Native macOS interaction has not been manually tested.