feat: find text within notes - #955
Conversation
WalkthroughAdds note-local Find support across editor APIs, provider state, keyboard and command shortcuts, workspace surfaces, focused native menus, routing, dependency versions, and associated tests. ChangesNote-local Find
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds note-local “Find in note” UI and wiring in the desktop app (with per-window sessions and native macOS Edit menu routing), leveraging the underlying rendered-text search added in Meowdown 0.59.0.
Changes:
- Bump
@meowdown/*to0.59.0and wiresearchQuery/onSearchChangethroughNoteEditor→NotePane. - Introduce a per-window
NoteFindProviderplusNoteFindBarchrome, and integrate it into both main and detached note windows. - Add
note.find*commands/shortcuts and route macOS Edit menu Find actions to the currently focused webview window.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Removes temporary commented-out Meowdown override pins. |
| pnpm-lock.yaml | Updates lockfile for Meowdown 0.59.0 and adds prosemirror-search. |
| packages/core/package.json | Bumps @meowdown/markdown to ^0.59.0. |
| apps/desktop/src/styles/index.css | Adds Find input focus-outline suppression styling. |
| apps/desktop/src/routing/route.ts | Adds focusedNotePathForRoute helper for note-scoped targeting. |
| apps/desktop/src/routing/route.test.ts | Tests focusedNotePathForRoute behavior. |
| apps/desktop/src/routing/app-shortcuts.ts | Wires note.find* into command context and macOS-native menu key handling. |
| apps/desktop/src/routing/app-shortcuts.test.tsx | Adds shortcut tests for ⌘F/⌘G behavior targeting the active note. |
| apps/desktop/src/routing/app-shortcuts-macos.test.tsx | Mocks Find actions for macOS shortcut/native menu behavior coverage. |
| apps/desktop/src/providers/note-find-provider.tsx | Implements per-window Find session state, actions, and detached-window listeners. |
| apps/desktop/src/mobile/screens/tasks.test.tsx | Updates mocked NoteEditorHandle with findNext/findPrevious. |
| apps/desktop/src/mobile/mobile-screen.test.tsx | Updates mocked NoteEditorHandle with findNext/findPrevious. |
| apps/desktop/src/lib/native-menu/menu.ts | Adds Find/Next/Previous to Edit menu; documents focused-window dispatch. |
| apps/desktop/src/lib/native-menu/menu.test.ts | Tests menu layout plus focused-window dispatch/event validation. |
| apps/desktop/src/lib/native-menu/dispatch.ts | Routes Find menu commands to focused webview; validates payloads with Zod. |
| apps/desktop/src/lib/commands/types.ts | Extends CommandContext with note Find capabilities. |
| apps/desktop/src/lib/commands/registry.test.ts | Updates command context fakes for new Find capabilities. |
| apps/desktop/src/lib/commands/app-commands.ts | Adds note.find, note.findNext, note.findPrevious commands and keybindings. |
| apps/desktop/src/lib/commands/app-commands.test.ts | Tests new Find commands delegate to CommandContext. |
| apps/desktop/src/lib/attach-files.test.ts | Updates test fakes for the expanded editor handle shape. |
| apps/desktop/src/editor/use-template-slash-items.test.tsx | Updates editor handle test fake with Find methods. |
| apps/desktop/src/editor/use-note-document.test.tsx | Updates editor handle test fake with Find methods. |
| apps/desktop/src/editor/note-editor.tsx | Adds find methods to NoteEditorHandle and passes searchQuery/onSearchChange to Meowdown. |
| apps/desktop/src/components/workspace-content.tsx | Renders NoteFindBar in the main workspace shell. |
| apps/desktop/src/components/workspace-content.test.tsx | Mocks NoteFindBar for workspace-content tests. |
| apps/desktop/src/components/sidebar/sidebar.test.tsx | Updates command context fakes for new Find capabilities. |
| apps/desktop/src/components/route-content.test.tsx | Updates note-editor mocks to include Find methods. |
| apps/desktop/src/components/note-window-content.tsx | Adds NoteFindBar and ensures container supports absolute positioning. |
| apps/desktop/src/components/note-pane.tsx | Connects Find session query/status reporting to mounted editors. |
| apps/desktop/src/components/note-find-bar.tsx | Adds the Find bar UI (query input, match count, next/prev, close). |
| apps/desktop/src/components/note-find-bar.test.tsx | Adds browser-mode tests for bar focus, Escape handling, and navigation. |
| apps/desktop/src/components/graph-workspace.tsx | Wraps window content with NoteFindProvider (main + detached windows). |
| apps/desktop/src/components/daily-stream.focus.test.tsx | Updates note-pane mock handle to include Find methods. |
| apps/desktop/src/components/command-palette/command-palette.test.tsx | Updates command context fakes for new Find capabilities. |
| apps/desktop/package.json | Bumps @meowdown/core and @meowdown/react to ^0.59.0. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| (restoreFocus: boolean): void => { | ||
| const handle = target === null ? null : noteEditorHandleFor(target) | ||
| setSession(null) | ||
| setStatus(NO_MATCHES) | ||
| if (restoreFocus) handle?.focus() |
| // Resolve through the focused stream day so a note-scoped command targets | ||
| // the same day the context sidebar shows (see `effectiveDailyDate`); off | ||
| // the daily views it falls back to the routed note. |
| /* ---- Find in note ------------------------------------------------------ | ||
| The pill is the focus treatment: it exists only while its input is focused, | ||
| so the global outline above would draw a second box inside it. Unlayered, | ||
| like the shadcn opt-out above, because a Tailwind utility cannot beat it. */ |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/providers/note-find-provider.tsx (1)
154-204: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEffect re-subscribes the native menu listener and window keydown handler on every keystroke.
next/previous(viamove) depend onqueryandtarget, so this effect's dependency array changes on every character typed into the Find bar in a detached note window. Each change tears down and re-registerslistenForFocusedNoteMenuCommands(an async Tauri IPC subscription) plus thekeydownlistener — unnecessary I/O-hot-path churn while typing. Thedisposedguard avoids a leak, but the repeated (un)registration is wasted work on a path that fires on every keystroke.The codebase already has a ref-based pattern for exactly this (see
apps/desktop/src/routing/app-shortcuts.ts, which reads frequently-changing state through refs so its keydown effect stays stable). Consider the same approach here: holdnotePath/query/target/next/previous/openForPathin a ref updated every render, and give this effect a stable (e.g.[]orisMainWindow-only) dependency array.♻️ Proposed refactor sketch
+ const stateRef = useRef({ notePath, query, target, next, previous, openForPath }) + useEffect(() => { + stateRef.current = { notePath, query, target, next, previous, openForPath } + }) + useEffect(() => { if (isMainWindow()) return let disposed = false let unlistenMenu = (): void => {} function onMenuCommand(command: FocusedNoteMenuCommand): void { + const { notePath, next, previous, openForPath } = stateRef.current switch (command) { case 'note.find': openForPath(notePath) break case 'note.findNext': next() break case 'note.findPrevious': previous() break } } ... function onKeyDown(event: KeyboardEvent): void { if (event.defaultPrevented || event.altKey || event.repeat || event.isComposing) return if (!event.metaKey && !event.ctrlKey) return const key = event.key.toLowerCase() + const { notePath, query, target, next, previous, openForPath } = stateRef.current if (key === 'f' && !event.shiftKey) { if (openForPath(notePath)) event.preventDefault() return } if (key === 'g' && (target !== null || query.length > 0)) { event.preventDefault() if (event.shiftKey) previous() else next() } } window.addEventListener('keydown', onKeyDown) return () => { disposed = true unlistenMenu() window.removeEventListener('keydown', onKeyDown) } - }, [next, notePath, openForPath, previous, query, target]) + }, [])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/providers/note-find-provider.tsx` around lines 154 - 204, Stabilize the detached-note subscription effect so typing does not re-register the native menu and keydown listeners. In the effect containing onMenuCommand and onKeyDown, store notePath, query, target, next, previous, and openForPath in a ref refreshed each render, read the latest values through that ref, and reduce the effect dependencies to stable initialization conditions such as isMainWindow. Preserve current shortcut and menu-command behavior while retaining cleanup of both listeners.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/styles/index.css`:
- Around line 659-666: Update the .reflect-find-input focus styling to preserve
a visible keyboard focus indicator: remove the input’s outline suppression and
add a compensating :focus-within style on the pill/container with a clear focus
treatment, ensuring keyboard focus remains visible without relying on the
input’s own ring.
---
Nitpick comments:
In `@apps/desktop/src/providers/note-find-provider.tsx`:
- Around line 154-204: Stabilize the detached-note subscription effect so typing
does not re-register the native menu and keydown listeners. In the effect
containing onMenuCommand and onKeyDown, store notePath, query, target, next,
previous, and openForPath in a ref refreshed each render, read the latest values
through that ref, and reduce the effect dependencies to stable initialization
conditions such as isMainWindow. Preserve current shortcut and menu-command
behavior while retaining cleanup of both listeners.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e34448b-dad9-4bd8-a9b8-1b841adf5966
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
apps/desktop/package.jsonapps/desktop/src/components/command-palette/command-palette.test.tsxapps/desktop/src/components/daily-stream.focus.test.tsxapps/desktop/src/components/graph-workspace.tsxapps/desktop/src/components/note-find-bar.test.tsxapps/desktop/src/components/note-find-bar.tsxapps/desktop/src/components/note-pane.tsxapps/desktop/src/components/note-window-content.tsxapps/desktop/src/components/route-content.test.tsxapps/desktop/src/components/sidebar/sidebar.test.tsxapps/desktop/src/components/workspace-content.test.tsxapps/desktop/src/components/workspace-content.tsxapps/desktop/src/editor/note-editor.tsxapps/desktop/src/editor/use-note-document.test.tsxapps/desktop/src/editor/use-template-slash-items.test.tsxapps/desktop/src/lib/attach-files.test.tsapps/desktop/src/lib/commands/app-commands.test.tsapps/desktop/src/lib/commands/app-commands.tsapps/desktop/src/lib/commands/registry.test.tsapps/desktop/src/lib/commands/types.tsapps/desktop/src/lib/native-menu/dispatch.tsapps/desktop/src/lib/native-menu/menu.test.tsapps/desktop/src/lib/native-menu/menu.tsapps/desktop/src/mobile/mobile-screen.test.tsxapps/desktop/src/mobile/screens/tasks.test.tsxapps/desktop/src/providers/note-find-provider.tsxapps/desktop/src/routing/app-shortcuts-macos.test.tsxapps/desktop/src/routing/app-shortcuts.test.tsxapps/desktop/src/routing/app-shortcuts.tsapps/desktop/src/routing/route.test.tsapps/desktop/src/routing/route.tsapps/desktop/src/styles/index.csspackages/core/package.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- pnpm-workspace.yaml
| /* ---- Find in note ------------------------------------------------------ | ||
| The pill is the focus treatment: it exists only while its input is focused, | ||
| so the global outline above would draw a second box inside it. Unlayered, | ||
| like the shadcn opt-out above, because a Tailwind utility cannot beat it. */ | ||
| .reflect-find-input:focus-visible { | ||
| outline: none; | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for any compensating focus-within style targeting the find bar pill.
rg -n 'reflect-find-input|focus-within' apps/desktop/src/styles/index.cssRepository: team-reflect/reflect-open
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== index.css around the rule ==\n'
sed -n '640,680p' apps/desktop/src/styles/index.css
printf '\n== note-find-bar.tsx outline/focus-related bits ==\n'
rg -n 'focus|outline|reflect-find-input|FIND_BUTTON_CLASS|className' apps/desktop/src -g '!*node_modules*'
printf '\n== note-find-bar.test.tsx focus-related test ==\n'
sed -n '1,260p' apps/desktop/src/**/note-find-bar.test.tsx 2>/dev/null || trueRepository: team-reflect/reflect-open
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== apps/desktop/src/styles/index.css (650-675) ==\n'
sed -n '650,675p' apps/desktop/src/styles/index.css
printf '\n== find note component/test locations ==\n'
rg -n 'reflect-find-input|FIND_BUTTON_CLASS|note-find-bar|find bar|find-open|focus-within' apps/desktop/src -g '!**/node_modules/**'
printf '\n== apps/desktop/src for the note find bar component ==\n'
fd -a 'note-find-bar.tsx|note-find-bar.test.tsx' apps/desktop/src
printf '\n== focus-within styles near the find input (if any) ==\n'
rg -n 'reflect-find-input|focus-within' apps/desktop/src/styles/index.css apps/desktop/src -g '!**/node_modules/**'Repository: team-reflect/reflect-open
Length of output: 7425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== note-find-bar.tsx (1-220) ==\n'
sed -n '1,220p' apps/desktop/src/components/note-find-bar.tsx
printf '\n== note-find-bar.test.tsx (1-240) ==\n'
sed -n '1,240p' apps/desktop/src/components/note-find-bar.test.tsxRepository: team-reflect/reflect-open
Length of output: 9926
Restore the find input’s keyboard focus indicator. apps/desktop/src/styles/index.css:663 removes the input’s only visible focus state, and there’s no compensating :focus-within treatment on the pill. Add a visible focus style on the container instead of suppressing the input ring.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/styles/index.css` around lines 659 - 666, Update the
.reflect-find-input focus styling to preserve a visible keyboard focus
indicator: remove the input’s outline suppression and add a compensating
:focus-within style on the pill/container with a clear focus treatment, ensuring
keyboard focus remains visible without relying on the input’s own ring.
Cmd/Ctrl+Fopens a Find bar for the note you are editing, with live match counts, highlights,Enter/Cmd+Gnavigation that wraps, and Find entries in the macOS Edit menu that route to the focused window.The search itself lives in meowdown (prosekit/meowdown#377, released in
0.59.0), so this app owns only the bar, the per-window session, and the command wiring.Supersedes #911.
Summary by CodeRabbit
New Features
Style
Tests