Conversation
- Teleport spotlight overlay to body to escape sidebar compositing context
- Drop contain:paint and will-change/translateZ from sidebar so spotlight
z-index can take effect across stacking contexts
- Add v-memo and rAF-batched mouseenter to result items; skip scrollIntoView
on mouse-driven activeIndex changes
- Raise SelectContent z-index to z-[80] so dropdowns render above the
ChatStatusBar popover (z-72) and other custom overlays
- Update Spotlight tasks.md to reflect actual completion state
- Refresh tests for deepchat_sessions schema v28, buildHistoryTurns 5-arg
signature, and ChatPage pending-lane DOM order
Contributor
📝 WalkthroughWalkthroughThis PR advances the Spotlight search toward completion (Teleport rendering, keyboard/mouse active-item distinction, hover throttling), tweaks Spotlight visuals, updates DB schema tests to v28, simplifies sidebar/select CSS, and reforms a few test assertions and docs. ChangesSpotlight Search Feature Completion & Layout Refinements
Sequence Diagram — Spotlight Interaction FlowsequenceDiagram
participant User
participant SpotlightOverlay
participant KeyboardHandler
participant HoverHandler
participant SpotlightStore
participant DOM
User->>SpotlightOverlay: Keyboard event (ArrowUp/Down/Home/End)
SpotlightOverlay->>KeyboardHandler: handleKeyNavigation
KeyboardHandler->>SpotlightStore: set activeChangeSource = 'keyboard'
KeyboardHandler->>SpotlightStore: update active index
SpotlightStore->>SpotlightOverlay: reactive update triggers watcher
SpotlightOverlay->>DOM: scroll active item into view
User->>SpotlightOverlay: Mouse enter on result item
SpotlightOverlay->>HoverHandler: handleItemMouseEnter(item)
HoverHandler->>HoverHandler: requestAnimationFrame coalescing
HoverHandler->>SpotlightStore: set activeChangeSource = 'mouse' and update active index
SpotlightStore->>SpotlightOverlay: reactive update triggers watcher
SpotlightOverlay->>DOM: watcher early-returns (skip scroll for mouse)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/spotlight/SpotlightOverlay.vue (1)
18-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset
activeChangeSourcebefore query-driven updates.After one hover, this watcher treats later active-index changes as mouse-driven until Arrow/Home/End runs again. Typing a new query after hovering can therefore skip
scrollIntoViewand leave the new active result offscreen.Suggested fix
- `@input`="spotlightStore.setQuery(($event.target as HTMLInputElement).value)" + `@input`="handleInput"const handleInput = (event: Event) => { activeChangeSource = 'keyboard' spotlightStore.setQuery((event.target as HTMLInputElement).value) }Also applies to: 124-126, 287-289
🤖 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 `@src/renderer/src/components/spotlight/SpotlightOverlay.vue` around lines 18 - 19, The input handler should reset activeChangeSource to 'keyboard' before calling spotlightStore.setQuery so subsequent active-index changes are treated as keyboard-driven (ensuring scrollIntoView runs); update the inline `@input` handler (or the handleInput function if present) to set activeChangeSource = 'keyboard' immediately before calling spotlightStore.setQuery, and apply the same change to the other input/update code paths referenced (the blocks around the other occurrences of spotlightStore.setQuery at the locations noted).
🤖 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 `@src/renderer/src/components/spotlight/SpotlightOverlay.vue`:
- Line 5: Spotlight's teleported root is currently using z-[70] and gets painted
under newer overlays at z-72 and z-[80]; update the container's class in
SpotlightOverlay.vue (the element with class "window-no-drag-region fixed
inset-0 z-[70] ...") to use a higher z-index (e.g., z-[90] or above) so the
spotlight renders above the other overlays, and ensure any other teleported
roots or portal wrappers for this component are updated to the same z value.
- Around line 125-126: The current rAF flow stores a stale numeric index
(pendingMouseEnterIndex) causing setActiveItem to pick the wrong or out-of-range
row; change to capture the hovered item's unique id instead: introduce
pendingMouseEnterId (string|number), set it when mouse enters, cancel/reset
mouseEnterRaf as before, and in the rAF callback look up the current item in
spotlightStore.results by id and call setActiveItem(foundItem) only if found;
mirror the same replacement for the second occurrence that uses
pendingMouseEnterIndex (lines ~227-244) and ensure you clear pendingMouseEnterId
when leaving or when the raf is cancelled.
- Line 28: The v-memo in SpotlightOverlay.vue is too narrow (v-memo="[item.id,
index === spotlightStore.activeIndex, spotlightStore.query]") so rows can remain
stale when item fields change; update the memo dependency to either remove
v-memo entirely or include all rendered item properties (e.g., item, item.icon,
item.kind, item.title/item.titleKey via resolveItemTitle(item), item.subtitle,
item.snippet) and the activeIndex/query flags so the cached subtree updates when
useSpotlightStore rebuilds results; locate the v-memo usage in the
SpotlightOverlay component and replace the dependency array with a broader
dependency (or memoize on the entire item object identity) referencing
resolveItemTitle, spotlightStore.activeIndex, and spotlightStore.query as
needed.
---
Outside diff comments:
In `@src/renderer/src/components/spotlight/SpotlightOverlay.vue`:
- Around line 18-19: The input handler should reset activeChangeSource to
'keyboard' before calling spotlightStore.setQuery so subsequent active-index
changes are treated as keyboard-driven (ensuring scrollIntoView runs); update
the inline `@input` handler (or the handleInput function if present) to set
activeChangeSource = 'keyboard' immediately before calling
spotlightStore.setQuery, and apply the same change to the other input/update
code paths referenced (the blocks around the other occurrences of
spotlightStore.setQuery at the locations noted).
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46abd22a-1d04-4f0f-aa3d-275d80f5588a
📒 Files selected for processing (7)
docs/features/app-spotlight-search/tasks.mdsrc/renderer/src/components/WindowSideBar.vuesrc/renderer/src/components/spotlight/SpotlightOverlay.vuesrc/shadcn/components/ui/select/SelectContent.vuetest/main/presenter/agentRuntimePresenter/compactionService.test.tstest/main/presenter/sqlitePresenter/deepchatSessionsTable.test.tstest/renderer/components/ChatPage.test.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/src/components/spotlight/SpotlightOverlay.vue (1)
227-254: 💤 Low valueOptional: collapse the duplicate
findIndex/findlookups.The rAF body iterates
spotlightStore.resultsthree times (the upfrontfindIndexon Line 228, thenfindon Line 243, thenfindIndexagain on Line 247). For typical result counts this is negligible, but it can be reduced to one pass without changing semantics. The early bail-out on Line 229 can also be made O(1) by comparing againstresults[activeIndex]?.idinstead of scanning the array on every hover.♻️ Suggested simplification
const handleItemMouseEnter = (item: SpotlightItem) => { - const currentIndex = spotlightStore.results.findIndex((r) => r.id === item.id) - if (currentIndex === -1 || spotlightStore.activeIndex === currentIndex) { + if (spotlightStore.results[spotlightStore.activeIndex]?.id === item.id) { return } pendingMouseEnterId = item.id if (mouseEnterRaf !== 0) { return } mouseEnterRaf = window.requestAnimationFrame(() => { mouseEnterRaf = 0 const targetId = pendingMouseEnterId pendingMouseEnterId = null if (targetId === null) { return } - const foundItem = spotlightStore.results.find((r) => r.id === targetId) - if (!foundItem) { - return - } const targetIndex = spotlightStore.results.findIndex((r) => r.id === targetId) if (targetIndex < 0 || spotlightStore.activeIndex === targetIndex) { return } activeChangeSource = 'mouse' spotlightStore.setActiveItem(targetIndex) }) }🤖 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 `@src/renderer/src/components/spotlight/SpotlightOverlay.vue` around lines 227 - 254, handleItemMouseEnter performs multiple scans of spotlightStore.results; simplify by caching const results = spotlightStore.results and use results[spotlightStore.activeIndex]?.id for the early bail-out instead of findIndex, then inside the RAF use the cached results to locate the target item and index in a single pass (or use results.findIndex once and derive the item from results[index])—update references to pendingMouseEnterId, mouseEnterRaf, activeChangeSource and call spotlightStore.setActiveItem(targetIndex) as before but without repeated array iterations.
🤖 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.
Nitpick comments:
In `@src/renderer/src/components/spotlight/SpotlightOverlay.vue`:
- Around line 227-254: handleItemMouseEnter performs multiple scans of
spotlightStore.results; simplify by caching const results =
spotlightStore.results and use results[spotlightStore.activeIndex]?.id for the
early bail-out instead of findIndex, then inside the RAF use the cached results
to locate the target item and index in a single pass (or use results.findIndex
once and derive the item from results[index])—update references to
pendingMouseEnterId, mouseEnterRaf, activeChangeSource and call
spotlightStore.setActiveItem(targetIndex) as before but without repeated array
iterations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86410631-4e3d-431c-9d6a-60c1209d5189
📒 Files selected for processing (1)
src/renderer/src/components/spotlight/SpotlightOverlay.vue
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.
Summary by CodeRabbit
Documentation
Bug Fixes
Improvements
Tests