Move Reindex button to Settings → Index tab - #129
Merged
Conversation
Indexing is automatic now — auto-index on open (#25), the fs-watch pulse re-projecting every external save, and a cancelled run healing off the DB-derived pending set on the next pass. A manual Reindex is therefore the exception, and permanent top-bar chrome for an exception trains the eye to skip the bar. The button moves to a new Settings section, Index, where it sits beside the coverage numbers that say whether you need it. What stays on the front page is the half you can't put behind a modal: the live progress meter and its Cancel. It moves to sit with the vault name — a run is *about* a vault, so the meter reads beside the one being indexed rather than floating at the far end of the bar (`.vault-status`). - settingstabs.ts: a fourth section, General / Index / Embedding / Keyboard. The rail's walk, roving tabstop and Home/End all follow from the list. - render.ts: `indexPanelHtml`, plus `reindexDisabled`/`reindexLabel` exported so the panel's paint and main.ts's targeted repaint (which runs on every streamed progress batch without a full render) can't drift on either. - main.ts: the click is wired inside the `settingsOpen` branch — that branch returns unconditionally, so a click in the dialog never reaches the shell's handlers. `paintReindex` now tolerates a button that isn't on screen. - `captureModalFocus` restores focus only to a control that can *take* it: pressing Reindex leaves the button present and disabled, which `.focus()` silently declines — the `<body>`-behind-a-backdrop outcome an overlay may never produce (obligation 3). Membership in `overlayFocusables()` is the test, since its selector already excludes `[disabled]`. render.test.ts pins the button's id (both halves restore by it), its disabled/renamed state mid-run, and that the panel counts indexed and embedded separately — the #26 honesty, in the one place a human comes to ask whether the index is finished. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGNb4cJT7YL6ejApdNizpM
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds an Index tab to Settings. It displays index coverage and Reindex states. Manual reindexing starts from Settings, while progress and cancellation remain in the top bar. Frontend test discovery now includes nested test files. ChangesIndex settings and reindex controls
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsDialog
participant MainShell
participant IndexingRun
participant TopBar
SettingsDialog->>MainShell: Click Reindex
MainShell->>IndexingRun: Start tracked indexing run
IndexingRun-->>MainShell: Report indexing state
MainShell-->>SettingsDialog: Update label and disabled state
MainShell-->>TopBar: Show progress and Cancel controls
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@ui/src/render.test.ts`:
- Around line 390-455: Update the test script in ui/package.json to use a
recursive test glob such as src/**/*.test.ts instead of src/*.test.ts, ensuring
ui/src/render.test.ts and future nested test files are discovered by the Node
test runner.
🪄 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 Plus
Run ID: 52069b5b-9645-4f5d-90c7-3aa6179d1f75
📒 Files selected for processing (6)
CLAUDE.mdui/src/main.tsui/src/render.test.tsui/src/render.tsui/src/settingstabs.tsui/style.css
`src/*.test.ts` already finds every test file — ui/src is flat, and the suite reports all 25 — so nothing was being skipped. But the claim the justfile and CLAUDE.md make for that glob is "a new file is never silently skipped", and a nested one would break it in the quietest way there is: a green run over a smaller set. `**` makes the promise true for that case too. Verified a strict superset before taking it: both globs discover and pass the same 25 files, so this cannot be the failure it protects against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGNb4cJT7YL6ejApdNizpM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Indexing is now automatic (on open + fs-watch), so the manual Reindex button no longer belongs in permanent top-bar chrome. This change relocates it to Settings → Index, where it sits beside the coverage numbers that tell you whether you need it.
Summary
The Reindex button moves from the top bar to a new "Index" tab in Settings (⌘,), while the progress meter and Cancel button remain in the top bar beside the vault name. This reflects the shift to automatic indexing: the exception (manual reindex) belongs where you go looking for it, not in permanent chrome.
Key changes
indexPanelHtml)buildShell) to Settings → Index panelreindexDisabled,reindexLabel) so the panel's initial paint and main.ts's targeted repaints (paintReindex) stay in sync.vault-statuscontainercaptureModalFocusto handle disabled elements correctly—a disabled button that exists can't take focus, so it falls through to the next focusable elementwireEvents(the only place the button can be clicked); Cancel stays in the shell handlersImplementation details
id="reindex"in both the panel paint and main.ts's targeted repaint, so focus restoration and click delegation both workreindexDisabledandreindexLabelare exported from render.ts so they're the single source of truth for button state.vault-statuswith tight gap andmin-width: 0for ellipsis)https://claude.ai/code/session_01QGNb4cJT7YL6ejApdNizpM
Summary by CodeRabbit