feat(desktop): Recently Viewed section in v2 Quick Open#3488
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a "recently viewed files" feature: constants, a hook, schema field, page integration to record views, and CommandPalette UI changes to show and dedupe recent files alongside search results. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommandPalette
participant Page
participant Hook as useRecentlyViewedFiles
participant CollectionsDB as Collections
User->>CommandPalette: open / search / select file
CommandPalette->>Page: request openFilePane(filePath)
Page->>Hook: recordView({ relativePath, absolutePath })
Hook->>CollectionsDB: read workspaceLocalState(rows) for workspaceId
CollectionsDB-->>Hook: return draft (or none)
Hook->>CollectionsDB: write updated recentlyViewedFiles (dedupe, prepend, slice)
CollectionsDB-->>Hook: ack
Hook-->>Page: (no return)
Page->>CommandPalette: open tab / update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR adds a Recently Viewed section to the v2 Quick Open palette ( Key changes:
Confidence Score: 4/5Safe to merge — core feature logic is correct, persistence is non-destructive, and all edge cases (empty state, dedup, open-tab ordering) are handled. The feature is well-structured: the hook, schema extension, and palette rendering logic are all sound. No data-loss or correctness bugs were found. The remaining issues are all style/P2: a redundant
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Page as page.tsx
participant Hook as useRecentlyViewedFiles
participant Store as v2WorkspaceLocalState
participant Palette as CommandPalette
User->>Page: Open file (Quick Open / Sidebar)
Page->>Hook: recordView({ relativePath, absolutePath })
Hook->>Store: update(workspaceId, draft => prepend + slice(25))
Store-->>Hook: live query update
Hook-->>Page: recentFiles[]
User->>Page: Open Quick Open (Cmd+P)
Page->>Palette: recentlyViewedFiles, openFilePaths
Palette->>Palette: orderedRecent (open tabs first, cap 10)
Palette->>Palette: filteredRecent (substring match on query)
Palette->>Palette: dedupedResults (remove recents from fuzzy list)
Palette-->>User: Recently Viewed section + fuzzy results
Reviews (1): Last reviewed commit: "feat(desktop): add Recently Viewed secti..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const orderedRecent = useMemo<RecentFile[]>(() => { | ||
| if (!showRecentSection || !recentlyViewedFiles) return []; | ||
| const openSet = openFilePaths ?? new Set<string>(); | ||
| const open: RecentFile[] = []; | ||
| const rest: RecentFile[] = []; | ||
| for (const file of recentlyViewedFiles) { | ||
| if (openSet.has(file.absolutePath)) { | ||
| open.push(file); | ||
| } else { | ||
| rest.push(file); | ||
| } | ||
| } | ||
| return [...open, ...rest].slice(0, RECENT_DISPLAY_LIMIT); | ||
| }, [showRecentSection, recentlyViewedFiles, openFilePaths]); |
There was a problem hiding this comment.
open shadows the component's open prop
const open: RecentFile[] = [] on line 68 shadows the open: boolean prop accepted by CommandPalette. While it doesn't cause a runtime bug here (the outer open isn't needed inside this memo), the shadowing is a footgun — any future edit that needs to read the open prop inside this memo will silently get the array instead of the boolean.
Consider renaming the local variable for clarity:
| const orderedRecent = useMemo<RecentFile[]>(() => { | |
| if (!showRecentSection || !recentlyViewedFiles) return []; | |
| const openSet = openFilePaths ?? new Set<string>(); | |
| const open: RecentFile[] = []; | |
| const rest: RecentFile[] = []; | |
| for (const file of recentlyViewedFiles) { | |
| if (openSet.has(file.absolutePath)) { | |
| open.push(file); | |
| } else { | |
| rest.push(file); | |
| } | |
| } | |
| return [...open, ...rest].slice(0, RECENT_DISPLAY_LIMIT); | |
| }, [showRecentSection, recentlyViewedFiles, openFilePaths]); | |
| const orderedRecent = useMemo<RecentFile[]>(() => { | |
| if (!showRecentSection || !recentlyViewedFiles) return []; | |
| const openSet = openFilePaths ?? new Set<string>(); | |
| const openFiles: RecentFile[] = []; | |
| const rest: RecentFile[] = []; | |
| for (const file of recentlyViewedFiles) { | |
| if (openSet.has(file.absolutePath)) { | |
| openFiles.push(file); | |
| } else { | |
| rest.push(file); | |
| } | |
| } | |
| return [...openFiles, ...rest].slice(0, RECENT_DISPLAY_LIMIT); | |
| }, [showRecentSection, recentlyViewedFiles, openFilePaths]); |
| const filteredRecent = useMemo<RecentFile[]>(() => { | ||
| if (!showRecentSection) return []; | ||
| if (!hasQuery) return orderedRecent; | ||
| const needle = trimmedQuery.toLowerCase(); | ||
| return orderedRecent.filter((file) => { | ||
| return ( | ||
| file.relativePath.toLowerCase().includes(needle) || | ||
| getFileName(file.relativePath).toLowerCase().includes(needle) | ||
| ); | ||
| }); | ||
| }, [showRecentSection, hasQuery, trimmedQuery, orderedRecent]); |
There was a problem hiding this comment.
getFileName check in filteredRecent
The second condition — getFileName(file.relativePath).toLowerCase().includes(needle) — is always a subset of the first — file.relativePath.toLowerCase().includes(needle). getFileName returns the last path segment, which is always a substring of the full relative path, so any needle that matches the filename will also match the full path. The second check can never be true when the first is false.
| const filteredRecent = useMemo<RecentFile[]>(() => { | |
| if (!showRecentSection) return []; | |
| if (!hasQuery) return orderedRecent; | |
| const needle = trimmedQuery.toLowerCase(); | |
| return orderedRecent.filter((file) => { | |
| return ( | |
| file.relativePath.toLowerCase().includes(needle) || | |
| getFileName(file.relativePath).toLowerCase().includes(needle) | |
| ); | |
| }); | |
| }, [showRecentSection, hasQuery, trimmedQuery, orderedRecent]); | |
| const filteredRecent = useMemo<RecentFile[]>(() => { | |
| if (!showRecentSection) return []; | |
| if (!hasQuery) return orderedRecent; | |
| const needle = trimmedQuery.toLowerCase(); | |
| return orderedRecent.filter((file) => | |
| file.relativePath.toLowerCase().includes(needle), | |
| ); | |
| }, [showRecentSection, hasQuery, trimmedQuery, orderedRecent]); |
| @@ -1,4 +1,5 @@ | |||
| { | |||
| "root": false, | |||
There was a problem hiding this comment.
"root": false at the repository root is likely unintentional
Setting "root": false tells Biome that this is not the root configuration file and that it should continue traversing parent directories looking for a root config. At the actual repository root there are no parent directories with a Biome config, so Biome will fall back to built-in defaults for anything not covered by this file.
This change appears unrelated to the Recently Viewed feature. Was this added intentionally? If not, it should be reverted — "root": true (or omitting the field entirely) is the expected value for a monorepo's top-level biome.jsonc.
| interface FileResultItemProps { | ||
| itemKey: string; | ||
| value: string; | ||
| fileName: string; | ||
| relativePath: string; | ||
| onSelect: () => void; |
There was a problem hiding this comment.
itemKey prop adds unnecessary API surface
itemKey is passed straight through to a JSX attribute on the sole child element. Since CommandPrimitive.Item is the only thing this component renders, that reconciliation hint is a no-op — React only needs it to tell siblings apart, and there are no siblings here. The parent call sites already provide the correct reconciliation hints on the FileResultItem element itself.
Removing itemKey from FileResultItemProps and dropping its usage on CommandPrimitive.Item would simplify the component interface.
![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
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/CommandPalette/components/FileResultItem/FileResultItem.tsx (1)
19-24: Remove redundantkeyprop inside component.The
key={itemKey}on line 21 has no effect - React'skeyprop only works when specified by the parent during iteration (e.g., in.map()). The parent inCommandPalette.tsxalready provides the key when rendering<FileResultItem key={...} />.🧹 Proposed cleanup
return ( <CommandPrimitive.Item - key={itemKey} value={value} onSelect={onSelect}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/CommandPalette/components/FileResultItem/FileResultItem.tsx` around lines 19 - 24, Remove the redundant key prop from the FileResultItem component JSX: inside FileResultItem (the render returning CommandPrimitive.Item) delete the key={itemKey} attribute because React keys must be set by the parent when mapping and the parent CommandPalette already provides the key; keep value={value}, onSelect={onSelect} and other props intact and verify no other logic depends on itemKey being passed as a React key.apps/desktop/src/renderer/screens/main/components/CommandPalette/CommandPalette.tsx (1)
65-78: Variable shadowing:openshadows the component prop.The local variable
open(line 68) shadows theopenprop (line 33), which can cause confusion when reading the code. Consider renaming toopenFilesorinOpenTabs.🧹 Proposed rename
const orderedRecent = useMemo<RecentFile[]>(() => { if (!showRecentSection || !recentlyViewedFiles) return []; const openSet = openFilePaths ?? new Set<string>(); - const open: RecentFile[] = []; + const openFiles: RecentFile[] = []; const rest: RecentFile[] = []; for (const file of recentlyViewedFiles) { if (openSet.has(file.absolutePath)) { - open.push(file); + openFiles.push(file); } else { rest.push(file); } } - return [...open, ...rest].slice(0, RECENT_DISPLAY_LIMIT); + return [...openFiles, ...rest].slice(0, RECENT_DISPLAY_LIMIT); }, [showRecentSection, recentlyViewedFiles, openFilePaths]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/CommandPalette/CommandPalette.tsx` around lines 65 - 78, The local variable named "open" inside the useMemo in orderedRecent shadows the component prop "open"; rename that local variable (e.g., to openFiles or inOpenTabs) and update all references within the useMemo block (where it is pushed and later spread into the returned array) so it no longer conflicts with the prop, keeping the logic that builds openFiles/rest from recentlyViewedFiles and returns [...openFiles, ...rest]. Ensure you only change the local variable name and its references (inside orderedRecent) and leave the prop "open" and other symbols (orderedRecent, recentlyViewedFiles, openFilePaths, RECENT_DISPLAY_LIMIT) untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@biome.jsonc`:
- Line 2: The Biome config currently sets "root": false which incorrectly marks
this as a nested config; edit the "root" setting in the file to be "root": true
(or remove the "root" field entirely to use Biome's default) so the
configuration is recognized as the repository root; locate the "root" key in the
shown JSONC and update its value accordingly.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/CommandPalette/CommandPalette.tsx`:
- Around line 65-78: The local variable named "open" inside the useMemo in
orderedRecent shadows the component prop "open"; rename that local variable
(e.g., to openFiles or inOpenTabs) and update all references within the useMemo
block (where it is pushed and later spread into the returned array) so it no
longer conflicts with the prop, keeping the logic that builds openFiles/rest
from recentlyViewedFiles and returns [...openFiles, ...rest]. Ensure you only
change the local variable name and its references (inside orderedRecent) and
leave the prop "open" and other symbols (orderedRecent, recentlyViewedFiles,
openFilePaths, RECENT_DISPLAY_LIMIT) untouched.
In
`@apps/desktop/src/renderer/screens/main/components/CommandPalette/components/FileResultItem/FileResultItem.tsx`:
- Around line 19-24: Remove the redundant key prop from the FileResultItem
component JSX: inside FileResultItem (the render returning
CommandPrimitive.Item) delete the key={itemKey} attribute because React keys
must be set by the parent when mapping and the parent CommandPalette already
provides the key; keep value={value}, onSelect={onSelect} and other props intact
and verify no other logic depends on itemKey being passed as a React key.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6015c641-2e58-4373-868b-34ece946245f
📒 Files selected for processing (9)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useRecentlyViewedFiles/constants.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useRecentlyViewedFiles/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useRecentlyViewedFiles/useRecentlyViewedFiles.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tsapps/desktop/src/renderer/screens/main/components/CommandPalette/CommandPalette.tsxapps/desktop/src/renderer/screens/main/components/CommandPalette/components/FileResultItem/FileResultItem.tsxapps/desktop/src/renderer/screens/main/components/CommandPalette/components/FileResultItem/index.tsbiome.jsonc
| @@ -1,4 +1,5 @@ | |||
| { | |||
| "root": false, | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether multiple Biome configs exist in-repo and inspect root setting.
fd -HI '^biome\.jsonc$' .
echo "----"
rg -n '"root"\s*:\s*(true|false)' --glob '**/biome.jsonc'Repository: superset-sh/superset
Length of output: 112
🌐 Web query:
Biome root configuration setting semantics documentation
💡 Result:
The "root" configuration setting in Biome's biome.json or biome.jsonc file determines whether the configuration file should be treated as a root configuration. By default, it is true, meaning any configuration file is considered a root. When set to true (default), the configuration is treated as a root. When a configuration file is intended as a "nested configuration" (e.g., in subdirectories or monorepos alongside other configs), it must explicitly set "root": false. Otherwise, Biome throws an error. This setting is required to allow Biome to properly orchestrate multiple configuration files simultaneously in CLI tools and editors, preventing conflicts during file scanning, linting, formatting, etc. Example for nested config: { "root": false } This applies to Biome's scanner, which discovers nested biome.json files and .gitignore files while respecting files.includes and ignore patterns.
Citations:
- 1: https://biomejs.dev/reference/configuration/
- 2: https://next.biomejs.dev/reference/configuration/
- 3: https://www.biomejs.cn/en/reference/configuration
- 4: https://biomejs.dev/guides/configure-biome/
- 5: Biome seems to ignore overrides when using --staged biomejs/biome#8036
🏁 Script executed:
# Check if there are any biome configs in parent directories (outside the repo)
# This helps understand if the root: false setting could pick up external configs
# For now, just verify what the current repo structure looks like and confirm
# the biome.jsonc content around line 2
head -20 ./biome.jsoncRepository: superset-sh/superset
Length of output: 417
Set "root": true to properly indicate this is the root Biome configuration.
At Line 2, "root": false indicates this is a nested configuration file (used in monorepos with multiple configs). Since the repository has a single Biome config at the root, set this to "root": true (Biome's default) to correctly indicate the configuration hierarchy.
Suggested change
- "root": false,
+ "root": true,(Or remove the field entirely to use Biome's default of "root": true.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "root": false, | |
| "root": true, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@biome.jsonc` at line 2, The Biome config currently sets "root": false which
incorrectly marks this as a nested config; edit the "root" setting in the file
to be "root": true (or remove the "root" field entirely to use Biome's default)
so the configuration is recognized as the repository root; locate the "root" key
in the shown JSONC and update its value accordingly.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
force-pushed
the
bedecked-clownfish
branch
from
380c85d to
334c584
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="biome.jsonc">
<violation number="1" location="biome.jsonc:2">
P2: Setting the repository’s main `biome.jsonc` to `"root": false` makes config resolution non-isolated and can cause parent-directory Biome configs to leak into this project.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -1,4 +1,5 @@ | |||
| { | |||
| "root": false, | |||
There was a problem hiding this comment.
P2: Setting the repository’s main biome.jsonc to "root": false makes config resolution non-isolated and can cause parent-directory Biome configs to leak into this project.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At biome.jsonc, line 2:
<comment>Setting the repository’s main `biome.jsonc` to `"root": false` makes config resolution non-isolated and can cause parent-directory Biome configs to leak into this project.</comment>
<file context>
@@ -1,4 +1,5 @@
{
+ "root": false,
"$schema": "https://biomejs.dev/schemas/2.4.2/schema.json",
"vcs": {
</file context>
| "root": false, | |
| "root": true, |
Pins recently-opened files at the top of the v2 workspace command palette, with currently-open tabs preferred. List is persisted per-workspace via the existing v2WorkspaceLocalState localStorage collection (cap 25 stored, 10 displayed). Empty query shows only the section; non-empty query filters recents by substring match and dedupes them out of the fuzzy results. Extracts the duplicated row markup into a shared FileResultItem component.
saddlepaddle
force-pushed
the
bedecked-clownfish
branch
from
334c584 to
c86af4e
Compare
upstream (superset-sh#3488) adds Recently Viewed to the CommandPrimitive-based palette, but fork ships a SearchDialog-based rewrite. Wire upstream's useRecentlyViewedFiles hook and FileResultItem into the fork's dialog so the feature actually works instead of being dead-imported. SearchDialog gets two new optional props: - preResultsSection: ReactNode rendered inside CommandList before results (for the "Recently Viewed" group + separator) - hasPreResults: suppresses the empty state when the recent section has matches but the main results are empty CommandPalette now computes orderedRecent / filteredRecent / dedupedResults (same logic as upstream) and wires them through to SearchDialog. Only active in workspace scope — global scope shows only the flat cross-workspace result list. v2 page passes recentFiles and openFilePaths (already being computed for upstream's abandoned integration) into the palette. Also rewrite FileResultItem to use the fork's wrapped CommandItem instead of raw CommandPrimitive.Item — fork's @superset/ui/command doesn't re-export CommandPrimitive. SidebarHeader non-compact path now reuses the shared `btn` variable via Fragment instead of duplicating the button element.
1 participant
Summary
Cmd+P/ Quick Open).v2WorkspaceLocalStatelocalStorage collection — store cap 25, display cap 10.FileResultItemcomponent.Test plan
package.jsonrecently viewed → it appears once under the heading and is removed from the fuzzy list below the separator..env.local) via the sidebar → it appears in Recently Viewed (spec: views are kept regardless of gitignore).Summary by CodeRabbit
Summary by cubic
Adds a Recently Viewed section to the v2 Quick Open (Cmd+P). It shows your last opened files at the top, prefers files currently open, and persists per workspace.
New Features
Refactors
useRecentlyViewedFilesandrecentlyViewedFilesinv2WorkspaceLocalState.FileResultItemfor the file row UI.Written for commit c86af4e. Summary will update on new commits.