-
-
Notifications
You must be signed in to change notification settings - Fork 493
feat(editor): add cmdk global search #240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@SwasthK is attempting to deploy a commit to the tweakcn OSS program Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a client-side CmdK command palette (Ctrl/Cmd+K) for navigation and theme preset selection, centralizes theme color previews into ThemeColors, extracts preset filtering/sorting utilities, introduces small UI components (Kbd, ColorBox), updates header layout for responsive visibility, and adjusts dark theme composition in theme helper. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CmdK as CmdK component
participant Store as Theme/Editor Stores
participant Router as Next.js Router
participant UI as UI (ThemeColors, lists)
User->>CmdK: Press Ctrl/Cmd+K (open)
CmdK->>UI: Render grouped items (Navigation, Saved, Built-in)
CmdK->>Store: Read current mode & active preset
User->>CmdK: Type search
CmdK->>UI: Filter items (filterPresets, NAVIGATION_ITEMS)
alt select navigation item
CmdK->>Router: navigate(href)
else select theme item
CmdK->>Store: applyThemePreset(presetName)
CmdK->>UI: indicate active / "New" badge, show ThemeColors
end
User->>CmdK: Press Esc or click outside
CmdK->>User: Close palette
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (2)
lib/search/filter-presets.ts (1)
3-15: Consider fallback to name when label is missing.The filtering logic is mostly correct, but line 11 will exclude presets that lack a
labelproperty, even if the presetnamematches the search term. For the "default" preset, you special-case it to match against "default", but other presets without labels become unsearchable.Consider adding a fallback to the preset name when the label is missing:
export const filterPresets = (presetNames: string[], presets: Record<string, ThemePreset>, search: string) => { const filteredList = search.trim() === "" ? presetNames : presetNames.filter((name) => { if (name === "default") { return "default".toLowerCase().includes(search.toLowerCase()); } - return presets[name]?.label?.toLowerCase().includes(search.toLowerCase()); + const searchTarget = (presets[name]?.label || name).toLowerCase(); + return searchTarget.includes(search.toLowerCase()); }); return filteredList; };This ensures all presets remain searchable, even if they lack an explicit label.
lib/search/is-theme-new.ts (1)
3-8: Make “new” window explicit and handle invalid dates defensivelyThe 5‑day window is currently a magic number and invalid
createdAtstrings silently fall through. Consider making the period configurable/constant and guarding against invalid dates to make behavior clearer:const NEW_THEME_WINDOW_DAYS = 5; export const isThemeNew = (preset: ThemePreset) => { if (!preset.createdAt) return false; const createdAt = new Date(preset.createdAt); if (Number.isNaN(createdAt.getTime())) return false; const threshold = new Date(); threshold.setDate(threshold.getDate() - NEW_THEME_WINDOW_DAYS); return createdAt > threshold; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
components/cmdk.tsx(1 hunks)components/editor/theme-preset-select.tsx(3 hunks)components/get-pro-cta.tsx(1 hunks)components/header.tsx(3 hunks)components/search/color-box.tsx(1 hunks)components/search/theme-colors.tsx(1 hunks)components/ui/command.tsx(1 hunks)components/ui/kbd.tsx(1 hunks)lib/search/constants/navigation.ts(1 hunks)lib/search/filter-presets.ts(1 hunks)lib/search/is-theme-new.ts(1 hunks)lib/search/sort-themes.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
components/editor/theme-preset-select.tsx (3)
lib/search/filter-presets.ts (1)
filterPresets(3-15)lib/search/sort-themes.ts (1)
sortThemes(3-13)components/search/theme-colors.tsx (1)
ThemeColors(10-20)
components/search/theme-colors.tsx (2)
utils/theme-preset-helper.ts (1)
getPresetThemeStyles(5-28)components/search/color-box.tsx (1)
ColorBox(5-7)
lib/search/filter-presets.ts (1)
types/theme.ts (1)
ThemePreset(101-109)
components/ui/kbd.tsx (1)
lib/utils.ts (1)
cn(6-8)
components/header.tsx (3)
components/get-pro-cta.tsx (1)
GetProCTA(11-35)components/cmdk.tsx (1)
CmdK(28-241)components/social-link.tsx (1)
SocialLink(8-23)
lib/search/sort-themes.ts (1)
types/theme.ts (1)
ThemePreset(101-109)
lib/search/is-theme-new.ts (1)
types/theme.ts (1)
ThemePreset(101-109)
components/cmdk.tsx (8)
store/editor-store.ts (1)
useEditorStore(34-231)store/theme-preset-store.ts (1)
useThemePresetStore(17-74)lib/auth-client.ts (1)
authClient(5-7)lib/search/filter-presets.ts (1)
filterPresets(3-15)lib/search/sort-themes.ts (1)
sortThemes(3-13)lib/search/constants/navigation.ts (1)
NAVIGATION_ITEMS(11-54)components/search/theme-colors.tsx (1)
ThemeColors(10-20)lib/search/is-theme-new.ts (1)
isThemeNew(3-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (11)
components/get-pro-cta.tsx (1)
31-31: LGTM: Clean responsive implementation.The responsive wrapper correctly hides the "Get Pro" label on small screens while keeping the icon visible, improving mobile UX.
components/search/color-box.tsx (1)
1-7: LGTM: Simple and reusable color preview component.The component correctly renders a small colored box with proper border and sizing. Using inline
styleforbackgroundColoris appropriate since the color value is dynamic.components/ui/command.tsx (1)
24-26: LGTM: Clean interface extension.The optional
titleprop is properly typed and correctly forwarded toDialogContent. This enhancement aligns with the CmdK feature's need for titled dialogs.Also applies to: 28-28, 31-31
components/ui/kbd.tsx (1)
3-16: LGTM: Well-styled keyboard key component.The
Kbdcomponent is properly implemented with comprehensive styling including theme variants, sizing, and tooltip-specific styles.components/editor/theme-preset-select.tsx (3)
13-20: LGTM: Clean imports for refactored utilities.The new imports support the extraction of filtering, sorting, and rendering logic into reusable utilities, improving code organization.
183-189: LGTM: Effective refactoring of filter and sort logic.Extracting
filterPresetsandsortThemesinto separate utilities improves maintainability and enables reuse in other components (like the CmdK palette). The logic remains functionally equivalent while being more modular.
210-210: LGTM: Consistent use of ThemeColors component.Replacing inline ColorBox rendering with the
ThemeColorscomponent centralizes the theme color preview logic, ensuring consistency across the UI and simplifying future updates.Also applies to: 290-290, 337-337
lib/search/sort-themes.ts (1)
3-13: LGTM: Clean and correct theme sorting logic.The function properly:
- Isolates the "default" theme to appear first
- Sorts remaining themes alphabetically by label (with fallback to name)
- Uses
localeComparefor locale-aware string comparisoncomponents/header.tsx (1)
34-47: CmdK integration and responsive header layout look goodPlacing
<CmdK />next toGetProCTAwith separators and hiding secondary social links on small screens is a sensible balance between discoverability and compactness; the breakpoint choices (sm/md) read well with the rest of the header layout.components/cmdk.tsx (2)
79-103: Global Cmd+K handler is straightforward and correctly cleaned upThe global
keydownhandler forCmd/Ctrl+Kis set up in an effect with a proper cleanup, so you won’t leak listeners across mounts, and the shortcut behavior is easy to follow.
59-69: Review comment is incorrect. No duplicate "default" preset issue exists.The review assumes
presetscontains a"default"key, but examination ofutils/theme-presets.tsconfirms thatdefaultPresetshas no"default"key. The object contains preset names like"modern-minimal","violet-bloom", etc., but not"default".Line 59 correctly prepends
"default"as an explicit literal string to the keyed presets, avoiding any duplication. The subsequent filtering logic (lines 65–66) intentionally treats"default"as a special case distinct from saved presets, confirming this is by design.Likely an incorrect or invalid review comment.
| const filteredPresets = useMemo(() => { | ||
| const filteredList = filterPresets(presetNames, presets, search); | ||
|
|
||
| // Separate saved and default themes | ||
| const savedThemesList = filteredList.filter((name) => name !== "default" && isSavedTheme(name)); | ||
| const defaultThemesList = filteredList.filter((name) => !savedThemesList.includes(name)); | ||
|
|
||
| return [...sortThemes(savedThemesList, presets), ...sortThemes(defaultThemesList, presets)]; | ||
| }, [presetNames, search, presets, isSavedTheme]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Harden filterPresets against missing labels
Because filterPresets ultimately does presets[name]?.label?.toLowerCase().includes(...), any preset without a label will cause a runtime error when searching. Given ThemePreset.label is optional, this is a plausible edge case.
You can guard against missing labels inside filterPresets (file: lib/search/filter-presets.ts) by short‑circuiting:
return presetNames.filter((name) => {
if (name === "default") {
return "default".includes(searchLower);
}
const label = presets[name]?.label;
return label ? label.toLowerCase().includes(searchLower) : false;
});Since CmdK relies on this helper for every keystroke, a defensive check here is worthwhile.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and inspect filterPresets implementation
echo "=== Searching for filterPresets implementation ==="
fd "filter-presets" --type f
Length of output: 79
🏁 Script executed:
#!/bin/bash
# Find ThemePreset type definition
echo "=== Searching for ThemePreset type definition ==="
rg -n "type ThemePreset|interface ThemePreset" --type ts --type tsxLength of output: 77
🏁 Script executed:
#!/bin/bash
# Show current filterPresets implementation
echo "=== Current filterPresets content ==="
cat lib/search/filter-presets.ts 2>/dev/null || echo "File not found at expected path"Length of output: 599
🏁 Script executed:
#!/bin/bash
# Search for ThemePreset definition in all TypeScript files
echo "=== Searching for ThemePreset definition ==="
rg -n "ThemePreset\s*=" --type ts --type tsx -A 5Length of output: 72
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for ThemePreset definition - corrected file type
echo "=== Searching for ThemePreset definition ==="
rg -n "ThemePreset" --type ts -A 8 | head -40
Length of output: 1974
🏁 Script executed:
#!/bin/bash
# Find the types/theme file
echo "=== Locating types/theme file ==="
fd "theme" --path "*/types/*" --type fLength of output: 249
🏁 Script executed:
#!/bin/bash
# Directly inspect the theme types file
echo "=== Contents of types/theme.ts ==="
cat types/theme.ts 2>/dev/null || cat src/types/theme.ts 2>/dev/null || echo "Searching for theme types..."Length of output: 4532
🏁 Script executed:
#!/bin/bash
# Fallback: search for ThemePreset export
echo "=== Finding ThemePreset export ==="
rg -n "export.*ThemePreset" --type ts -B 2 -A 10Length of output: 8723
Guard against missing labels in filterPresets
The current implementation has a runtime error: when presets[name]?.label is undefined (since ThemePreset.label is optional per types/theme.ts:104), calling .toLowerCase() returns undefined, then .includes() fails with TypeError.
Fix in lib/search/filter-presets.ts:11 by short-circuiting when label is missing:
return label ? label.toLowerCase().includes(search.toLowerCase()) : false;🤖 Prompt for AI Agents
In components/cmdk.tsx around lines 61-69, the filteredPresets logic triggers a
runtime error when a preset's optional label is missing; update the filter
function in lib/search/filter-presets.ts (around line 11) to guard against
undefined labels by returning false if label is falsy, otherwise call
toLowerCase() on the label and check includes() against the search lowercased
string so the filter short-circuits instead of throwing.
| function KbdGroup({ className, ...props }: React.ComponentProps<"div">) { | ||
| return ( | ||
| <kbd | ||
| data-slot="kbd-group" | ||
| className={cn("inline-flex items-center gap-1", className)} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type mismatch: props typed as div but renders kbd.
Line 18 declares props as React.ComponentProps<"div"> but the component renders a <kbd> element at line 20. This creates a type inconsistency where the props interface doesn't match the rendered element.
Apply this diff to fix the type mismatch:
-function KbdGroup({ className, ...props }: React.ComponentProps<"div">) {
+function KbdGroup({ className, ...props }: React.ComponentProps<"kbd">) {
return (
<kbd
data-slot="kbd-group"
className={cn("inline-flex items-center gap-1", className)}
{...props}
/>
)
}📝 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.
| function KbdGroup({ className, ...props }: React.ComponentProps<"div">) { | |
| return ( | |
| <kbd | |
| data-slot="kbd-group" | |
| className={cn("inline-flex items-center gap-1", className)} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| function KbdGroup({ className, ...props }: React.ComponentProps<"kbd">) { | |
| return ( | |
| <kbd | |
| data-slot="kbd-group" | |
| className={cn("inline-flex items-center gap-1", className)} | |
| {...props} | |
| /> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In components/ui/kbd.tsx around lines 18 to 26, the component props are typed as
React.ComponentProps<"div"> but the component renders a <kbd> element; change
the props type to React.ComponentProps<"kbd"> (or a compatible HTML attributes
type like React.HTMLAttributes<HTMLElement>) so the declared props match the
rendered element, and adjust any imports/types accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/editor/theme-preset-select.tsx (1)
179-179: Potential duplicate "default" preset in the list.This line has the same issue as
components/cmdk.tsxline 59. Ifpresetsalready contains a"default"key, the array will have"default"twice, causing duplicate entries in the theme selector.Apply the same fix suggested for
cmdk.tsx:- const presetNames = useMemo(() => ["default", ...Object.keys(presets)], [presets]); + const presetNames = useMemo(() => { + const keys = Object.keys(presets); + return keys.includes("default") ? keys : ["default", ...keys]; + }, [presets]);Or use a Set for deduplication:
- const presetNames = useMemo(() => ["default", ...Object.keys(presets)], [presets]); + const presetNames = useMemo(() => Array.from(new Set(["default", ...Object.keys(presets)])), [presets]);
♻️ Duplicate comments (4)
utils/search/constants/navigation.ts (1)
1-10: Past review concern is already addressed.The
LucideIcontype is correctly imported and used for theiconproperty, avoiding the implicit React dependency that was flagged in the previous review.utils/search/filter-presets.ts (1)
3-17: Past review concern is already addressed.The function correctly guards against missing labels with the ternary check on line 13, preventing runtime errors when
ThemePreset.labelis undefined.components/header.tsx (1)
57-65: Past review concern is already addressed.The Figma export button now includes proper accessibility attributes:
aria-hidden="true"on the icon, a screen-reader-only label, and visible text at thelgbreakpoint. This ensures the button is accessible at all viewport sizes.components/cmdk.tsx (1)
119-134: Past review concern is already addressed.The search button now includes proper accessibility attributes:
aria-label="Open search",aria-hidden="true"on the icon, and ansr-onlyspan. This ensures the button has an accessible name at all viewport sizes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
components/cmdk.tsx(1 hunks)components/editor/theme-preset-select.tsx(3 hunks)components/header.tsx(3 hunks)components/ui/kbd.tsx(1 hunks)utils/search/constants/navigation.ts(1 hunks)utils/search/filter-presets.ts(1 hunks)utils/search/is-theme-new.ts(1 hunks)utils/search/sort-themes.ts(1 hunks)utils/theme-preset-helper.ts(0 hunks)
💤 Files with no reviewable changes (1)
- utils/theme-preset-helper.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- components/ui/kbd.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: aa5hi5h
Repo: jnsahaj/tweakcn PR: 209
File: components/home/theme-hotkey-handler.tsx:175-253
Timestamp: 2025-08-06T05:17:30.477Z
Learning: In the tweakcn codebase, the KeyboardShortcutsOverlay component handles its own Ctrl+/ shortcut logic internally rather than having it managed by the ThemeHotKeyHandler component. This follows a pattern where components manage their own keyboard interactions instead of centralizing all shortcuts in one handler.
Learnt from: aa5hi5h
Repo: jnsahaj/tweakcn PR: 209
File: components/home/theme-hotkey-handler.tsx:175-253
Timestamp: 2025-08-06T05:17:30.477Z
Learning: In the tweakcn codebase, the KeyboardShortcutsOverlay component handles its own Ctrl+/ shortcut logic internally rather than having it managed by the ThemeHotKeyHandler component. This follows a pattern where components manage their own keyboard interactions.
🧬 Code graph analysis (6)
utils/search/is-theme-new.ts (1)
types/theme.ts (1)
ThemePreset(101-109)
utils/search/sort-themes.ts (1)
types/theme.ts (1)
ThemePreset(101-109)
components/editor/theme-preset-select.tsx (3)
utils/search/filter-presets.ts (1)
filterPresets(3-17)utils/search/sort-themes.ts (1)
sortThemes(3-13)components/search/theme-colors.tsx (1)
ThemeColors(10-20)
utils/search/filter-presets.ts (1)
types/theme.ts (1)
ThemePreset(101-109)
components/cmdk.tsx (8)
store/editor-store.ts (1)
useEditorStore(34-231)store/theme-preset-store.ts (1)
useThemePresetStore(17-74)lib/auth-client.ts (1)
authClient(5-7)utils/search/filter-presets.ts (1)
filterPresets(3-17)utils/search/sort-themes.ts (1)
sortThemes(3-13)utils/search/constants/navigation.ts (1)
NAVIGATION_ITEMS(12-55)components/search/theme-colors.tsx (1)
ThemeColors(10-20)utils/search/is-theme-new.ts (1)
isThemeNew(3-9)
components/header.tsx (4)
components/get-pro-cta.tsx (1)
GetProCTA(11-35)components/cmdk.tsx (1)
CmdK(28-243)components/social-link.tsx (1)
SocialLink(8-23)utils/format.ts (1)
formatCompactNumber(4-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (14)
utils/search/is-theme-new.ts (1)
3-9: LGTM!The function correctly handles the optional
createdAtfield and gracefully returnsfalsefor missing or invalid dates. The 5-day threshold is reasonable for flagging new themes.utils/search/constants/navigation.ts (1)
12-55: LGTM!The navigation items are well-structured with consistent properties and useful keywords for search filtering.
utils/search/sort-themes.ts (1)
3-13: LGTM!The sorting logic correctly prioritizes the "default" theme and alphabetically orders others by label (with a safe fallback to the theme name). The optional chaining properly handles missing presets or labels.
components/header.tsx (3)
18-18: LGTM!The CmdK component is correctly imported and integrated into the header.
30-36: LGTM!The responsive visibility adjustments improve the mobile experience by hiding less critical elements on smaller screens while keeping essential navigation visible.
38-56: LGTM!The responsive layout changes progressively reveal social links and separators at appropriate breakpoints (sm, md), creating a clean mobile-first experience.
components/cmdk.tsx (3)
28-116: LGTM!The component state management, filtering logic, keyboard handling, and callbacks are well-structured. The use of
useMemoanduseCallbackappropriately optimizes performance.
135-237: LGTM!The command palette UI is well-organized with clear grouping (Navigation, Saved Themes, Built-in Themes), proper conditional rendering, and good use of the ThemeColors component for visual previews. The "New" badge and checkmark for the current preset enhance usability.
245-254: LGTM!The footer provides a helpful hint about the Enter key functionality.
components/editor/theme-preset-select.tsx (5)
13-20: LGTM!The new imports centralize theme-related utilities and components, promoting code reuse and maintainability.
182-190: LGTM!The filtering and sorting logic now uses centralized utilities (
filterPresetsandsortThemes), improving code reuse and consistency across the application.
209-211: LGTM!The use of the
ThemeColorscomponent provides a consistent visual preview of the current theme.
278-305: LGTM!The saved themes section consistently uses
ThemeColorsfor previews andisThemeNewfor the "New" badge, providing a polished user experience.
324-354: LGTM!The built-in themes section mirrors the saved themes structure, maintaining UI consistency and using the same utility functions and components.
components/cmdk.tsx
Outdated
| [presets] | ||
| ); | ||
|
|
||
| const presetNames = useMemo(() => ["default", ...Object.keys(presets)], [presets]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential duplicate "default" preset in the list.
Line 59 prepends "default" to Object.keys(presets), but if presets already contains a "default" key (which is likely based on the store initialization), the resulting array will have "default" twice. This would cause duplicate entries in the theme list UI.
Consider deduplicating:
- const presetNames = useMemo(() => ["default", ...Object.keys(presets)], [presets]);
+ const presetNames = useMemo(() => {
+ const keys = Object.keys(presets);
+ return keys.includes("default") ? keys : ["default", ...keys];
+ }, [presets]);Alternatively, if "default" should always be listed even when not in presets, use a Set:
- const presetNames = useMemo(() => ["default", ...Object.keys(presets)], [presets]);
+ const presetNames = useMemo(() => Array.from(new Set(["default", ...Object.keys(presets)])), [presets]);Note: The same pattern appears in components/editor/theme-preset-select.tsx at line 179 and should be fixed consistently.
📝 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.
| const presetNames = useMemo(() => ["default", ...Object.keys(presets)], [presets]); | |
| const presetNames = useMemo(() => { | |
| const keys = Object.keys(presets); | |
| return keys.includes("default") ? keys : ["default", ...keys]; | |
| }, [presets]); |
🤖 Prompt for AI Agents
In components/cmdk.tsx around line 59 the code prepends "default" to
Object.keys(presets) which can create a duplicate if presets already contains
"default"; change this to produce a deduplicated ordered list (e.g., ensure
"default" appears once and other preset keys follow — either build a Set from
["default", ...Object.keys(presets)] or use ["default",
...Object.keys(presets).filter(k => k !== "default")]) so order is preserved and
duplicates removed; apply the same fix in
components/editor/theme-preset-select.tsx at line 179 for consistent behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
components/cmdk.tsx (3)
105-115: Consider clearing search state when the dialog closes.Currently,
searchis only cleared when an item is selected. If the user closes the dialog via Escape or by clicking outside, the search term persists and reappears when the dialog reopens. Consider adding an effect or callback to reset the search state whenever the dialog closes:+ useEffect(() => { + if (!open) { + setSearch(""); + } + }, [open]);This provides a fresh search experience each time the dialog opens, which is typically the expected behavior for command palettes.
180-203: Simplify CommandItem value and key props.Lines 183, 215: The
valueprop includes${presetName}-${index}, which is inconsistent with the navigation items (line 160) that use justitem.id. SincepresetNamesis already deduplicated (line 59), the index is unnecessary.While your custom search bypasses cmdk's internal filtering, cmdk may still use
valuefor keyboard navigation or other internal logic. Using the index makes the value semantically incorrect—it doesn't represent what the user sees or searches for.Apply this pattern consistently:
<CommandItem - key={`${presetName}-${index}`} - value={`${presetName}-${index}`} + key={presetName} + value={presetName} onSelect={() => onThemeSelect(presetName)} className="flex items-center gap-2" >Apply the same change to both Saved Themes (lines 181-183) and Built-in Themes (lines 213-215) sections.
Also applies to: 212-234
44-50: Consider performance implications for users with many saved themes.The effect loads all saved presets via
loadSavedPresets()without pagination or limits. For users with a large number of saved themes, this could impact initial load time and memory usage. While not a blocker for this PR, consider implementing pagination or virtual scrolling for the theme list if the app scales to support power users with hundreds of themes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/cmdk.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/cmdk.tsx (8)
store/editor-store.ts (1)
useEditorStore(34-231)store/theme-preset-store.ts (1)
useThemePresetStore(17-74)lib/auth-client.ts (1)
authClient(5-7)utils/search/filter-presets.ts (1)
filterPresets(3-17)utils/search/sort-themes.ts (1)
sortThemes(3-13)utils/search/constants/navigation.ts (1)
NAVIGATION_ITEMS(12-55)components/search/theme-colors.tsx (1)
ThemeColors(10-20)utils/search/is-theme-new.ts (1)
isThemeNew(3-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (6)
components/cmdk.tsx (6)
1-26: LGTM!Imports are well-organized and appropriate for a client-side command palette. The use of "use client" directive, React 19 hooks, and Next.js 15 App Router patterns are all correct.
28-50: LGTM!The state management and effects are well-structured. The preset loading logic correctly responds to authentication state changes, and the dependency array is appropriate for Zustand store selectors.
52-91: LGTM!The memoization and filtering logic is well-implemented. Line 59 correctly uses
Setto deduplicate preset names, addressing the previous review concern. The separation of saved vs. built-in themes is clear and the dependencies are appropriate.
93-103: LGTM!The keyboard shortcut implementation follows best practices with proper event prevention and cleanup.
119-134: Accessibility improvements successfully applied.The button now has proper accessibility attributes as suggested in the previous review, ensuring it's usable at all viewport sizes.
245-254: LGTM!The footer provides a helpful keyboard hint in a visually consistent manner.
Adding
cmd+kglobal search optionThis search bar exposes some navigation and theme toggling functionality
Re-arranging already existing search utility methods to be reusable
Also adding responsiveness for the editor navbar in the mobile screens
cmdk.mp4
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.