ui changes #1575
ui changes #1575
Conversation
Implement custom key handling to prevent default browser shortcuts like Cmd/Ctrl+W, Cmd/Ctrl+N, and Cmd/Ctrl+T from interfering with the editor's functionality. This ensures a more consistent editing experience by blocking these common browser navigation shortcuts.
Implement new hotkeys for creating and closing tabs with mod+n and mod+t. Add middle-click tab closing and update tab styling for better usability. Enhance hotkey behavior to work with form and content editable elements.
📝 WalkthroughWalkthroughAdds onMouseDown forwarding to InteractiveButton; introduces mod+n/mod+t new-tab hotkeys; restructures session headers (InnerHeader, SessionMetadata, breadcrumbs, dropdowns); adds TimelineView sidebar and MeetingMetadataChip UI; several presentational updates and small behavioral tweaks across desktop2 and UI packages. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Tab as InteractiveButton / TabItem
participant Handler as Tab MouseDown Handler
participant Store as Tabs Store
participant UI as Tab List
User->>Tab: middle-click
Tab->>Handler: onMouseDown callback
Handler->>Handler: if button === 1: preventDefault & stopPropagation
Handler->>Store: handleCloseThis()
Store->>UI: update tabs
UI->>User: re-render without closed tab
sequenceDiagram
autonumber
participant User
participant App as App Window / Editor
participant Hotkeys as useNewTabHotkeys
participant Store as Persisted Store
participant Tabs as Tab Manager
User->>App: Press Mod+N or Mod+T
App->>Hotkeys: keydown event
Hotkeys->>Hotkeys: intercept & prevent default
Hotkeys->>Store: create new session row (persist)
Hotkeys->>Tabs: open new "sessions" tab (state: { editor: "raw" })
Tabs->>App: render new session tab
sequenceDiagram
autonumber
participant Timeline as TimelineView
participant Hooks as useTimelineData / useTimelineScroll
participant Store as Persisted Store
participant UI as Buckets / TodayIndicator
Timeline->>Hooks: init
Hooks->>Store: query events & sessions
Store-->>Hooks: timeline items
Hooks->>UI: build buckets & compute current time position
UI->>User: render buckets and CurrentTimeIndicator
User->>UI: click "Go to Today"
UI->>Hooks: scrollToToday -> update scroll position
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ 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)
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
88-100: Usecninstead ofclsxfor consistency.The coding guidelines specify using the
cnutility for Tailwind className composition in this path pattern. The file already imports and usescnat lines 43-46, but here usesclsx.As per coding guidelines
Apply this diff:
- className={clsx([ + className={cn([ "group", "relative", "absolute left-1/2 transform -translate-x-1/2", "bg-white hover:bg-gray-50", "border border-gray-200", "rounded-full shadow-lg", "text-gray-700 z-40", isScrolledPastToday ? "top-2" : "bottom-2", isScrolledPastToday ? "shadow-[0_-2px_8px_rgba(248,113,113,0.3)] hover:shadow-[0_-2px_12px_rgba(239,68,68,0.4)]" : "shadow-[0_2px_8px_rgba(248,113,113,0.3)] hover:shadow-[0_2px_12px_rgba(239,68,68,0.4)]", ])}apps/desktop2/src/components/main/body/sessions/index.tsx (1)
363-391: Initialize title when creating a new session via hotkeys to avoid controlled/uncontrolled input warnings.Current setRow omits title, while click path sets title: "". TitleInput uses value={title}, which can be undefined initially.
- persistedStore?.setRow("sessions", sessionId, { user_id, created_at: new Date().toISOString() }); + persistedStore?.setRow("sessions", sessionId, { + user_id, + created_at: new Date().toISOString(), + title: "", + });apps/desktop2/src/components/main/body/index.tsx (1)
125-132: Tabs container sizing regression: w-fit prevents flex growth and reliable scrolling.With w-fit, the tabs row grows with content and can squeeze the Search/“+” section; overflow-x-auto won’t help because the container isn’t constrained. Restore flex-1 + min-w-0 so tabs consume remaining space and scroll horizontally.
- "w-fit overflow-x-auto overflow-y-hidden h-full", + "flex-1 min-w-0 overflow-x-auto overflow-y-hidden h-full",
🧹 Nitpick comments (16)
packages/ui/src/components/block/participants-chip.tsx (2)
285-285: Remove redundant hover color class.The hover state specifies the same color as the default state (
text-color4), making it unnecessary.Apply this diff:
- className="text-color4 hover:text-color4 transition-colors flex-shrink-0" + className="text-color4 flex-shrink-0"
314-315: Remove redundant color class from nested span.The nested span specifies
text-color4, but it already inherits this color from its parent span, making the class unnecessary.Apply this diff:
- <span className="flex items-center gap-1 font-medium text-color4"> - Create <span className="text-color4">"{value.trim()}"</span> + <span className="flex items-center gap-1 font-medium text-color4"> + Create <span>"{value.trim()}"</span>apps/desktop2/src/components/main/body/sessions/note-input/raw.tsx (1)
68-72: Prefer color tokens over hex values for consistency.Other components in this PR use color tokens like
text-color4andtext-color3(e.g., event-chip.tsx). Using the hex value#e5e5e5here breaks consistency and makes theme maintenance harder.Apply this diff if
#e5e5e5maps totext-color4:- <span className="text-[#e5e5e5]"> + <span className="text-color4"> Take notes or press <kbd>/</kbd> for commands. </span> <div className={cn("flex flex-row items-center gap-1", isNarrow && "hidden")}> - <span className="text-[#e5e5e5]">You can also upload/drop an</span> + <span className="text-color4">You can also upload/drop an</span>Or use
text-color3if that's the appropriate semantic token for placeholder text.apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
232-242: Complete the delete implementation.Line 238 contains a
console.logstub instead of actual delete logic. This should be implemented before merging.Do you want me to generate a complete delete handler implementation, or would you like to open a tracking issue for this?
Example implementation:
+ const handleDelete = useCallback(() => { + if (!store) { + return; + } + if (item.type === "event") { + store.delRow("events", item.id); + } else { + store.delRow("sessions", item.id); + } + }, [store, item.id, item.type]); + const contextMenu = ( <> <ContextMenuItem onClick={() => handleCmdClick()}> <ExternalLink className="w-4 h-4 mr-2" /> New Tab </ContextMenuItem> - <ContextMenuItem className="text-red-500" onClick={() => console.log("Delete:", item.type, item.id)}> + <ContextMenuItem className="text-red-500" onClick={handleDelete}> <Trash2 className="w-4 h-4 mr-2 text-red-500" /> Delete </ContextMenuItem> </> );apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
56-70: Add tab semantics (role/aria) to improve a11y and tooling.Make the container a tablist and each button a tab with aria-selected.
- <div className="flex gap-4"> + <div role="tablist" className="flex gap-4"> {editorTabs.map((view) => ( <button key={view} onClick={() => handleTabChange(view)} + role="tab" + aria-selected={currentTab === view} + type="button" className={cn([ "relative py-2 text-xs font-medium transition-all duration-200 border-b-2 -mb-px", currentTab === view ? ["text-neutral-900", "border-neutral-900"] : ["text-neutral-600", "border-transparent", "hover:text-neutral-800"], ])} >apps/desktop2/src/components/main/body/sessions/index.tsx (2)
73-78: Add an accessible label to the sidebar toggle.- <Button size="icon" variant="ghost" onClick={() => leftsidebar.setExpanded(true)}> + <Button size="icon" variant="ghost" aria-label="Open sidebar" onClick={() => leftsidebar.setExpanded(true)}> <PanelLeftOpenIcon size={16} /> </Button>
163-178: Add an accessible label to the new note button.- <button + <button + aria-label="New note" onClick={handleNewNote} className={cn([ "flex items-center justify-center h-full px-1.5 shrink-0", "rounded-lg bg-white hover:bg-gray-50 transition-colors cursor-pointer", ])} >apps/desktop2/src/components/main/body/shared.tsx (3)
57-62: Use cn([...]) instead of clsx([...]) in apps/desktop2 per guidelines; keeps Tailwind merges consistent.Switch to cn and keep array style.
-import { clsx } from "clsx"; +import { cn } from "@hypr/ui/lib/utils"; ... - className={clsx([ + className={cn([ "flex items-center gap-2 cursor-pointer group", "w-48 h-full px-2", "bg-color1 rounded-lg border", active ? "text-black border-black" : "text-color3 border-transparent", ])} ... - className={clsx([ + className={cn([ "flex-shrink-0 transition-opacity", active ? "opacity-100 text-color4" : "opacity-0 group-hover:opacity-100 text-color3", ])}As per coding guidelines.
Also applies to: 73-78
59-59: Fixed width may truncate long titles; consider responsive min/max with truncation.- "w-48 h-full px-2", + "min-w-[100px] max-w-[200px] h-full px-2",
69-81: Add aria-label to the close button for screen readers.- <button + <button + aria-label="Close tab" onClick={(e) => { e.stopPropagation(); handleCloseThis(); }}apps/desktop2/src/components/interactive-button.tsx (1)
8-13: Solid: onMouseDown passthrough added correctly; set default type="button" when rendering a .Prevents accidental form submits if used inside forms.
<Element onClick={handleClick} onMouseDown={onMouseDown} className={className} - disabled={!asChild ? disabled : undefined} + disabled={!asChild ? disabled : undefined} + {...(!asChild ? { type: "button" } : {})} > ... <Element onClick={handleClick} onMouseDown={onMouseDown} className={className} - disabled={!asChild ? disabled : undefined} + disabled={!asChild ? disabled : undefined} + {...(!asChild ? { type: "button" } : {})} >Also applies to: 19-24, 45-53, 59-67
apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (2)
82-86: Add aria-label to the overflow trigger for a11y.- <DropdownMenuTrigger asChild> - <Button size="icon" variant="ghost"> + <DropdownMenuTrigger asChild> + <Button size="icon" variant="ghost" aria-label="More actions"> <MoreHorizontalIcon size={16} /> </Button> </DropdownMenuTrigger>
31-37: Optional: move static folders list outside the component to avoid re-allocations.apps/desktop2/src/components/main/body/sessions/inner-header.tsx (1)
48-53: Align with project’s cn([...]) array convention and add tab roles/aria.Switch cn calls to array form and add tab semantics; also set type="button".
- return ( - <div className="relative"> - <div className="bg-white px-2"> - <div className="flex"> - <div className="flex border-b border-neutral-100 w-full"> + return ( + <div className="relative"> + <div className="bg-white px-2"> + <div className="flex"> + <div role="tablist" className="flex border-b border-neutral-100 w-full"> {shouldShowEnhancedTab && ( <button onClick={() => handleTabChange("enhanced")} - className={cn( - "relative px-2 py-2 text-xs pl-1 font-medium transition-all duration-200 border-b-2 -mb-px flex items-center gap-1.5", - currentTab === "enhanced" - ? "text-neutral-900 border-neutral-900" - : "text-neutral-600 border-transparent hover:text-neutral-800", - )} + role="tab" + aria-selected={currentTab === "enhanced"} + type="button" + className={cn([ + "relative px-2 py-2 text-xs pl-1 font-medium transition-all duration-200 border-b-2 -mb-px flex items-center gap-1.5", + currentTab === "enhanced" + ? ["text-neutral-900", "border-neutral-900"] + : ["text-neutral-600", "border-transparent", "hover:text-neutral-800"], + ])} > Summary </button> )} <button onClick={() => handleTabChange("raw")} - className={cn( - "relative py-2 text-xs font-medium transition-all duration-200 border-b-2 -mb-px flex items-center gap-1.5", - shouldShowEnhancedTab ? "pl-3 px-4" : "pl-1 px-2", - currentTab === "raw" - ? "text-neutral-900 border-neutral-900" - : "text-neutral-600 border-transparent hover:text-neutral-800", - )} + role="tab" + aria-selected={currentTab === "raw"} + type="button" + className={cn([ + "relative py-2 text-xs font-medium transition-all duration-200 border-b-2 -mb-px flex items-center gap-1.5", + shouldShowEnhancedTab ? ["pl-3", "px-4"] : ["pl-1", "px-2"], + currentTab === "raw" + ? ["text-neutral-900", "border-neutral-900"] + : ["text-neutral-600", "border-transparent", "hover:text-neutral-800"], + ])} > Memos </button> <button onClick={() => handleTabChange("transcript")} - className={cn( - "relative px-4 py-2 text-xs pl-3 font-medium transition-all duration-200 border-b-2 -mb-px flex items-center gap-1.5", - currentTab === "transcript" - ? "text-neutral-900 border-neutral-900" - : "text-neutral-600 border-transparent hover:text-neutral-800", - )} + role="tab" + aria-selected={currentTab === "transcript"} + type="button" + className={cn([ + "relative px-4 py-2 text-xs pl-3 font-medium transition-all duration-200 border-b-2 -mb-px flex items-center gap-1.5", + currentTab === "transcript" + ? ["text-neutral-900", "border-neutral-900"] + : ["text-neutral-600", "border-transparent", "hover:text-neutral-800"], + ])} > TranscriptAs per coding guidelines.
Also applies to: 54-66, 67-79, 80-97
apps/desktop2/src/components/main/body/index.tsx (2)
73-78: Add aria-label to icon-only toggle for a11y.Screen readers need a label for the sidebar toggle.
- <Button size="icon" variant="ghost" onClick={() => leftsidebar.setExpanded(true)}> + <Button + size="icon" + variant="ghost" + aria-label="Open left sidebar" + onClick={() => leftsidebar.setExpanded(true)} + >
167-175: Use shared Button for the “new session” control + add aria-label.Keep UI consistent with the header’s Button and improve accessibility.
- <button - onClick={handleNewNote} - className={cn([ - "flex items-center justify-center h-full px-1.5 shrink-0", - "rounded-lg bg-white hover:bg-gray-50 transition-colors cursor-pointer", - ])} - > - <PlusIcon className="h-4 w-4 text-color3 " /> - </button> + <Button + size="icon" + variant="ghost" + aria-label="New session" + onClick={handleNewNote} + > + <PlusIcon className="h-4 w-4 text-color3" /> + </Button>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/desktop2/src/components/interactive-button.tsx(4 hunks)apps/desktop2/src/components/main/body/index.tsx(8 hunks)apps/desktop2/src/components/main/body/sessions/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/inner-header.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/note-input/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/note-input/raw.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx(4 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx(1 hunks)apps/desktop2/src/components/main/body/shared.tsx(4 hunks)apps/desktop2/src/components/main/sidebar/index.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/timeline.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/timeline/index.tsx(1 hunks)packages/tiptap/src/editor/index.tsx(2 hunks)packages/ui/src/components/block/event-chip.tsx(6 hunks)packages/ui/src/components/block/participants-chip.tsx(9 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/interactive-button.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/components/main/body/sessions/note-input/raw.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsxapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/main/body/sessions/inner-header.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/sidebar/index.tsxapps/desktop2/src/components/main/body/sessions/note-input/index.tsxapps/desktop2/src/components/main/body/sessions/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.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/interactive-button.tsxpackages/ui/src/components/block/participants-chip.tsxpackages/ui/src/components/block/event-chip.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxpackages/tiptap/src/editor/index.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/components/main/body/sessions/note-input/raw.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsxapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/main/body/sessions/inner-header.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/sidebar/index.tsxapps/desktop2/src/components/main/body/sessions/note-input/index.tsxapps/desktop2/src/components/main/body/sessions/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx
🧬 Code graph analysis (11)
packages/ui/src/components/block/participants-chip.tsx (1)
packages/ui/src/components/ui/avatar.tsx (1)
AvatarFallback(58-58)
apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (4)
packages/db/src/schema.ts (1)
folders(30-34)packages/ui/src/components/ui/dropdown-menu.tsx (8)
DropdownMenu(180-180)DropdownMenuTrigger(194-194)DropdownMenuContent(182-182)DropdownMenuItem(184-184)DropdownMenuSub(191-191)DropdownMenuSubTrigger(193-193)DropdownMenuSubContent(192-192)DropdownMenuSeparator(189-189)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)packages/ui/src/components/ui/switch.tsx (1)
Switch(57-57)
apps/desktop2/src/components/main/body/sessions/note-input/raw.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
apps/desktop2/src/components/main/body/index.tsx (4)
packages/ui/src/components/ui/button.tsx (1)
Button(37-89)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop2/src/components/main/body/search.tsx (1)
Search(8-107)apps/desktop2/src/store/zustand/tabs/index.ts (1)
useTabs(15-20)
apps/desktop2/src/components/main/body/sessions/inner-header.tsx (3)
apps/desktop2/src/store/zustand/tabs/schema.ts (1)
Tab(53-53)apps/desktop2/src/store/zustand/tabs/index.ts (1)
useTabs(15-20)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/sidebar/timeline.tsx (5)
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
TimelineView(14-108)apps/desktop2/src/components/main/sidebar/timeline/realtime.tsx (2)
useCurrentTime(9-25)CurrentTimeIndicator(3-7)apps/desktop2/src/components/main/sidebar/timeline/item.tsx (1)
TimelineItemComponent(12-162)apps/desktop2/src/utils/timeline.ts (4)
TimelineBucket(30-34)TimelineItem(24-26)TimelinePrecision(28-28)buildTimelineBuckets(112-200)apps/desktop2/src/components/interactive-button.tsx (1)
InteractiveButton(15-71)
apps/desktop2/src/components/main/sidebar/index.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/sessions/index.tsx (4)
apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx (1)
OuterHeader(9-27)apps/desktop2/src/components/main/body/sessions/title-input.tsx (1)
TitleInput(6-32)apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
NoteInput(12-38)apps/desktop2/src/components/main/body/sessions/floating/index.tsx (1)
FloatingActionButton(8-18)
apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (2)
packages/ui/src/components/ui/breadcrumb.tsx (6)
Breadcrumb(108-108)BreadcrumbList(112-112)BreadcrumbSeparator(114-114)BreadcrumbItem(110-110)BreadcrumbLink(111-111)BreadcrumbPage(113-113)packages/ui/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(180-180)DropdownMenuTrigger(194-194)DropdownMenuContent(182-182)DropdownMenuItem(184-184)
⏰ 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: zizmor
🔇 Additional comments (20)
packages/ui/src/components/block/participants-chip.tsx (1)
84-86: Color token migration looks good.The replacement of hard-coded neutral colors with
color3/color4tokens is consistent and aligns with design system best practices.Also applies to: 143-143, 201-201, 219-219, 222-222, 231-231, 240-240, 274-274, 280-280, 303-303
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
102-103: LGTM!The Button component correctly accepts multiple children directly. The removal of the wrapper div simplifies the structure.
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (1)
1-8: LGTM!Clean migration to the prebuilt Button component with appropriate size and variant props.
packages/ui/src/components/block/event-chip.tsx (2)
110-128: LGTM!The color token migration from
text-neutral-500totext-color4promotes consistent theming. The gap reduction fromgap-2togap-1tightens the layout appropriately.
214-214: LGTM!Consistent application of color tokens (
text-color4andplaceholder:text-color3) throughout the component.Also applies to: 239-239, 261-267, 273-273, 288-288
apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (3)
1-44: LGTM!The refactor to Breadcrumb components provides better semantic structure and improved navigation UX. The conditional rendering for folders with/without parent is clean and well-organized.
95-127: LGTM!The new folder selection dropdown is well-implemented. The disabled state when no folders exist provides good UX feedback.
131-162: LGTM!The new helper components (FolderMenuItem and FolderQuestionIcon) are properly structured. The custom SVG icon provides a clear visual indicator for the folder selection prompt.
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
15-81: LGTM!The timeline implementation is well-structured with proper:
- Separation of concerns (data hooks, scroll management, rendering)
- React patterns (memo, useCallback, useEffect with correct dependencies)
- Intersection Observer for visibility tracking
- Smooth scrolling UX with directional indicators
Also applies to: 83-151, 153-155
apps/desktop2/src/components/main/sidebar/index.tsx (1)
29-33: LGTM!Consistent migration to the Button component for icon actions, using appropriate
size="icon"andvariant="ghost"props.apps/desktop2/src/components/main/body/sessions/index.tsx (6)
1-1: LGTM: shared Button import usage fits new header UI.
28-29: LGTM: new tab hotkeys hook is registered.
68-71: Alignment tweak looks good.
126-131: Tabs scroller change to w-fit is fine; verify long tab lists still remain scrollable.
308-310: LGTM: StandardTabWrapper padding reduction is safe.
333-333: Good call enabling hotkeys inside inputs/editors.Ensure this doesn’t conflict with Tiptap editor shortcuts.
Also applies to: 358-359
apps/desktop2/src/components/main/body/shared.tsx (1)
35-41: Middle‑click to close: nice UX; verify no conflict with drag/reorder.Since Reorder.Group uses pointer/mouse events, confirm middle‑click does not interfere with dragging or context menu.
Also applies to: 56-56
apps/desktop2/src/components/main/body/index.tsx (3)
308-310: Padding tweak looks good.Reducing to p-2 simplifies spacing without affecting layout.
321-335: Confirm: closing the app with mod+w while editing content.Hotkeys enabled on contentEditable may close tabs/app during text editing. If intentional, all good; otherwise consider disabling in rich-text contexts.
341-360: Confirm: tab selection hotkeys active in contentEditable.Enabling on contentEditable may switch tabs during editing. Validate UX intent.
| <DropdownMenuItem onClick={handleToggleLock}> | ||
| <LockIcon /> | ||
| <span>Lock note</span> | ||
| <Switch checked={isLocked} onCheckedChange={handleToggleLock} className="ml-auto" /> | ||
| </DropdownMenuItem> | ||
|
|
There was a problem hiding this comment.
Bug: Lock toggle likely fires twice when clicking the Switch (item onClick + Switch onCheckedChange).
Remove the item onClick; use onSelect with preventDefault to keep the menu open; rely on Switch’s handler.
- <DropdownMenuItem onClick={handleToggleLock}>
+ <DropdownMenuItem
+ onSelect={(e) => e.preventDefault()}
+ >
<LockIcon />
<span>Lock note</span>
- <Switch checked={isLocked} onCheckedChange={handleToggleLock} className="ml-auto" />
+ <Switch
+ checked={isLocked}
+ onCheckedChange={handleToggleLock}
+ className="ml-auto"
+ />
</DropdownMenuItem>📝 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.
| <DropdownMenuItem onClick={handleToggleLock}> | |
| <LockIcon /> | |
| <span>Lock note</span> | |
| <Switch checked={isLocked} onCheckedChange={handleToggleLock} className="ml-auto" /> | |
| </DropdownMenuItem> | |
| <DropdownMenuItem | |
| onSelect={(e) => e.preventDefault()} | |
| > | |
| <LockIcon /> | |
| <span>Lock note</span> | |
| <Switch | |
| checked={isLocked} | |
| onCheckedChange={handleToggleLock} | |
| className="ml-auto" | |
| /> | |
| </DropdownMenuItem> |
🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx around
lines 108 to 113, the DropdownMenuItem currently has an onClick that duplicates
the Switch's onCheckedChange and can fire the lock toggle twice; remove the
onClick from the item, add an onSelect handler that calls event.preventDefault()
to prevent the menu from closing, and rely solely on the Switch’s
onCheckedChange (handleToggleLock) for toggling the lock state so the action
runs only once.
| handleKeyDown: (_, event) => { | ||
| if ((event.metaKey || event.ctrlKey) && (event.key === "w" || event.key === "n" || event.key === "t")) { | ||
| return false; | ||
| } | ||
| return false; | ||
| }, |
There was a problem hiding this comment.
Critical: handleKeyDown blocks ALL keyboard input.
The unconditional return false on line 70 prevents all keydown events from propagating, not just Ctrl/Cmd+w/n/t. This will break all text input and keyboard navigation in the editor.
Apply this diff:
handleKeyDown: (_, event) => {
if ((event.metaKey || event.ctrlKey) && (event.key === "w" || event.key === "n" || event.key === "t")) {
return false;
}
- return false;
},Alternatively, if you want to explicitly allow other keys:
handleKeyDown: (_, event) => {
if ((event.metaKey || event.ctrlKey) && (event.key === "w" || event.key === "n" || event.key === "t")) {
return false;
}
+ return true;
},(In TipTap, false = "handled, stop propagation" and true = "not handled, continue to other handlers")
📝 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.
| handleKeyDown: (_, event) => { | |
| if ((event.metaKey || event.ctrlKey) && (event.key === "w" || event.key === "n" || event.key === "t")) { | |
| return false; | |
| } | |
| return false; | |
| }, | |
| handleKeyDown: (_, event) => { | |
| if ((event.metaKey || event.ctrlKey) && (event.key === "w" || event.key === "n" || event.key === "t")) { | |
| return false; | |
| } | |
| }, |
🤖 Prompt for AI Agents
In packages/tiptap/src/editor/index.tsx around lines 66 to 71, handleKeyDown
currently returns false unconditionally which stops all keyboard input; change
it so it only returns false for the specific blocked combos (Ctrl/Cmd + w|n|t)
and returns true (or undefined) for all other keys so events propagate normally
— i.e., detect the meta/ctrl + key combinations and return false only in that
branch, otherwise return true (or omit returning false) to allow normal TipTap
input and navigation.
| if ((e.metaKey || e.ctrlKey) && (e.key === "w" || e.key === "n" || e.key === "t")) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify: early return doesn't prevent default for blocked shortcuts.
The global listener returns early for Ctrl/Cmd+w/n/t without calling preventDefault(). This differs from the handleKeyDown approach (line 67-69) which returns false to block propagation. Ensure this early return achieves the intended behavior of suppressing browser shortcuts.
🏁 Script executed:
#!/bin/bash
# Search for any documentation or tests related to keyboard shortcut handling in the editor
rg -n -C3 'preventDefault|handleKeyDown' --type=ts --type=tsx -g '!node_modules' -g 'packages/tiptap/**'Length of output: 27
I'll fix the script and examine the actual code to understand the event listener context and verify the review concern.
Now let me examine the actual code file to understand the event listener context:
Now let me examine the actual code file:
Hmm, let me correct that - I need to ensure the repository is available:
Perfect! I found the critical information. In ProseMirror's handleDOMEvents, when returning true from such a function, you are responsible for calling preventDefault yourself. This is the key difference from other event handlers.
Let me now examine the actual code to confirm the context:
# Find and display the editor index.tsx file, focusing on lines 67-69 and 106-108
fd "index.tsx" packages/tiptap/src/editor -type f -exec cat {} \;Verify: early return doesn't prevent default for blocked shortcuts.
The review comment correctly identifies a critical issue. In ProseMirror's handleDOMEvents, when returning true from a function, you are responsible for calling preventDefault yourself (or not, if you want to allow the default behavior). This means:
- An early
returnstatement withoute.preventDefault()will not suppress browser shortcuts like Ctrl/Cmd+W/N/T - The code at lines 106-108 returns early but never calls
preventDefault()on the event - Therefore, the browser's default behavior for these keyboard shortcuts will still execute
The comparison to handleKeyDown returning false (line 67-69) is also misleading—return false from a handler is an exception only when assigned using on attributes, and in all other cases, return value is ignored.
To fix: Add e.preventDefault() before the early return:
if ((e.metaKey || e.ctrlKey) && (e.key === "w" || e.key === "n" || e.key === "t")) {
e.preventDefault();
return;
}
🏁 Script executed:
#!/bin/bash
# View the editor file around lines 106-108 and 67-69 for context
fd -n "index.tsx" packages/tiptap/src/editor | head -1 | xargs catLength of output: 230
Let me fix the fd command syntax:
# Find and display the editor index.tsx file
fd "index.tsx" packages/tiptap/src/editor -type f -exec cat {} \;🏁 Script executed:
#!/bin/bash
# Find and display the editor index.tsx file to examine lines 67-69 and 106-108
fd "index.tsx" packages/tiptap/src/editor -type f | head -1 | xargs catLength of output: 170
🏁 Script executed:
#!/bin/bash
# Display the editor index.tsx file directly
cat packages/tiptap/src/editor/index.tsxLength of output: 3955
Add e.preventDefault() to actually suppress browser shortcuts.
The review comment is correct. The code at lines 106-108 uses an addEventListener callback, where a bare return statement has no effect on default browser behavior. Only e.preventDefault() actually suppresses Ctrl/Cmd+W/N/T shortcuts.
In vanilla JavaScript event listeners added via addEventListener, the return value is ignored entirely. The comparison to handleKeyDown at lines 67-69 is misleading—that returns false within editorProps.handleKeyDown (a ProseMirror handler context where return values have meaning), whereas the problematic code at lines 106-108 is in a standard DOM event listener where return values are irrelevant.
Without e.preventDefault(), the browser will still execute its default actions for these keyboard shortcuts (closing the tab for Ctrl+W, opening a new window for Ctrl+N, opening a new tab for Ctrl+T).
Fix: Add e.preventDefault(); before the return statement at line 107:
if ((e.metaKey || e.ctrlKey) && (e.key === "w" || e.key === "n" || e.key === "t")) {
e.preventDefault();
return;
}
🤖 Prompt for AI Agents
In packages/tiptap/src/editor/index.tsx around lines 106 to 108, the DOM keydown
listener currently returns early for Cmd/Ctrl+W/N/T but does not call
e.preventDefault(), so the browser shortcuts still run; update the handler to
call e.preventDefault() before returning when (e.metaKey || e.ctrlKey) && (e.key
=== "w" || e.key === "n" || e.key === "t") so the default browser actions are
suppressed.
ComputelessComputer
force-pushed
the
jj-branch-20
branch
2 times, most recently
from
128c1a5 to
982cca4
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (1)
108-112: Fix double-toggle on Lock; use onSelect and Switch handler only.Item onClick + Switch onCheckedChange fires twice. Remove item onClick, keep Switch handler; prevent menu close via onSelect.
Apply:
- <DropdownMenuItem onClick={handleToggleLock}> + <DropdownMenuItem + onSelect={(e) => e.preventDefault()} + > <LockIcon /> <span>Lock note</span> - <Switch checked={isLocked} onCheckedChange={handleToggleLock} className="ml-auto" /> + <Switch + checked={isLocked} + onCheckedChange={handleToggleLock} + className="ml-auto" + /> </DropdownMenuItem>Also make the toggle resilient to stale state:
- const handleToggleLock = () => { - setIsLocked(!isLocked); + const handleToggleLock = () => { + setIsLocked((v) => !v);apps/desktop2/src/components/main/body/index.tsx (2)
137-151: Drag region overlaps interactive controls (repeat of prior finding).Keep drag region but opt-out children by marking them “non-drag,” or remove the region from this wrapper.
Option B — keep drag-region, opt-out children:
<div data-tauri-drag-region className="flex-1 flex h-full items-center justify-between" > - <Button + <Button + data-tauri-drag-region="none" onClick={handleNewNote} variant="ghost" size="icon" className="text-color3" > <PlusIcon size={16} /> </Button> - <Search /> + <div data-tauri-drag-region="none"> + <Search /> + </div> </div>
336-364: Align new‑tab hotkey payload, guard key repeat; confirm mod+n close behavior.Match session payload to other creation sites and avoid repeated opens when key is held. Also confirm the destructive “close current tab on mod+n” choice.
useHotkeys( ["mod+n", "mod+t"], - (e) => { + (e) => { + if (e.repeat) return; e.preventDefault(); const sessionId = id(); const user_id = internalStore?.getValue("user_id"); - persistedStore?.setRow("sessions", sessionId, { user_id, created_at: new Date().toISOString() }); + persistedStore?.setRow("sessions", sessionId, { + user_id, + created_at: new Date().toISOString(), + title: "", + }); if (e.key === "n" && currentTab) { close(currentTab); }If helpful, I can run a repo-wide check to ensure all sessions setRow usages include title consistently.
🧹 Nitpick comments (10)
plugins/notification/src/handler.rs (1)
103-110: LGTM! Consider extracting the ignore list for maintainability.The added application IDs are correctly formatted and integrate seamlessly with the existing ignore logic. The list now covers a comprehensive set of applications where microphone activity should not trigger meeting detection notifications.
Optional: As the ignore list has grown to 19 entries, consider extracting it to a module-level constant for improved maintainability:
+const DEFAULT_IGNORED_APP_IDS: &[&str] = &[ + "com.electron.wispr-flow", + "com.seewillow.WillowMac", + "com.superduper.superwhisper", + "dev.warp.Warp-Stable", + "so.cap.desktop", + "com.timpler.screenstudio", + "com.loom.desktop", + "com.obsproject.obs-studio", + "com.prakashjoshipax.VoiceInk", + "com.goodsnooze.macwhisper", + "com.descript.beachcube", + "com.openai.chat", + "com.anthropic.claudefordesktop", + "com.raycast.macos", + "com.apple.VoiceMemos", + "com.exafunction.windsurf", + "dev.zed.Zed", + "com.microsoft.VSCode", + "com.todesktop.230313mzl4w4u92", +]; + fn handle_detect_event(app_handle: &AppHandle<tauri::Wry>, trigger: NotificationTriggerDetect) {Then replace the vec! with:
- if apps.iter().any(|app| { - vec![ - "com.electron.wispr-flow", - // ... all entries ... - ] - .contains(&app.id.as_str()) - }) { + if apps.iter().any(|app| { + DEFAULT_IGNORED_APP_IDS.contains(&app.id.as_str()) + }) {packages/ui/src/components/ui/button.tsx (1)
52-66: Add minimal ARIA to reflect loading/disabled state.Expose state to screen readers without changing visuals/logic.
Apply:
<button ref={ref} - disabled={disabled || isLoading} + disabled={disabled || isLoading} + aria-busy={isLoading || undefined} + aria-disabled={(disabled || isLoading) || undefined} className={cn( styles.base, styles.variants[variant], styles.sizes[size], isLoading && "cursor-wait opacity-80", className, )} {...props} > {isLoading && <Spinner size={16} />} {children} </button>Also applies to: 4-4, 17-17, 64-64
apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (2)
100-104: Pass folder id to the handler; prefer onSelect for menu items.Make the API correct now to avoid refactors later and improve keyboard UX.
Apply:
- const handleMoveToFolder = () => { + const handleMoveToFolder = (folderId: string) => { // TODO: Implement move to folder functionality - console.log("Move to folder"); + console.log("Move to folder", folderId); };- {folders.map((folder) => ( - <DropdownMenuItem key={folder.id} onClick={handleMoveToFolder}> + {folders.map((folder) => ( + <DropdownMenuItem + key={folder.id} + onSelect={() => handleMoveToFolder(folder.id)} + > <FolderIcon /> <span>{folder.name}</span> </DropdownMenuItem> ))}
48-50: Use functional setState for toggles and onSelect for commands.
- Functional updates avoid stale reads in rapid interactions.
- onSelect gives better keyboard semantics and lets you control menu closing.
Apply:
- const handleToggleListening = () => { - setIsListening(!isListening); + const handleToggleListening = () => { + setIsListening((v) => !v);Example switching onClick -> onSelect:
- <DropdownMenuItem onClick={handleCopyLink}> + <DropdownMenuItem onSelect={handleCopyLink}> ... - <DropdownMenuItem onClick={handleExportPDF}> + <DropdownMenuItem onSelect={handleExportPDF}> ... - <DropdownMenuItem onClick={handleDeleteNote} className="text-red-600 focus:text-red-600"> + <DropdownMenuItem onSelect={handleDeleteNote} className="text-red-600 focus:text-red-600"> ... - <DropdownMenuItem onClick={handleHistory}> + <DropdownMenuItem onSelect={handleHistory}>Also applies to: 59-61, 88-91, 114-117, 128-136, 140-143
packages/ui/src/components/block/meeting-metadata-chip.tsx (1)
335-366: Optional: add listbox semantics to the search results.Improve keyboard/screen‑reader UX without changing visuals.
- Input: add
role="combobox"andaria-expanded,aria-controls.- Results container:
role="listbox", each rowrole="option"andaria-selected={selectedIndex === index}.- Keep current key handling; this only adds semantics.
Also applies to: 286-334, 252-273
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
1-1: Use cn instead of clsx in apps/desktop2; drop clsx import.Project guideline prefers cn for Tailwind composition under apps/desktop2. Replace clsx([...]) with cn([...]) and remove the clsx import.
- import { clsx } from "clsx"; + // (remove — not used after refactor) @@ - className={clsx([ + className={cn([ "group", "relative", "absolute left-1/2 transform -translate-x-1/2", "bg-white hover:bg-gray-50", "border border-gray-200", "rounded-full shadow-lg", "text-gray-700 z-40", isScrolledPastToday ? "top-2" : "bottom-2", isScrolledPastToday ? "shadow-[0_-2px_8px_rgba(248,113,113,0.3)] hover:shadow-[0_-2px_12px_rgba(239,68,68,0.4)]" : "shadow-[0_2px_8px_rgba(248,113,113,0.3)] hover:shadow-[0_2px_12px_rgba(239,68,68,0.4)]", ])}As per coding guidelines.
Also applies to: 88-101
apps/desktop2/src/components/main/body/shared.tsx (2)
1-6: Adopt cn for class composition in apps/desktop2; remove clsx.Follow repo convention: use cn([...]) and group logically. Add cn import and replace clsx usages.
+import { cn } from "@hypr/ui/lib/utils"; @@ - className={clsx([ + className={cn([ "flex items-center gap-2 cursor-pointer group", "w-48 h-full pl-2 pr-1", "bg-color1 rounded-lg border", active ? "text-black border-black" : "text-color3 border-transparent", ])} @@ - className={clsx([ + className={cn([ "flex-shrink-0 transition-opacity", active ? "opacity-100 text-color4" : "opacity-0 group-hover:opacity-100 text-color3", ])}As per coding guidelines.
Also applies to: 59-64, 75-80
70-86: Add accessible name to the close icon button.Provide aria-label for screen readers.
- <Button + <Button + aria-label="Close tab" onClick={(e) => { e.stopPropagation(); handleCloseThis(); }} className={cn([ "flex-shrink-0 transition-opacity", active ? "opacity-100 text-color4" : "opacity-0 group-hover:opacity-100 text-color3", ])} size="icon" variant="ghost" > <X size={14} /> </Button>apps/desktop2/src/components/main/sidebar/timeline.tsx (2)
65-74: Use cn instead of clsx in apps/desktop2.Replace clsx([...]) with cn([...]) and add cn import; drop clsx import.
+import { cn } from "@hypr/ui/lib/utils"; @@ - className={clsx([ + className={cn([ "absolute left-1/2 transform -translate-x-1/2", "rounded-full bg-white hover:bg-gray-50", "text-gray-700 border border-gray-200", "z-40 flex items-center gap-1", "shadow-[inset_0_-4px_6px_-1px_rgba(255,0,0,0.1),inset_0_-2px_4px_-2px_rgba(255,0,0,0.1)]", isScrolledPastToday ? "top-2" : "bottom-2", ])} @@ - className={clsx([ + className={cn([ "w-full text-left px-3 py-2 rounded-lg", active && "bg-gray-200", !active && "hover:bg-gray-100", ])}As per coding guidelines.
Also applies to: 272-276
364-378: Avoid duplicating useCurrentTime; import the shared hook instead.There’s already a realtime hook under timeline/realtime.tsx. Import it to keep a single source of truth and reduce drift.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/desktop2/src/components/main/body/index.tsx(7 hunks)apps/desktop2/src/components/main/body/sessions/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/note-input/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx(4 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx(2 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx(1 hunks)apps/desktop2/src/components/main/body/shared.tsx(3 hunks)apps/desktop2/src/components/main/sidebar/index.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/timeline.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/timeline/index.tsx(1 hunks)packages/ui/src/components/block/event-chip.tsx(6 hunks)packages/ui/src/components/block/meeting-metadata-chip.tsx(1 hunks)packages/ui/src/components/block/participants-chip.tsx(10 hunks)packages/ui/src/components/icons/linkedin.tsx(1 hunks)packages/ui/src/components/ui/button.tsx(3 hunks)plugins/notification/src/handler.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/desktop2/src/components/main/body/sessions/note-input/index.tsx
- packages/ui/src/components/icons/linkedin.tsx
- apps/desktop2/src/components/main/sidebar/index.tsx
- apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx
- packages/ui/src/components/block/event-chip.tsx
- packages/ui/src/components/block/participants-chip.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/sessions/outer-header/index.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsxapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsxapps/desktop2/src/components/main/body/sessions/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/sessions/outer-header/index.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxpackages/ui/src/components/block/meeting-metadata-chip.tsxpackages/ui/src/components/ui/button.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsxapps/desktop2/src/components/main/body/index.tsxplugins/notification/src/handler.rsapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsxapps/desktop2/src/components/main/body/sessions/index.tsx
🧬 Code graph analysis (9)
apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx (1)
apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx (1)
SessionMetadata(14-223)
apps/desktop2/src/components/main/sidebar/timeline.tsx (5)
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
TimelineView(14-108)apps/desktop2/src/components/main/sidebar/timeline/realtime.tsx (2)
useCurrentTime(9-25)CurrentTimeIndicator(3-7)apps/desktop2/src/components/main/sidebar/timeline/item.tsx (1)
TimelineItemComponent(12-162)apps/desktop2/src/utils/timeline.ts (4)
TimelineBucket(30-34)TimelineItem(24-26)TimelinePrecision(28-28)buildTimelineBuckets(112-200)apps/desktop2/src/components/interactive-button.tsx (1)
InteractiveButton(15-71)
apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (4)
packages/db/src/schema.ts (1)
folders(30-34)packages/ui/src/components/ui/dropdown-menu.tsx (8)
DropdownMenu(180-180)DropdownMenuTrigger(194-194)DropdownMenuContent(182-182)DropdownMenuItem(184-184)DropdownMenuSub(191-191)DropdownMenuSubTrigger(193-193)DropdownMenuSubContent(192-192)DropdownMenuSeparator(189-189)packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/components/ui/switch.tsx (1)
Switch(57-57)
packages/ui/src/components/block/meeting-metadata-chip.tsx (5)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/components/ui/popover.tsx (3)
Popover(85-85)PopoverTrigger(85-85)PopoverContent(85-85)packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/avatar.tsx (2)
Avatar(58-58)AvatarFallback(58-58)packages/ui/src/components/icons/linkedin.tsx (1)
LinkedInIcon(7-23)
apps/desktop2/src/components/main/body/shared.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)
apps/desktop2/src/components/main/body/index.tsx (4)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop2/src/components/main/body/search.tsx (1)
Search(8-107)apps/desktop2/src/store/zustand/tabs/index.ts (1)
useTabs(15-20)
apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (2)
packages/ui/src/components/ui/breadcrumb.tsx (6)
Breadcrumb(108-108)BreadcrumbList(112-112)BreadcrumbSeparator(114-114)BreadcrumbItem(110-110)BreadcrumbLink(111-111)BreadcrumbPage(113-113)packages/ui/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(180-180)DropdownMenuTrigger(194-194)DropdownMenuContent(182-182)DropdownMenuItem(184-184)
apps/desktop2/src/components/main/body/sessions/index.tsx (4)
apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx (1)
OuterHeader(8-25)apps/desktop2/src/components/main/body/sessions/title-input.tsx (1)
TitleInput(6-32)apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
NoteInput(12-38)apps/desktop2/src/components/main/body/sessions/floating/index.tsx (1)
FloatingActionButton(8-18)
🪛 GitHub Actions: .github/workflows/fmt.yaml
apps/desktop2/src/components/main/body/shared.tsx
[error] 1-2: dprint formatting check failed. Found 1 not formatted file. Command '/.dprint/bin/dprint check --config dprint.json' exited with code 20. Run '/.dprint/bin/dprint fmt' to fix code style issues in this file.
🔇 Additional comments (7)
apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx (1)
16-19: Consolidation looks good.Replacing per‑piece event/participants with SessionMetadata simplifies the header and keeps concerns localized.
apps/desktop2/src/components/main/body/sessions/index.tsx (1)
41-48: Verify FAB anchoring after wrapper change.With FAB now inside the mt-3 px-2 container, confirm its absolute positioning anchors as intended (not clipped by NoteInput’s scroll area or Timeline). If needed, make the intended container
relativeor move FAB outside the scrolled region.apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (1)
35-42: Breadcrumb + picker implementation reads well.Good use of Breadcrumb, onSelect in menu, and localized title editing. Fallbacks covered.
Also applies to: 65-71, 107-119, 130-139, 146-149
packages/ui/src/components/block/meeting-metadata-chip.tsx (1)
127-133: Confirm Popover trigger works with stopPropagation on mouse down.Some popover libs rely on pointer/mouse down for focus management; stopping propagation there can prevent open/close or focus. If you only need to avoid parent handlers, stopping click is usually enough.
Suggestion if issues appear:
- onMouseDown={(e) => { e.stopPropagation(); }} + // Consider removing or using onPointerDownCapture if needed for parent suppressionapps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
102-104: Button children restructure is good.Inline icon + label directly under Button improves hit target without changing behavior.
apps/desktop2/src/components/main/body/shared.tsx (2)
37-44: Middle‑click close behavior works.onMouseDown with e.button === 1 is a simple, effective intercept for “close tab on middle click.”
1-3: Manual verification required; dprint executable unavailable in sandbox.The dprint formatter is not available in the sandbox environment, so I cannot execute the suggested command to verify or fix formatting. The file
apps/desktop2/src/components/main/body/shared.tsxshows imports organized into logical groups separated by blank lines, which appears well-structured.You must run
dprint fmt --config dprint.jsonlocally to verify and correct any formatting issues the CI detected.
| import { Button } from "@hypr/ui/components/ui/button"; | ||
| import { ContextMenuItem } from "@hypr/ui/components/ui/context-menu"; | ||
|
|
||
| import { clsx } from "clsx"; | ||
| import { ChevronDownIcon, ChevronUpIcon, ExternalLink, Trash2 } from "lucide-react"; | ||
| import { forwardRef, Fragment, useCallback, useEffect, useMemo, useRef, useState } from "react"; | ||
|
|
||
| import * as persisted from "../../../store/tinybase/persisted"; | ||
| import { Tab, useTabs } from "../../../store/zustand/tabs"; | ||
| import { id } from "../../../utils"; | ||
| import { buildTimelineBuckets } from "../../../utils/timeline"; | ||
| import type { TimelineBucket, TimelineItem, TimelinePrecision } from "../../../utils/timeline"; | ||
| import { InteractiveButton } from "../../interactive-button"; | ||
|
|
||
| export function TimelineView() { |
There was a problem hiding this comment.
🧩 Analysis chain
Module resolution collision: timeline.tsx shadows timeline/index.tsx.
Having both sidebar/timeline.tsx and sidebar/timeline/index.tsx exporting TimelineView makes import "./timeline" resolve to the file (timeline.tsx) instead of the directory index, changing behavior silently. Remove one source or change imports to point explicitly to the intended module (e.g., "./timeline/index"). This is brittle and can cause regressions.
🏁 Script executed:
#!/usr/bin/env bash
echo "Imports resolving to './timeline' or '*sidebar/timeline':"
rg -nP --no-heading '\bfrom\s+[\"\'](\.\/timeline|.*\/sidebar\/timeline)[\"\']' -C1
echo ""
echo "Files present that can collide:"
fd -a 'timeline.tsx' apps/desktop2/src/components/main/sidebar
fd -a 'index.tsx' apps/desktop2/src/components/main/sidebar/timelineLength of output: 228
🏁 Script executed:
#!/bin/bash
# Find all timeline-related files
echo "=== Files in sidebar directory ==="
fd -a 'timeline' apps/desktop2/src/components/main/sidebar
echo ""
echo "=== Imports from './timeline' (simplified) ==="
rg -n 'from.*timeline' apps/desktop2/src/components/main/sidebar -A1 -B1 --type tsx
echo ""
echo "=== TimelineView exports ==="
rg -n 'export.*TimelineView' apps/desktop2/src/components/main/sidebar --type tsxLength of output: 313
🏁 Script executed:
#!/bin/bash
# Check what's in the timeline directory
echo "=== Contents of timeline/ directory ==="
fd . apps/desktop2/src/components/main/sidebar/timeline
echo ""
echo "=== Export from timeline.tsx ==="
head -20 apps/desktop2/src/components/main/sidebar/timeline.tsx | grep -E 'export'
echo ""
echo "=== Export from timeline/index.tsx (if exists) ==="
if [ -f apps/desktop2/src/components/main/sidebar/timeline/index.tsx ]; then
head -20 apps/desktop2/src/components/main/sidebar/timeline/index.tsx | grep -E 'export'
else
echo "File does not exist"
fi
echo ""
echo "=== Imports referencing 'timeline' in sidebar ==="
rg 'from.*timeline' apps/desktop2/src/components/main/sidebar -n
echo ""
echo "=== Imports referencing 'timeline' in entire apps/desktop2 ==="
rg 'from.*sidebar/timeline' apps/desktop2/src -n | head -20Length of output: 1363
🏁 Script executed:
#!/bin/bash
# Compare the two TimelineView implementations
echo "=== Content of timeline.tsx ==="
wc -l apps/desktop2/src/components/main/sidebar/timeline.tsx
echo ""
echo "=== Content of timeline/index.tsx ==="
wc -l apps/desktop2/src/components/main/sidebar/timeline/index.tsx
echo ""
echo "=== First 30 lines of timeline.tsx ==="
head -30 apps/desktop2/src/components/main/sidebar/timeline.tsx
echo ""
echo "=== First 30 lines of timeline/index.tsx ==="
head -30 apps/desktop2/src/components/main/sidebar/timeline/index.tsxLength of output: 2676
Resolve module resolution collision: timeline.tsx and timeline/index.tsx both export TimelineView.
The import in sidebar/index.tsx:10 (import { TimelineView } from "./timeline") resolves to timeline.tsx instead of the directory index due to file-level precedence in module resolution. This creates a silent collision—both files export identically named functions with different implementations (402 vs 218 lines). Remove or refactor one source to eliminate ambiguity, or change imports to explicitly reference the intended module (e.g., "./timeline/index").
Additionally, timeline.tsx uses clsx instead of the cn utility from @hypr/ui/lib/utils as required by the codebase guidelines for this path pattern.
🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/sidebar/timeline.tsx lines 1-15, there is a
module resolution collision because both timeline.tsx and timeline/index.tsx
export TimelineView and imports like import { TimelineView } from "./timeline"
resolve to the file instead of the directory; fix by either removing or
refactoring one of the duplicate exports (rename the component or move
implementation into the index file) OR update all imports to explicitly
reference the intended module (e.g., "./timeline/index") to remove ambiguity,
and while editing this file replace the clsx usage with the codebase-standard cn
utility by importing cn from "@hypr/ui/lib/utils" and swapping clsx calls to cn.
982cca4 to
0dcc9be
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
88-100: Replaceclsxwithcnutility per coding guidelines.Line 88 uses
clsxwhile line 43 correctly usescn. The coding guidelines require using thecnutility for this path pattern.As per coding guidelines.
Apply this diff:
className={clsx([ + className={cn([ "group", "relative", "absolute left-1/2 transform -translate-x-1/2",
♻️ Duplicate comments (6)
apps/desktop2/src/components/main/sidebar/timeline.tsx (2)
232-241: Delete action is a no-op.This issue was already flagged in a previous review. The delete action only logs to console instead of removing the item from the store.
1-14: Replaceclsxwithcnutility per coding guidelines.Line 4 imports
clsx, but the coding guidelines require usingcnfrom@hypr/ui/lib/utilsfor this path pattern. Thecnutility is not imported, andclsxis used at lines 65 and 272.As per coding guidelines.
Apply this diff:
import { Button } from "@hypr/ui/components/ui/button"; import { ContextMenuItem } from "@hypr/ui/components/ui/context-menu"; +import { cn } from "@hypr/ui/lib/utils"; -import { clsx } from "clsx"; import { ChevronDownIcon, ChevronUpIcon, ExternalLink, Trash2 } from "lucide-react";Then replace
clsxwithcnat lines 65 and 272.apps/desktop2/src/components/main/body/index.tsx (3)
336-364: Align new-tab hotkey payload + add key-repeat guard; confirm mod+n close.
- Add title: "" to sessions row (matches handleNewNote).
- Guard e.repeat to avoid duplicate tabs.
- Verify intent: mod+n currently closes the current tab before opening.
useHotkeys( ["mod+n", "mod+t"], - (e) => { + (e) => { + if (e.repeat) return; e.preventDefault(); @@ - persistedStore?.setRow("sessions", sessionId, { user_id, created_at: new Date().toISOString() }); + persistedStore?.setRow("sessions", sessionId, { + user_id, + created_at: new Date().toISOString(), + title: "", + }); @@ - if (e.key === "n" && currentTab) { + if (e.key === "n" && currentTab) { close(currentTab); }Quick check script to find other session creations (expect consistent payloads):
#!/bin/bash rg -nP --no-heading 'setRow\(\s*["\']sessions["\']\s*,\s*[^,]+,\s*\{[^}]*\}\s*\)' -C2
99-106: Remove drag-region from tabs scroller; blocks clicks/reorder.This was flagged earlier and still present. Tabs won’t receive click/drag reliably under a drag-region.
- <div - ref={tabsScrollContainerRef} - data-tauri-drag-region - className={cn([ + <div + ref={tabsScrollContainerRef} + className={cn([ "[&::-webkit-scrollbar]:hidden [-ms-overflow-style:none] [scrollbar-width:none]", "w-fit overflow-x-auto overflow-y-hidden h-full", ])} >
137-151: Drag-region overlaps button and Search; makes them non-interactive.Keep drag only on non-interactive areas or opt-out children. This regression mirrors a previously fixed issue.
Option A — remove drag-region:
- <div - data-tauri-drag-region - className="flex-1 flex h-full items-center justify-between" - > + <div className="flex-1 flex h-full items-center justify-between">Option B — opt-out interactive children:
- <Button + <Button data-tauri-drag-region="none" onClick={handleNewNote} variant="ghost" size="icon" className="text-color3" > <PlusIcon size={16} /> </Button> - <Search /> + <div data-tauri-drag-region="none"> + <Search /> + </div>apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (1)
108-113: Bug: Lock toggle fires twice (item onClick + Switch).Remove item onClick; keep menu open via onSelect; rely on Switch handler only. Same as prior review; still unresolved.
- <DropdownMenuItem onClick={handleToggleLock}> + <DropdownMenuItem + onSelect={(e) => e.preventDefault()} + > <LockIcon /> <span>Lock note</span> - <Switch checked={isLocked} onCheckedChange={handleToggleLock} className="ml-auto" /> + <Switch + checked={isLocked} + onCheckedChange={handleToggleLock} + className="ml-auto" + /> </DropdownMenuItem>
🧹 Nitpick comments (10)
apps/desktop2/src/components/main/sidebar/timeline.tsx (2)
220-230: Refactor complex active state calculation.The TODO comment indicates this logic is "not ideal." The active state calculation uses a complex nested expression with an IIFE that could be extracted into a helper function for clarity.
Consider extracting to a helper:
const isItemActive = useCallback(() => { if (currentTab?.type !== "sessions") { return false; } if (item.type === "session") { return currentTab.id === item.id; } if (item.type === "event" && item.id === eventId && store) { const session = store.getRow("sessions", currentTab.id); return session?.event_id === eventId; } return false; }, [currentTab, item, eventId, store]); const active = isItemActive();
183-218: Consider optimizing session lookup.The function manually iterates through all sessions to find one with a matching
event_id. For large session counts, this could be slow. Consider using a TinyBase query or maintaining an index.apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
56-70: Add ARIA roles for tabs (accessibility).Treat these buttons as tabs for AT: add role="tablist" on the container and role="tab"/aria-selected on each button.
- return ( - <div className="flex gap-4"> - {editorTabs.map((view) => ( - <button + return ( + <div className="flex gap-4" role="tablist" aria-label="Editor views"> + {editorTabs.map((view) => ( + <button key={view} onClick={() => handleTabChange(view)} + role="tab" + aria-selected={currentTab === view} className={cn([ "relative py-2 text-xs font-medium transition-all duration-200 border-b-2 -mb-px", currentTab === view ? ["text-neutral-900", "border-neutral-900"] : ["text-neutral-600", "border-transparent", "hover:text-neutral-800"], ])} > {labelForEditorView(view)} </button> ))} </div> );apps/desktop2/src/components/main/body/shared.tsx (2)
1-6: Usecnutility per repo guidelines; removeclsx.apps/desktop2 requires
cnfrom @hypr/ui/lib/utils with array composition. Replaceclsxusages.- import { clsx } from "clsx"; + import { cn } from "@hypr/ui/lib/utils"; @@ - className={clsx([ + className={cn([ "flex items-center gap-2 cursor-pointer group", "w-48 h-full pl-2 pr-1", "bg-color1 rounded-lg border", active ? "text-black border-black" : "text-color3 border-transparent", ])} @@ - <Button + <Button onClick={(e) => { e.stopPropagation(); handleCloseThis(); }} - className={clsx([ + className={cn([ "flex-shrink-0 transition-opacity", active ? "opacity-100 text-color4" : "opacity-0 group-hover:opacity-100 text-color3", ])} size="icon" variant="ghost" >Also applies to: 59-64, 75-83
70-86: Icon-only close control needs an accessible name.Add an aria-label so screen readers announce the action.
- <Button + <Button + aria-label="Close tab" onClick={(e) => { e.stopPropagation(); handleCloseThis(); }} className={cn([ "flex-shrink-0 transition-opacity", active ? "opacity-100 text-color4" : "opacity-0 group-hover:opacity-100 text-color3", ])} size="icon" variant="ghost" >apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (2)
64-71: Make crumb buttons explicittype="button".Prevents accidental form submit when nested in forms.
- <BreadcrumbLink asChild> - <button onClick={() => openNew({ type: "folders", id, active: true })}> + <BreadcrumbLink asChild> + <button + type="button" + onClick={() => openNew({ type: "folders", id, active: true })} + > {name} </button> </BreadcrumbLink>
107-114: Trigger consistency (optional).Consider using
DropdownMenuTrigger asChildwith a Button for consistency with other triggers in the codebase.apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (1)
121-124: Optional: keep menu open while toggling listening.Use
onSelect={(e) => e.preventDefault()}if you want the menu to remain open when toggling.packages/ui/src/components/block/meeting-metadata-chip.tsx (2)
283-283: Consider removing structural "what" comments.These comments describe what the code sections are rather than why they exist. Per coding guidelines, comments should focus on "why" rather than "what."
Based on coding guidelines.
Apply this diff:
- {/* Existing Participants Chips */} {participants.length > 0 && (- {/* Search Input */} <div className="relative">- {/* Search Results Dropdown */} {isFocused && searchQuery.trim() && (Also applies to: 298-298, 332-332
377-384: Add defensive checks to handle edge cases in name splitting.The
getInitialsfunction may produce unexpected results iffull_namecontains consecutive spaces (e.g., "John Doe" would includeundefinedin the result).Apply this diff to filter out empty segments:
const getInitials = (name: string) => { return name .split(" ") + .filter((n) => n.length > 0) .map((n) => n[0]) .join("") .toUpperCase() .slice(0, 2); };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/desktop2/src/components/main/body/index.tsx(7 hunks)apps/desktop2/src/components/main/body/sessions/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/note-input/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx(4 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx(2 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx(1 hunks)apps/desktop2/src/components/main/body/shared.tsx(3 hunks)apps/desktop2/src/components/main/sidebar/index.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/timeline.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/timeline/index.tsx(1 hunks)packages/ui/src/components/block/event-chip.tsx(6 hunks)packages/ui/src/components/block/meeting-metadata-chip.tsx(1 hunks)packages/ui/src/components/block/participants-chip.tsx(10 hunks)packages/ui/src/components/icons/linkedin.tsx(1 hunks)packages/ui/src/components/ui/button.tsx(3 hunks)plugins/notification/src/handler.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx
- plugins/notification/src/handler.rs
- apps/desktop2/src/components/main/sidebar/index.tsx
- apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx
- packages/ui/src/components/block/event-chip.tsx
- packages/ui/src/components/block/participants-chip.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:
packages/ui/src/components/ui/button.tsxapps/desktop2/src/components/main/body/sessions/index.tsxpackages/ui/src/components/icons/linkedin.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxapps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsxpackages/ui/src/components/block/meeting-metadata-chip.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/body/sessions/note-input/index.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/body/sessions/index.tsxapps/desktop2/src/components/main/sidebar/timeline.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxapps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/body/sessions/note-input/index.tsxapps/desktop2/src/components/main/body/index.tsx
🧬 Code graph analysis (10)
packages/ui/src/components/ui/button.tsx (1)
packages/ui/src/components/ui/spinner.tsx (1)
Spinner(58-58)
apps/desktop2/src/components/main/body/sessions/index.tsx (4)
apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx (1)
OuterHeader(8-25)apps/desktop2/src/components/main/body/sessions/title-input.tsx (1)
TitleInput(6-32)apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
NoteInput(12-38)apps/desktop2/src/components/main/body/sessions/floating/index.tsx (1)
FloatingActionButton(8-18)
apps/desktop2/src/components/main/sidebar/timeline.tsx (4)
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
TimelineView(14-108)packages/ui/src/components/ui/button.tsx (1)
Button(38-69)apps/desktop2/src/utils/timeline.ts (4)
TimelineBucket(30-34)TimelineItem(24-26)TimelinePrecision(28-28)buildTimelineBuckets(112-200)apps/desktop2/src/components/interactive-button.tsx (1)
InteractiveButton(15-71)
apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (4)
packages/db/src/schema.ts (1)
folders(30-34)packages/ui/src/components/ui/dropdown-menu.tsx (8)
DropdownMenu(180-180)DropdownMenuTrigger(194-194)DropdownMenuContent(182-182)DropdownMenuItem(184-184)DropdownMenuSub(191-191)DropdownMenuSubTrigger(193-193)DropdownMenuSubContent(192-192)DropdownMenuSeparator(189-189)packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/components/ui/switch.tsx (1)
Switch(57-57)
apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx (3)
apps/desktop2/src/store/zustand/tabs/index.ts (1)
useTabs(15-20)packages/ui/src/components/block/meeting-metadata-chip.tsx (3)
MeetingMetadata(47-56)MeetingParticipant(58-68)MeetingMetadataChip(85-209)apps/desktop2/src/hooks/useQuery.ts (1)
useQuery(3-46)
packages/ui/src/components/block/meeting-metadata-chip.tsx (5)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/components/ui/popover.tsx (3)
Popover(85-85)PopoverTrigger(85-85)PopoverContent(85-85)packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/avatar.tsx (2)
Avatar(58-58)AvatarFallback(58-58)packages/ui/src/components/icons/linkedin.tsx (1)
LinkedInIcon(7-23)
apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (2)
packages/ui/src/components/ui/breadcrumb.tsx (6)
Breadcrumb(108-108)BreadcrumbList(112-112)BreadcrumbSeparator(114-114)BreadcrumbItem(110-110)BreadcrumbLink(111-111)BreadcrumbPage(113-113)packages/ui/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(180-180)DropdownMenuTrigger(194-194)DropdownMenuContent(182-182)DropdownMenuItem(184-184)
apps/desktop2/src/components/main/body/shared.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)
apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/index.tsx (4)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop2/src/components/main/body/search.tsx (1)
Search(8-107)apps/desktop2/src/store/zustand/tabs/index.ts (1)
useTabs(15-20)
🔇 Additional comments (11)
packages/ui/src/components/ui/button.tsx (2)
4-4: LGTM: Clean refactor to shared Spinner component.The refactor from inline SVG to the dedicated
Spinnercomponent improves maintainability and consistency across the UI.Also applies to: 64-64
17-17: LGTM: Visual spacing refinement.The gap reduction from
gap-2togap-1tightens the spacing between button content, which appears to be an intentional UI polish aligned with this PR's objectives.apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
102-103: LGTM! Button structure simplified.Removing the wrapper div is correct—the Button component accepts children directly.
apps/desktop2/src/components/main/sidebar/timeline.tsx (3)
286-342: LGTM! Clean indicator positioning logic.The TodayBucket correctly positions the CurrentTimeIndicator based on item timestamps and handles the empty state gracefully.
364-378: LGTM! Efficient time updates.The one-minute interval for current time updates is appropriate for timeline precision, and the cleanup is properly handled.
396-402: LGTM! Proper forwardRef usage.The component correctly uses forwardRef and sets displayName for better debugging.
apps/desktop2/src/components/main/body/index.tsx (1)
281-283: LGTM on container/padding tweak.Simpler layout (p-2, rounded border) is clean and consistent.
apps/desktop2/src/components/main/body/sessions/index.tsx (2)
41-47: Layout restructure looks good.Header before inputs + compact container spacing is sane. No logic changes observed.
41-49: FAB positioning is correctly implemented.The FloatingActionButton is nested within StandardTabWrapper, which contains a relative positioned inner div (at line 282 of apps/desktop2/src/components/main/body/index.tsx with className including "relative"). This ensures the absolute positioned FAB anchors within the tab container, not the page, as required.
packages/ui/src/components/icons/linkedin.tsx (1)
7-25: SVG implementation is solid.ForwardRef, size prop, currentColor, and displayName are all correct.
apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx (1)
210-221: Overall integration reads clean.Memoized metadata, chip wiring, and add/remove handlers look good.
| const sessionRow = persisted.UI.useRow("sessions", sessionId, persisted.STORE_ID); | ||
|
|
||
| const eventRow = persisted.UI.useRow( | ||
| "events", | ||
| sessionRow.event_id || "dummy-event-id", | ||
| persisted.STORE_ID, | ||
| ); | ||
|
|
There was a problem hiding this comment.
Null-safety: guard sessionRow access; stabilize dependencies.
Avoid crashing when the session row is briefly undefined and keep the hook stable.
- const sessionRow = persisted.UI.useRow("sessions", sessionId, persisted.STORE_ID);
-
- const eventRow = persisted.UI.useRow(
- "events",
- sessionRow.event_id || "dummy-event-id",
- persisted.STORE_ID,
- );
+ const sessionRow = persisted.UI.useRow("sessions", sessionId, persisted.STORE_ID);
+ const eventId = (sessionRow?.event_id as string | undefined) ?? "dummy-event-id";
+ const eventRow = persisted.UI.useRow("events", eventId, persisted.STORE_ID);
@@
- const meetingMetadata: MeetingMetadata | null = useMemo(() => {
- if (!sessionRow.event_id || !eventRow || !eventRow.started_at || !eventRow.ended_at) {
+ const meetingMetadata: MeetingMetadata | null = useMemo(() => {
+ if (!eventId || !eventRow || !eventRow.started_at || !eventRow.ended_at) {
return null;
}
@@
- }, [sessionRow.event_id, eventRow, store, participantMappingIds]);
+ }, [eventId, eventRow, store, participantMappingIds]);Also applies to: 42-46, 84-84
| if ( | ||
| name && !name.toLowerCase().includes(normalizedQuery) | ||
| && (!email || !email.toLowerCase().includes(normalizedQuery)) | ||
| ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Search filter lets non-matching rows through.
People without name (or email) can bypass the filter; require a match in either.
- if (
- name && !name.toLowerCase().includes(normalizedQuery)
- && (!email || !email.toLowerCase().includes(normalizedQuery))
- ) {
- return;
- }
+ const matches =
+ (name?.toLowerCase().includes(normalizedQuery) ?? false)
+ || (email?.toLowerCase().includes(normalizedQuery) ?? false);
+ if (!matches) {
+ return;
+ }📝 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.
| if ( | |
| name && !name.toLowerCase().includes(normalizedQuery) | |
| && (!email || !email.toLowerCase().includes(normalizedQuery)) | |
| ) { | |
| return; | |
| } | |
| const matches = | |
| (name?.toLowerCase().includes(normalizedQuery) ?? false) | |
| || (email?.toLowerCase().includes(normalizedQuery) ?? false); | |
| if (!matches) { | |
| return; | |
| } |
🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx
around lines 135-140, the current filter lets rows through when one field is
missing (e.g., no name) because it only checks absence of name match and then
allows missing email; change the logic to require at least one field to match
the query: compute nameMatches = name?.toLowerCase().includes(normalizedQuery)
and emailMatches = email?.toLowerCase().includes(normalizedQuery), then if
(!nameMatches && !emailMatches) return; this ensures rows are kept only when
either name or email contains the normalized query.
| const handleMoveToFolder = () => { | ||
| // TODO: Implement move to folder functionality | ||
| console.log("Move to folder"); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Pass the selected folder id to the move handler.
Currently the handler can’t know which folder was chosen.
- const handleMoveToFolder = () => {
+ const handleMoveToFolder = (folderId: string) => {
// TODO: Implement move to folder functionality
- console.log("Move to folder");
+ console.log("Move to folder", folderId);
};
@@
- <DropdownMenuItem key={folder.id} onClick={handleMoveToFolder}>
+ <DropdownMenuItem
+ key={folder.id}
+ onClick={() => handleMoveToFolder(folder.id)}
+ >
<FolderIcon />
<span>{folder.name}</span>
</DropdownMenuItem>Also applies to: 99-104
🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx around
lines 43-47 (and similarly for lines 99-104), the handleMoveToFolder handler is
stubbed and does not receive the selected folder id; update the handler
signature to accept a folderId parameter and ensure the UI element that triggers
it (dropdown/menu item) calls handleMoveToFolder(selectedFolderId). Inside the
handler forward the folderId to the existing move logic (or dispatch/event)
instead of just logging; validate the id is present before proceeding and handle
or surface errors appropriately.
| export interface MeetingMetadataChipProps { | ||
| metadata?: MeetingMetadata | null; | ||
| isVeryNarrow?: boolean; | ||
| isNarrow?: boolean; | ||
| onJoinMeeting?: (meetingLink: string) => void; | ||
| onParticipantClick?: (participant: MeetingParticipant) => void; | ||
| onParticipantAdd?: (participantId: string) => void; | ||
| onParticipantRemove?: (participantId: string) => void; | ||
| participantSearchQuery?: string; | ||
| onParticipantSearchChange?: (query: string) => void; | ||
| participantSearchResults?: MeetingParticipant[]; | ||
| currentUserId?: string; | ||
| formatRelativeDate?: (date: string) => string; | ||
| } |
There was a problem hiding this comment.
Remove unused props from the interface.
The isVeryNarrow, isNarrow, and formatRelativeDate props are declared in the interface but never used in the component implementation.
Based on coding guidelines.
Apply this diff to remove the unused props:
export interface MeetingMetadataChipProps {
metadata?: MeetingMetadata | null;
- isVeryNarrow?: boolean;
- isNarrow?: boolean;
onJoinMeeting?: (meetingLink: string) => void;
onParticipantClick?: (participant: MeetingParticipant) => void;
onParticipantAdd?: (participantId: string) => void;
onParticipantRemove?: (participantId: string) => void;
participantSearchQuery?: string;
onParticipantSearchChange?: (query: string) => void;
participantSearchResults?: MeetingParticipant[];
currentUserId?: string;
- formatRelativeDate?: (date: string) => string;
}📝 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.
| export interface MeetingMetadataChipProps { | |
| metadata?: MeetingMetadata | null; | |
| isVeryNarrow?: boolean; | |
| isNarrow?: boolean; | |
| onJoinMeeting?: (meetingLink: string) => void; | |
| onParticipantClick?: (participant: MeetingParticipant) => void; | |
| onParticipantAdd?: (participantId: string) => void; | |
| onParticipantRemove?: (participantId: string) => void; | |
| participantSearchQuery?: string; | |
| onParticipantSearchChange?: (query: string) => void; | |
| participantSearchResults?: MeetingParticipant[]; | |
| currentUserId?: string; | |
| formatRelativeDate?: (date: string) => string; | |
| } | |
| export interface MeetingMetadataChipProps { | |
| metadata?: MeetingMetadata | null; | |
| onJoinMeeting?: (meetingLink: string) => void; | |
| onParticipantClick?: (participant: MeetingParticipant) => void; | |
| onParticipantAdd?: (participantId: string) => void; | |
| onParticipantRemove?: (participantId: string) => void; | |
| participantSearchQuery?: string; | |
| onParticipantSearchChange?: (query: string) => void; | |
| participantSearchResults?: MeetingParticipant[]; | |
| currentUserId?: string; | |
| } |
🤖 Prompt for AI Agents
In packages/ui/src/components/block/meeting-metadata-chip.tsx around lines 70 to
83, the MeetingMetadataChipProps interface declares isVeryNarrow, isNarrow, and
formatRelativeDate but these props are not used in the component; remove those
three unused properties from the interface definition to match the
implementation and coding guidelines, and run typecheck to ensure no other
references to them remain elsewhere in the codebase.
| onBlur={() => { | ||
| // Delay to allow click on results | ||
| setTimeout(() => setIsFocused(false), 200); | ||
| }} |
There was a problem hiding this comment.
Add cleanup for setTimeout to prevent memory leak.
The setTimeout in the onBlur handler lacks cleanup. If the component unmounts before the 200ms delay completes, setIsFocused will be called on an unmounted component, causing a memory leak and potential React warnings.
Consider using a ref to track the timeout and clean it up:
function ParticipantsSection({
participants,
searchQuery,
searchResults,
onSearchChange,
onParticipantAdd,
onParticipantClick,
onParticipantRemove,
currentUserId,
}: ParticipantsSectionProps) {
const [isFocused, setIsFocused] = useState(false);
const [selectedIndex, setSelectedIndex] = useState(-1);
+ const blurTimeoutRef = useRef<NodeJS.Timeout | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (blurTimeoutRef.current) {
+ clearTimeout(blurTimeoutRef.current);
+ }
+ };
+ }, []);
// ... rest of component
onFocus={() => setIsFocused(true)}
onBlur={() => {
+ if (blurTimeoutRef.current) {
+ clearTimeout(blurTimeoutRef.current);
+ }
// Delay to allow click on results
- setTimeout(() => setIsFocused(false), 200);
+ blurTimeoutRef.current = setTimeout(() => setIsFocused(false), 200);
}}Don't forget to import useRef and useEffect from React.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/ui/src/components/block/meeting-metadata-chip.tsx around lines 311
to 314, the onBlur handler sets a 200ms setTimeout without cleanup; change it to
store the timeout ID in a useRef, clear any existing timeout before scheduling a
new one, and clear the timeout in a useEffect cleanup when the component
unmounts so setIsFocused is not called after unmount; also add imports for
useRef and useEffect from React.
No description provided.