Enhance Desktop2 UI — Turbo Command, Sidebar Update, and Improved Search #1567
Conversation
📝 WalkthroughWalkthroughAdds Tauri npm scripts, new Tailwind color tokens, styling/layout tweaks across chat and sidebar, a refactored main body with a scrollable, reorderable tab strip and hotkey (mod+w), per-tab navigation history (goBack/goNext) in the Zustand store, and tests for tab history behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Body as Body (main/body/index)
participant Hotkey as useTabCloseHotkey
participant Tabs as TabsStore
participant Win as AppWindow (Tauri)
Note over Body,Hotkey: Register mod+w on mount
User->>Body: Press mod+w
Body->>Hotkey: onKey(mod+w)
Hotkey->>Tabs: getTabCount()
alt Multiple tabs
Hotkey->>Tabs: closeActiveTab()
else Single tab
Hotkey->>Win: getCurrentWebviewWindow()
Hotkey->>Win: close()
end
sequenceDiagram
autonumber
actor User
participant UI as Tab UI (main/body)
participant Tabs as TabsStore
participant History as TabHistory (store)
User->>UI: Click "New Tab" (+)
UI->>Tabs: openNew()
Tabs->>History: pushHistory(newTabSlot, initialEntry)
Tabs-->>UI: newTabId
UI->>UI: scrollActiveTabIntoView()
sequenceDiagram
autonumber
actor User
participant UI as Tab UI
participant Tabs as TabsStore
participant History as TabHistory
User->>UI: Click Back (or call goBack)
UI->>Tabs: goBack()
Tabs->>History: decrement currentIndex, get previousEntry
Tabs->>UI: select(previousEntry.tabId)
UI->>UI: render selected tab state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 2
🧹 Nitpick comments (1)
apps/desktop2/tailwind.config.ts (1)
13-18: Color tokens added for consistent theming.The new color palette provides a foundation for consistent styling across components. These tokens are already in use throughout the codebase.
Consider using semantic names like
backgroundPrimary,backgroundSecondary,textMuted,textSecondaryinstead ofcolor1-4for better self-documentation:colors: { - color1: "#FBFBFB", - color2: "#F4F4F4", - color3: "#8e8e8e", - color4: "#484848", + backgroundPrimary: "#FBFBFB", + backgroundSecondary: "#F4F4F4", + textMuted: "#8e8e8e", + textPrimary: "#484848", },This would require updating all usages across the codebase.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/desktop2/package.json(1 hunks)apps/desktop2/src/components/chat/body.tsx(1 hunks)apps/desktop2/src/components/chat/header.tsx(2 hunks)apps/desktop2/src/components/chat/input.tsx(1 hunks)apps/desktop2/src/components/main/body/index.tsx(5 hunks)apps/desktop2/src/components/main/body/search.tsx(4 hunks)apps/desktop2/src/components/main/body/shared.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/index.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/profile/index.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/search/index.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/timeline.tsx(1 hunks)apps/desktop2/src/routes/app/main/_layout.index.tsx(2 hunks)apps/desktop2/tailwind.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/tailwind.config.tsapps/desktop2/src/components/chat/header.tsxapps/desktop2/src/components/chat/input.tsxapps/desktop2/src/components/main/sidebar/search/index.tsxapps/desktop2/src/components/main/sidebar/index.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/sidebar/profile/index.tsxapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/chat/body.tsxapps/desktop2/src/components/main/body/search.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/src/routes/app/main/_layout.index.tsx
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/components/chat/header.tsxapps/desktop2/src/components/chat/input.tsxapps/desktop2/src/components/main/sidebar/search/index.tsxapps/desktop2/src/components/main/sidebar/index.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/sidebar/profile/index.tsxapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/chat/body.tsxapps/desktop2/src/components/main/body/search.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/src/routes/app/main/_layout.index.tsx
🧬 Code graph analysis (4)
apps/desktop2/src/components/main/sidebar/search/index.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/index.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/search.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/routes/app/main/_layout.index.tsx (1)
apps/desktop2/src/components/main/sidebar/index.tsx (1)
LeftSidebar(10-41)
⏰ 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). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (19)
apps/desktop2/src/components/main/sidebar/search/index.tsx (1)
13-13: LGTM! Color token usage is consistent.The change from
bg-gray-50tobg-color1aligns with the new color palette defined intailwind.config.ts.apps/desktop2/src/components/main/sidebar/timeline.tsx (2)
20-20: LGTM! Consistent color token adoption.The background color update to
bg-color1and the addition ofrounded-lgalign with the broader UI refresh.
23-23: Cleaner styling with background removed.Removing the background from the sticky date header while preserving positioning is appropriate.
apps/desktop2/package.json (1)
13-14: Convenient Tauri shortcuts added.The new scripts provide ergonomic aliases for common Tauri development and build workflows.
apps/desktop2/src/components/chat/input.tsx (1)
72-74: Refined input styling for better panel integration.The border radius change to
rounded-mdand margin removal inRightPanelOpenmode create a cleaner visual connection with adjacent panels.apps/desktop2/src/components/chat/body.tsx (1)
38-38: Consistent margin removal for panel alignment.Removing
mr-1inRightPanelOpenmode aligns with the margin adjustments ininput.tsx, ensuring proper panel spacing.apps/desktop2/src/routes/app/main/_layout.index.tsx (3)
19-19: Improved outer container spacing.Adding
p-1 gap-1creates consistent padding and gaps throughout the layout.
25-25: Panel size persistence enabled.The
autoSaveId="main-chat"prop allows the resizable panel to remember user's size preferences across sessions, improving UX.
33-33: Balanced chat panel spacing.The
pl-1class on the chat panel creates visual balance with the overall layout spacing.apps/desktop2/src/components/main/sidebar/profile/index.tsx (2)
67-67: Consistent background color token.Updated to
bg-color1aligning with the new color palette.
105-105: Expanded state styling updated.The color token usage in the expanded state maintains consistency with the overall color scheme.
apps/desktop2/src/components/chat/header.tsx (1)
27-29: LGTM! Layout adjustments align with PR objectives.The padding and height changes improve the visual consistency of the header across different chat modes.
apps/desktop2/src/components/main/sidebar/index.tsx (1)
17-38: LGTM! Layout updates align with PR objectives.The sidebar width increase to 280px and the addition of gap spacing improve the visual hierarchy and match the stated PR goal of setting the left sidebar width to 280px. The color token migration to
bg-color1ensures consistency with the new design system.apps/desktop2/src/components/main/body/index.tsx (2)
29-29: LGTM! Layout adjustment improves spacing consistency.Replacing
p-1withgap-1provides better visual separation between child elements.
133-139: LGTM! Color token migration improves theming consistency.The migration from
text-gray-800totext-color3aligns with the new color token system introduced in the PR.apps/desktop2/src/components/main/body/search.tsx (3)
11-11: LGTM! Focus state management correctly syncs local and context state.The introduction of local
isFocusedstate combined with handlers that call bothsetIsFocusedand context callbacks ensures proper synchronization between component-level and app-level focus state.Also applies to: 53-61
64-69: LGTM! Animated width expansion improves UX.The dynamic width change from 200px to 240px with a 300ms transition provides smooth visual feedback when the search input receives focus.
84-87: LGTM! Border styling enhances focus visibility.The border changes from
border-0toborder border-transparentwithfocus:border-blackprovide clearer visual feedback on focus without breaking the unfocused appearance.apps/desktop2/src/components/main/body/shared.tsx (1)
26-27: LGTM! Color token migration establishes consistent theming.The migration from hardcoded colors to design tokens (
color1,color3,color4) aligns with the broader theming system introduced in this PR. This provides a single source of truth for colors and facilitates future theme customization.Also applies to: 44-45
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop2/src/components/main/body/index.tsx (1)
88-113: Add onClick handlers for navigation buttons.The arrow navigation buttons render without functional
onClickhandlers, making them non-interactive placeholders. This issue was previously flagged.Implement navigation logic or add TODO comments:
<button + type="button" + onClick={() => { + // TODO: Implement backward navigation through tab history + }} className={cn([ "flex items-center justify-center", "h-full", "px-1.5", "rounded-lg", "hover:bg-gray-50", "transition-colors", "group", ])} > <ArrowLeftIcon className="h-4 w-4 text-color3 cursor-pointer group-hover:text-black" /> </button>Apply similar changes to the right arrow button (lines 101-113).
Do you want me to help implement browser-style forward/backward navigation logic, or should this be tracked as a follow-up task?
🧹 Nitpick comments (1)
apps/desktop2/src/components/main/body/search.tsx (1)
20-51: Clarify purpose of console.log statements in keyboard handlers.The
down,up, andenterhotkey handlers contain onlyconsole.logstatements without implementing navigation logic.Are these debug statements that should be removed, or placeholders for pending functionality? If the latter, consider replacing them with TODO comments:
useHotkeys( "down", (event) => { if (document.activeElement === inputRef.current) { event.preventDefault(); - console.log("down"); + // TODO: Navigate to next search result } }, { enableOnFormTags: true }, );Do you want me to help implement the keyboard navigation logic for search results, or should this be tracked as a follow-up task?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop2/src/components/main/body/index.tsx(4 hunks)apps/desktop2/src/components/main/body/search.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/main/body/search.tsx
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/main/body/search.tsx
🔇 Additional comments (4)
apps/desktop2/src/components/main/body/search.tsx (2)
53-61: Focus state management looks good.The local
isFocusedstate correctly synchronizes with context callbacks, enabling smooth UI transitions while maintaining proper focus event propagation.
64-87: Dynamic width transition enhances UX.The wrapper's animated width change (200px → 240px on focus) provides visual feedback, and the updated input styling with border transitions follows modern design patterns.
apps/desktop2/src/components/main/body/index.tsx (2)
58-66: Auto-scroll to new tabs improves discoverability.The
setTimeout(() => {...}, 0)ensures the DOM updates before scrolling, and the smooth scroll behavior provides a polished user experience when creating tabs.
117-146: Scrollable tabs structure is well-organized.The ref-based scrolling container integrates cleanly with the Reorder.Group, and the hidden scrollbar styling maintains a clean aesthetic while preserving horizontal scroll functionality.
ComputelessComputer
force-pushed
the
jj-branch-19
branch
from
e33c487 to
a2ac5c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop2/src/components/main/body/index.tsx (1)
86-114: Navigation buttons still lack onClick handlers.As previously noted in earlier reviews, these navigation buttons render without functional onClick handlers, making them non-interactive. The PR objectives mention "Added forward/backward navigation buttons," but the navigation logic remains unimplemented.
If navigation logic is deferred, add explicit TODO comments:
<button + // TODO: Implement backward navigation logic className={cn([ "flex items-center justify-center", "h-full",Additionally, consider adding explicit
type="button"to prevent unintended form submissions:<button + type="button" className={cn([
🧹 Nitpick comments (2)
apps/desktop2/src/components/main/body/index.tsx (2)
58-66: ConsiderrequestAnimationFramefor more reliable timing.The
setTimeout(0)approach works but may occasionally fire before DOM updates complete. For more predictable behavior, consider usingrequestAnimationFrame:- setTimeout(() => { + requestAnimationFrame(() => { if (tabsScrollContainerRef.current) { tabsScrollContainerRef.current.scrollTo({ left: tabsScrollContainerRef.current.scrollWidth, behavior: "smooth", }); } - }, 0); + });
148-161: LGTM with minor accessibility suggestion.The plus button is properly implemented with the correct onClick handler and styling.
For enhanced accessibility, consider adding explicit
type="button":<button + type="button" onClick={handleNewNote} className={cn([
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop2/src/components/main/body/index.tsx(4 hunks)apps/desktop2/src/components/main/body/search.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop2/src/components/main/body/search.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/components/main/body/index.tsx
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/components/main/body/index.tsx
🧬 Code graph analysis (1)
apps/desktop2/src/components/main/body/index.tsx (3)
packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop2/src/store/zustand/tabs.ts (1)
uniqueIdfromTab(196-215)apps/desktop2/src/components/main/body/search.tsx (1)
Search(8-107)
⏰ 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). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (5)
apps/desktop2/src/components/main/body/index.tsx (5)
2-2: LGTM!Import additions are correct and all used within the component.
Also applies to: 4-4
44-44: LGTM!The ref is properly typed and used for controlling the scrollable tabs container.
76-84: LGTM!The left sidebar toggle is properly structured with appropriate z-index layering and follows the coding guidelines for
cnusage.
116-146: LGTM!The scrollable tabs container is well-structured with proper ref assignment, hidden scrollbars, and correct
Reorder.Groupnesting. Thecnusage follows coding guidelines.
163-164: LGTM!The Search component is properly integrated into the header layout and includes focus states with animated width transitions.
ComputelessComputer
force-pushed
the
jj-branch-19
branch
from
d4200a6 to
a2eea39
Compare
ComputelessComputer
force-pushed
the
jj-branch-19
branch
from
7b88086 to
018f59e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/desktop2/src/components/main/body/index.tsx (1)
106-131: Add onClick handlers for navigation buttons.These buttons still lack click handlers, making them non-functional UI placeholders.
If navigation logic is not ready, add explicit TODO comments and ensure buttons have
type="button":<button + type="button" + // TODO: Implement backward navigation through tab history className={cn([ "flex items-center justify-center", "h-full", "px-1.5", "rounded-lg", "hover:bg-gray-50", "transition-colors", "group", ])} > <ArrowLeftIcon className="h-4 w-4 text-color3 cursor-pointer group-hover:text-black" /> </button> <button + type="button" + // TODO: Implement forward navigation through tab history className={cn([Do you want me to help design the navigation state management (e.g., maintaining a history stack per tab or a global navigation history)?
🧹 Nitpick comments (3)
apps/desktop2/tailwind.config.ts (1)
13-18: Consider semantic color token names for better maintainability.The color tokens
color1,color2,color3, andcolor4are not semantically named. Using descriptive names likebackground-primary,background-secondary,text-muted, andtext-darkwould improve code readability and make future maintenance easier.Apply this diff to use semantic naming:
colors: { - color1: "#FBFBFB", - color2: "#F4F4F4", - color3: "#8e8e8e", - color4: "#484848", + "bg-primary": "#FBFBFB", + "bg-secondary": "#F4F4F4", + "text-muted": "#8e8e8e", + "text-dark": "#484848", },apps/desktop2/src/components/main/body/index.tsx (2)
33-38: Remove explanatory comments.These comments describe what the code does rather than why. The control flow is self-evident from reading the conditional logic.
Apply this diff to remove the comments:
- if (tabs.length > 1) { - // If more than one tab, close the current tab + if (tabs.length > 1) { if (currentTab) { close(currentTab); } - } else if (tabs.length === 1) { - // If only one tab left, close the window + } else if (tabs.length === 1) { const appWindow = getCurrentWebviewWindow(); await appWindow.close(); }
74-87: Replace setTimeout with requestAnimationFrame in scrollTabsToEnd to ensure the scroll runs after the new tab has rendered.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/desktop2/package.json(1 hunks)apps/desktop2/src/components/chat/body.tsx(1 hunks)apps/desktop2/src/components/chat/header.tsx(2 hunks)apps/desktop2/src/components/chat/input.tsx(1 hunks)apps/desktop2/src/components/main/body/index.tsx(4 hunks)apps/desktop2/src/components/main/body/search.tsx(4 hunks)apps/desktop2/src/components/main/body/shared.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/index.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/profile/index.tsx(5 hunks)apps/desktop2/src/components/main/sidebar/search/index.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/timeline.tsx(1 hunks)apps/desktop2/src/routes/app/main/_layout.index.tsx(2 hunks)apps/desktop2/src/utils/tabs-scroll.ts(1 hunks)apps/desktop2/tailwind.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/desktop2/src/components/main/sidebar/profile/index.tsx
- apps/desktop2/src/components/chat/body.tsx
- apps/desktop2/package.json
- apps/desktop2/src/components/chat/header.tsx
- apps/desktop2/src/components/main/sidebar/search/index.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/utils/tabs-scroll.tsapps/desktop2/src/components/main/sidebar/index.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/tailwind.config.tsapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/chat/input.tsxapps/desktop2/src/routes/app/main/_layout.index.tsxapps/desktop2/src/components/main/body/search.tsxapps/desktop2/src/components/main/body/index.tsx
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/components/main/sidebar/index.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/chat/input.tsxapps/desktop2/src/routes/app/main/_layout.index.tsxapps/desktop2/src/components/main/body/search.tsxapps/desktop2/src/components/main/body/index.tsx
🧬 Code graph analysis (3)
apps/desktop2/src/routes/app/main/_layout.index.tsx (1)
apps/desktop2/src/components/main/sidebar/index.tsx (1)
LeftSidebar(10-41)
apps/desktop2/src/components/main/body/search.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/index.tsx (2)
apps/desktop2/src/store/zustand/tabs.ts (2)
useTabs(31-121)uniqueIdfromTab(196-215)apps/desktop2/src/utils/tabs-scroll.ts (2)
setTabsScrollContainer(7-9)scrollTabsToEnd(11-20)
⏰ 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). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (10)
apps/desktop2/src/components/main/body/search.tsx (2)
53-61: LGTM!The focus handlers correctly combine local state management with context callbacks, enabling the animated width transition while maintaining integration with the search context.
64-88: LGTM!The animated width transition on focus provides good UX feedback, and the implementation correctly uses the
cnutility with logical grouping of Tailwind classes.apps/desktop2/src/components/main/sidebar/timeline.tsx (2)
20-20: LGTM!The background color update to
bg-color1is consistent with the new color token system introduced in the Tailwind configuration.
23-23: LGTM!Removing the background from the sticky header simplifies the styling while maintaining the sticky positioning behavior.
apps/desktop2/src/components/chat/input.tsx (1)
72-74: LGTM!The border radius change to
rounded-mdand simplified RightPanelOpen styling align with the overall UI consistency improvements in this PR.apps/desktop2/src/routes/app/main/_layout.index.tsx (2)
19-19: LGTM!Adding padding and gap to the outer container improves visual spacing and consistency across the layout.
25-25: LGTM!The
autoSaveIdprop enables persistence of the user's panel size preferences across sessions, improving UX.apps/desktop2/src/components/main/sidebar/index.tsx (1)
17-17: LGTM!The width increase to 280px and addition of gap-1 align with the PR objectives for sidebar layout improvements.
apps/desktop2/src/components/main/body/index.tsx (2)
27-45: LGTM! Window close behavior is well-implemented.The hotkey handler correctly distinguishes between closing a tab (when multiple exist) and closing the entire window (when only one tab remains). The
enableOnFormTags: trueoption is appropriate here since Cmd/Ctrl+W is a standard close action that users expect to work even in form fields.
143-143: Verify Reorder.Group key behavior.The key
{leftsidebar.expanded ? "expanded" : "collapsed"}forces React to remount the entireReorder.Groupand all its children whenever the sidebar expands or collapses. This will:
- Reset internal animation state in Framer Motion
- Lose any in-flight drag operations
- Potentially cause a brief flicker
Confirm this remounting is intentional (e.g., to recalculate positions after sidebar width changes). If not, consider removing the key or using a stable identifier.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop2/src/components/main/body/index.tsx (1)
81-101: Nav buttons wired correctly; add small a11y tweaksonClick handlers resolve the earlier “missing navigation logic” comment. Add aria-labels and type="button" to improve accessibility and avoid implicit submits.
Apply this diff:
- <button - onClick={goBack} + <button + type="button" + aria-label="Go back" + onClick={goBack} disabled={!canGoBack} className={cn([ "flex items-center justify-center", "h-full", "px-1.5", "rounded-lg", "transition-colors", canGoBack && ["hover:bg-gray-50", "group"], !canGoBack && "cursor-not-allowed", ])} > @@ - <button - onClick={goNext} + <button + type="button" + aria-label="Go forward" + onClick={goNext} disabled={!canGoNext} className={cn([ "flex items-center justify-center", "h-full", "px-1.5", "rounded-lg", "transition-colors", canGoNext && ["hover:bg-gray-50", "group"], !canGoNext && "cursor-not-allowed", ])} > @@ - <button - onClick={handleNewNote} + <button + type="button" + aria-label="New note" + onClick={handleNewNote} className={cn([ "flex items-center justify-center", "h-full", "px-1.5", "rounded-lg", "bg-white hover:bg-gray-50", "transition-colors", "shrink-0", ])} >Nice use of cn([...]) and grouped class arrays.
Also applies to: 102-122, 157-171
🧹 Nitpick comments (3)
apps/desktop2/src/store/zustand/tabs.test.ts (1)
4-151: Good coverage for back/forward and forward-history truncationScenarios match the store semantics and validate flags well. Consider adding tests for:
- select() switching tabs preserves active-slot history flags.
- Closing the last tab resets flags/history to a clean state.
apps/desktop2/src/store/zustand/tabs.ts (2)
166-168: Clear history when the last tab closesLeaving history populated when tabs become empty retains stale state and may confuse subsequent sessions.
Apply this diff:
- if (remainingTabs.length === 0) { - return set({ tabs: [], currentTab: null, canGoBack: false, canGoNext: false }); - } + if (remainingTabs.length === 0) { + return set({ tabs: [], currentTab: null, history: new Map(), canGoBack: false, canGoNext: false }); + }
90-94: Add upper-bound guard when updating history current entryRare, but safer to ensure currentIndex is within stack bounds before assignment.
Apply this diff:
- if (existing && existing.currentIndex >= 0) { + if (existing && existing.currentIndex >= 0 && existing.currentIndex < existing.stack.length) { const newStack = [...existing.stack]; newStack[existing.currentIndex] = tab; newHistory.set(slotId, { ...existing, stack: newStack }); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop2/src/components/main/body/index.tsx(5 hunks)apps/desktop2/src/store/zustand/tabs.test.ts(1 hunks)apps/desktop2/src/store/zustand/tabs.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/store/zustand/tabs.tsapps/desktop2/src/store/zustand/tabs.test.tsapps/desktop2/src/components/main/body/index.tsx
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/components/main/body/index.tsx
🧬 Code graph analysis (2)
apps/desktop2/src/store/zustand/tabs.test.ts (1)
apps/desktop2/src/store/zustand/tabs.ts (2)
useTabs(99-271)Tab(322-322)
apps/desktop2/src/components/main/body/index.tsx (1)
apps/desktop2/src/store/zustand/tabs.ts (3)
useTabs(99-271)uniqueIdfromTab(346-365)Tab(322-322)
⏰ 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). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (3)
apps/desktop2/src/store/zustand/tabs.ts (1)
14-17: Note: Map in Zustand state isn’t serializableIf this store is ever persisted, Map won’t serialize by default. Ensure this slice is excluded from persistence or provide a (de)serialization transform.
Do you persist this store slice? If yes, I can draft a small transform to serialize history to arrays and back.
apps/desktop2/src/components/main/body/index.tsx (2)
125-133: Tauri drag-region may block tab clicks/drag-reorderdata-tauri-drag-region on the tabs scroll container can intercept clicks/drag within child controls (tab selection/reorder). Prefer restricting the drag region to non-interactive zones or marking interactive areas as no-drag.
Consider either:
- Remove data-tauri-drag-region from the scroll container, or
- Add data-tauri-drag-region="no-drag" to interactive wrappers (e.g., Reorder.Group/Reorder.Item or TabItem root) so clicks/drag reorder work reliably.
If desired, I can propose a concrete diff after you confirm desired drag behavior area.
Also applies to: 133-140
233-251: Hotkey behavior looks goodmod+w closes the active tab or the window when only one remains. No extra error handling added; deps array is correct.
| select: (tab) => { | ||
| const { tabs } = get(); | ||
| const { tabs, history } = get(); | ||
| const nextTabs = tabs.map((t) => ({ ...t, active: isSameTab(t, tab) })); | ||
| set({ tabs: nextTabs, currentTab: tab }); | ||
| const flags = computeHistoryFlags(history, tab); | ||
| set({ tabs: nextTabs, currentTab: tab, ...flags }); | ||
| }, |
There was a problem hiding this comment.
Bug: select() sets currentTab to a stale (inactive) object and computes flags on wrong slot
select() should set currentTab to the newly active tab instance and compute flags using that. Current code can leave canGoBack/canGoNext incorrect and break goBack/goNext lookups.
Apply this diff:
select: (tab) => {
- const { tabs, history } = get();
- const nextTabs = tabs.map((t) => ({ ...t, active: isSameTab(t, tab) }));
- const flags = computeHistoryFlags(history, tab);
- set({ tabs: nextTabs, currentTab: tab, ...flags });
+ const { tabs, history } = get();
+ const nextTabs = tabs.map((t) => ({ ...t, active: isSameTab(t, tab) }));
+ const nextCurrentTab = nextTabs.find((t) => t.active) || null;
+ const flags = computeHistoryFlags(history, nextCurrentTab);
+ set({ tabs: nextTabs, currentTab: nextCurrentTab, ...flags });
},📝 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.
| select: (tab) => { | |
| const { tabs } = get(); | |
| const { tabs, history } = get(); | |
| const nextTabs = tabs.map((t) => ({ ...t, active: isSameTab(t, tab) })); | |
| set({ tabs: nextTabs, currentTab: tab }); | |
| const flags = computeHistoryFlags(history, tab); | |
| set({ tabs: nextTabs, currentTab: tab, ...flags }); | |
| }, | |
| select: (tab) => { | |
| const { tabs, history } = get(); | |
| const nextTabs = tabs.map((t) => ({ ...t, active: isSameTab(t, tab) })); | |
| const nextCurrentTab = nextTabs.find((t) => t.active) || null; | |
| const flags = computeHistoryFlags(history, nextCurrentTab); | |
| set({ tabs: nextTabs, currentTab: nextCurrentTab, ...flags }); | |
| }, |
🤖 Prompt for AI Agents
In apps/desktop2/src/store/zustand/tabs.ts around lines 156 to 161, select()
currently sets currentTab to the passed-in tab and computes history flags using
that stale object; instead, after mapping nextTabs, find the newly created
active tab instance (e.g. nextTabs.find(t => t.active)) and use that instance
when calling computeHistoryFlags and when setting currentTab so
canGoBack/canGoNext and subsequent goBack/goNext lookups use the fresh, active
object; fall back to the original tab if the find returns undefined.
• Added turbo command to desktop2
• Implemented pre-defined colors
• Set left sidebar width to 280px
• Updated tab area UI
• Added forward/backward navigation buttons
• Improved search functionality