Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
📝 WalkthroughWalkthroughThe PR upgrades React/TanStack dependencies, widens/refines ref nullability/types across components/hooks/stores, replaces zod-adapter validators with direct validateSearch schemas, refactors desktop2 routing/UI (new /app/main layout, removed old route), updates TipTap configs/APIs, and adds Tauri window permissions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router
participant Route as /app/main/_layout.index
participant Store as persistedStore
User->>Router: Navigate to /app/main with search
Router->>Route: beforeLoad(search)
alt search.new == true
Route->>Store: insert sessions/{id} with defaults
Route-->>Router: redirect to /app/main?tabs=[...new active]&new=false
else search.tabs missing or empty
Route-->>Router: redirect to /app/main?new=true
else tabs provided
opt normalize active
Route->>Route: ensure exactly one active tab
end
Route-->>Router: redirect to /app/main?tabs=[normalized]
end
Router->>Route: loader(tabs)
Route->>Route: render LeftSidebar + MainHeader + MainContent (+Chat if expanded)
note over Route: validateSearch applied to search params
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 ignore |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/editor-area/index.tsx (1)
175-185: Fix useEffect dependency array.The dependency
[editorRef.current?.editor]won't trigger re-renders because ref mutations don't cause React to re-run effects. This effect likely won't fire when the editor changes.Apply this diff to fix the dependency array:
useEffect(() => { if (editorRef.current?.editor) { globalEditorRef.current = editorRef.current.editor; } - // Clear on unmount return () => { if (globalEditorRef.current === editorRef.current?.editor) { globalEditorRef.current = null; } }; - }, [editorRef.current?.editor]); + }, []);Since
editorRefis initialized with a stable object ({ editor: null }), and theEditorcomponent presumably assigns toeditorRef.current.editordirectly, you should trigger this effect via a separate state change or callback from the Editor component when it mounts/unmounts.
🧹 Nitpick comments (1)
apps/desktop/src/components/editor-area/index.tsx (1)
324-326: Type cast is correct but verbose.The union type cast correctly aligns with the updated ref shapes.
Consider extracting a type alias to reduce verbosity:
type EditorRefType = | React.RefObject<TranscriptEditorRef | null> | React.RefObject<{ editor: TiptapEditor | null }>; // Then use: editorRef={... as EditorRefType}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/desktop2/src/routeTree.gen.tsis excluded by!**/*.gen.tspnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
apps/desktop/package.json(3 hunks)apps/desktop/src/components/editor-area/index.tsx(2 hunks)apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx(1 hunks)apps/desktop/src/components/editor-area/note-header/index.tsx(2 hunks)apps/desktop/src/components/editor-area/text-selection-popover.tsx(3 hunks)apps/desktop/src/components/right-panel/utils/chat-utils.ts(1 hunks)apps/desktop/src/contexts/search.tsx(1 hunks)apps/desktop/src/hooks/use-container-width.ts(1 hunks)apps/desktop/src/routes/app.control.tsx(1 hunks)apps/desktop/src/routes/app.finder.tsx(2 hunks)apps/desktop/src/routes/app.new.tsx(1 hunks)apps/desktop/src/routes/app.settings.tsx(2 hunks)apps/desktop/src/routes/video.tsx(1 hunks)apps/desktop/src/stores/search.ts(3 hunks)apps/desktop2/package.json(1 hunks)apps/desktop2/src-tauri/capabilities/default.json(1 hunks)apps/desktop2/src/components/main/left-sidebar.tsx(1 hunks)apps/desktop2/src/components/main/main-area.tsx(1 hunks)apps/desktop2/src/hooks/useTabs.ts(1 hunks)apps/desktop2/src/routes/app._layout.main.index.tsx(0 hunks)apps/desktop2/src/routes/app/main/_layout.index.tsx(1 hunks)apps/desktop2/src/routes/app/main/_layout.tsx(1 hunks)apps/desktop2/src/routes/app/settings.tsx(1 hunks)packages/tiptap/package.json(3 hunks)packages/tiptap/src/editor/mention.tsx(2 hunks)packages/tiptap/src/shared/extensions.ts(1 hunks)packages/tiptap/src/transcript/views.tsx(2 hunks)packages/ui/package.json(2 hunks)packages/ui/src/components/ui/particles.tsx(1 hunks)packages/ui/src/components/ui/splash.tsx(1 hunks)packages/utils/package.json(1 hunks)packages/utils/src/contexts/right-panel.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop2/src/routes/app._layout.main.index.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/splash.tsxapps/desktop/src/components/right-panel/utils/chat-utils.tsapps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsxapps/desktop2/src/hooks/useTabs.tsapps/desktop/src/hooks/use-container-width.tsapps/desktop/src/routes/app.control.tsxpackages/tiptap/src/transcript/views.tsxapps/desktop/src/stores/search.tsapps/desktop/src/contexts/search.tsxapps/desktop/src/components/editor-area/index.tsxapps/desktop2/src/routes/app/main/_layout.tsxapps/desktop/src/routes/app.settings.tsxapps/desktop/src/components/editor-area/note-header/index.tsxapps/desktop2/src/routes/app/main/_layout.index.tsxpackages/ui/src/components/ui/particles.tsxpackages/tiptap/src/editor/mention.tsxapps/desktop2/src/components/main/main-area.tsxapps/desktop/src/routes/video.tsxapps/desktop2/src/routes/app/settings.tsxapps/desktop2/src/components/main/left-sidebar.tsxpackages/tiptap/src/shared/extensions.tsapps/desktop/src/routes/app.finder.tsxapps/desktop/src/routes/app.new.tsxpackages/utils/src/contexts/right-panel.tsxapps/desktop/src/components/editor-area/text-selection-popover.tsx
🧬 Code graph analysis (7)
apps/desktop/src/components/editor-area/index.tsx (2)
packages/tiptap/src/editor/index.tsx (1)
TiptapEditor(11-11)packages/tiptap/src/transcript/index.tsx (1)
TranscriptEditorRef(33-41)
apps/desktop2/src/routes/app/main/_layout.tsx (1)
apps/desktop2/src/routes/app/main/_layout.index.tsx (1)
Route(17-73)
apps/desktop2/src/routes/app/main/_layout.index.tsx (6)
apps/desktop2/src/types/index.ts (1)
tabSchema(16-18)packages/utils/src/contexts/right-panel.tsx (1)
useRightPanel(112-118)apps/desktop2/src/components/main/left-sidebar.tsx (1)
LeftSidebar(15-46)apps/desktop2/src/components/main/main-area.tsx (2)
MainHeader(22-62)MainContent(11-20)packages/ui/src/components/ui/resizable.tsx (3)
ResizablePanelGroup(43-43)ResizablePanel(43-43)ResizableHandle(43-43)apps/desktop2/src/components/chat.tsx (1)
Chat(7-92)
apps/desktop2/src/components/main/main-area.tsx (2)
apps/desktop2/src/hooks/useTabs.ts (1)
useTabs(4-76)packages/ui/src/components/block/title-input.tsx (1)
TitleInput(12-59)
apps/desktop/src/routes/video.tsx (4)
apps/desktop/src/routes/app.finder.tsx (1)
Route(19-162)apps/desktop/src/routes/app.new.tsx (1)
Route(11-84)apps/desktop/src/routes/app.settings.tsx (1)
Route(32-35)apps/desktop2/src/routes/app/main/_layout.index.tsx (1)
Route(17-73)
apps/desktop/src/routes/app.new.tsx (4)
apps/desktop/src/routes/app.finder.tsx (1)
Route(19-162)apps/desktop/src/routes/app.settings.tsx (1)
Route(32-35)apps/desktop/src/routes/video.tsx (1)
Route(17-24)apps/desktop2/src/routes/app/main/_layout.index.tsx (1)
Route(17-73)
apps/desktop/src/components/editor-area/text-selection-popover.tsx (1)
packages/tiptap/src/editor/index.tsx (1)
TiptapEditor(11-11)
⏰ 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 (28)
packages/ui/src/components/ui/splash.tsx (1)
26-26: LGTM: Type refinement aligns with React 19 stricter typing.The change from
useRef<number>(null)touseRef<number | null>(null)corrects a type mismatch between the generic parameter and the initialization value. The null check on lines 48-50 ensures safe usage before callingcancelAnimationFrame.packages/ui/src/components/ui/particles.tsx (1)
94-94: LGTM! Type refinement improves explicitness.The change to explicitly type
resizeTimeoutasReturnType<typeof setTimeout> | nullwith null initialization is correct and aligns with the PR's broader goal of refining ref nullability. All usage sites (lines 104-106, 118-120) properly check for null before clearing the timeout.packages/tiptap/src/transcript/views.tsx (2)
3-3: LGTM! Import properly supports type refinement.The
ReactElementimport is correctly added to support the type signature change on line 72.
72-72: LGTM! Type refinement aligns with React 19.The change from
JSX.ElementtoReactElementis appropriate for React 19 compatibility.ReactElementis more generic and allows better type inference while remaining backward compatible.packages/tiptap/src/shared/extensions.ts (1)
19-22: LGTM! Clean extension configuration pattern.Disabling StarterKit defaults and replacing them with custom-configured extensions is the correct approach. The
listKeymap: falseis particularly important since you're providingCustomListKeymapwith custom behavior.Note:
underline: falseandlink: falseare technically unnecessary since StarterKit doesn't include these extensions by default, but they serve as defensive coding and documentation of intent, which is harmless.apps/desktop/src/contexts/search.tsx (1)
26-26: LGTM! Ref nullability pattern aligns with React 19.The explicit
| nulltype annotation is consistent with React 19's stricter ref typing. The code already guards all ref accesses with optional chaining, so this change is safe.apps/desktop2/src/routes/app/settings.tsx (1)
3-4: LGTM! Path adjustments reflect route restructuring.The import path updates from
../to../../are consistent with the route file being moved one level deeper in the directory structure.packages/tiptap/package.json (1)
13-14: Dependency updates look good.The React 19 upgrade, TanStack Router minor bump (^1.132.37), and lucide-react patch update (^0.544.0) are all part of the coordinated workspace-wide upgrade.
Also applies to: 23-23, 45-45, 53-54
apps/desktop/src/components/right-panel/utils/chat-utils.ts (1)
31-35: LGTM! Nullable ref type is safely handled.The function signature update to accept
React.RefObject<HTMLTextAreaElement | null>is consistent with the broader nullability pattern. The existing guard at line 32 safely handles null refs.apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (1)
426-426: LGTM! Prop type aligns with workspace-wide ref pattern.The nullable ref type is consistent with the React 19 upgrade. The component already safely handles null refs with optional chaining (lines 498, 502).
packages/ui/package.json (1)
41-41: Dependency updates complete the workspace-wide upgrade.The React 19 and lucide-react updates in the UI package complete the coordinated upgrade across all workspace packages (utils, tiptap, ui, desktop, desktop2).
Also applies to: 53-54, 58-59
packages/utils/package.json (1)
33-33: React 19 upgrade verified across workspaceAll packages now consistently depend on React ^19.2 and @types/react ^19.2.0. Confirm compatibility with React 19’s stricter ref and hooks type requirements via your test suite.
apps/desktop/src/routes/app.control.tsx (1)
230-230: No changes required fortoolbarReftype.
useRef<HTMLDivElement>(null)returns aRefObject<HTMLDivElement>whose.currentisHTMLDivElement | null, making it compatible withReact.RefObject<HTMLDivElement | null>.Likely an incorrect or invalid review comment.
apps/desktop/src/hooks/use-container-width.ts (1)
3-3: LGTM! Type broadening aligns with React 19 patterns.The nullability refinement is safe since the runtime guard at lines 17-19 already handles null refs.
packages/tiptap/src/editor/mention.tsx (2)
129-129: LGTM! Removed unused parameter.The
editorparameter was not used in the function body.
155-155: LGTM! Removed unused error parameter.The error parameter was not used in the catch block.
apps/desktop/src/components/editor-area/text-selection-popover.tsx (1)
7-7: LGTM! Type safety improvements.Replacing
anywithTiptapEditor | nulland adding nullability todelayTimeoutRefimproves type safety and aligns with React 19 patterns.Also applies to: 16-16, 28-28
packages/utils/src/contexts/right-panel.tsx (1)
16-16: LGTM! Consistent nullability refinement.The type changes align with the broader pattern of nullable refs across the codebase, and runtime guards at lines 41 and 68 already handle null refs safely.
Also applies to: 32-32
apps/desktop/package.json (1)
59-60: Verify package versions and security advisories.Similar to apps/desktop2/package.json, verify these upgraded packages for security and compatibility. You can use the same verification script provided in the review comment for apps/desktop2/package.json.
Also applies to: 84-84, 88-90, 110-110, 116-117
apps/desktop/src/stores/search.ts (1)
32-32: LGTM! Consistent ref nullability pattern.The type refinements align with the broader codebase pattern, and runtime guards at lines 156 and 163 already handle null refs safely.
Also applies to: 42-42, 166-166
apps/desktop/src/components/editor-area/note-header/index.tsx (1)
7-7: LGTM! Import consolidation and consistent type refinement.Moving TitleInput to the shared UI package improves code organization, and the containerRef nullability aligns with the broader pattern in this PR.
Also applies to: 35-35
apps/desktop2/package.json (1)
15-15: Security check passed
- No known vulnerabilities in React 19.2.0, TanStack Router 1.132.37, Zod 4.1.11, or lucide-react 0.544.0
- Validate application compatibility with React 19 before merging
apps/desktop/src/routes/video.tsx (1)
13-18: LGTM! Clean migration to direct zod validation.The migration from
zodValidatorwrapper to directvalidateSearchaligns with TanStack Router's updated validation API. The schema definition and route behavior remain unchanged.apps/desktop/src/components/editor-area/index.tsx (1)
162-163: LGTM! Improved ref initialization.Initializing
editorRefwith{ editor: null }provides a stable reference object, and explicitly typingtranscriptRefas nullable improves type safety.apps/desktop/src/routes/app.finder.tsx (1)
11-20: LGTM! Clean migration with new contact filtering parameters.The migration to direct validation is correct, and the new optional
personIdandorgIdfields properly support the ContactView filtering (used on lines 248-249).apps/desktop/src/routes/app.new.tsx (2)
6-12: LGTM! Clean migration to direct validation.The migration from
zodValidatorto directvalidateSearchis consistent with the updated TanStack Router API.
13-83: Verify error handling behavior after removing try-catch.The
beforeLoadlogic no longer catches errors that might occur during session creation, participant assignment, or query invalidation. These errors will now propagate to TanStack Router's error boundary.Confirm this behavior change is intentional and that route-level error boundaries properly handle failures in:
queryClient.fetchQuery(line 20-23)dbCommands.upsertSession(lines 25-38, 46-59)dbCommands.sessionAddParticipant(lines 41, 60)queryClient.invalidateQueries(lines 71-77)Consider whether any of these operations need explicit error handling or user-facing error messages.
apps/desktop/src/routes/app.settings.tsx (1)
24-33: LGTM! The optional baseUrl and apiKey fields align with deeplink.rs handling.
| <TabsTrigger value="folder" className="flex-1"> | ||
| <FolderOpenIcon /> | ||
| </TabsTrigger> | ||
| <TabsTrigger value="timeline" className="flex-1"> | ||
| <ChartNoAxesGantt /> | ||
| </TabsTrigger> |
There was a problem hiding this comment.
Add accessible labels to icon-only tab triggers.
These triggers render only icons, leaving them unnamed for assistive tech. Add an accessible label (e.g., aria-label or visually hidden text) so screen readers can announce their purpose.
- <TabsTrigger value="folder" className="flex-1">
- <FolderOpenIcon />
- </TabsTrigger>
- <TabsTrigger value="timeline" className="flex-1">
- <ChartNoAxesGantt />
- </TabsTrigger>
+ <TabsTrigger value="folder" className="flex-1" aria-label="Folders">
+ <FolderOpenIcon aria-hidden="true" />
+ </TabsTrigger>
+ <TabsTrigger value="timeline" className="flex-1" aria-label="Timeline">
+ <ChartNoAxesGantt aria-hidden="true" />
+ </TabsTrigger>📝 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.
| <TabsTrigger value="folder" className="flex-1"> | |
| <FolderOpenIcon /> | |
| </TabsTrigger> | |
| <TabsTrigger value="timeline" className="flex-1"> | |
| <ChartNoAxesGantt /> | |
| </TabsTrigger> | |
| <TabsTrigger value="folder" className="flex-1" aria-label="Folders"> | |
| <FolderOpenIcon aria-hidden="true" /> | |
| </TabsTrigger> | |
| <TabsTrigger value="timeline" className="flex-1" aria-label="Timeline"> | |
| <ChartNoAxesGantt aria-hidden="true" /> | |
| </TabsTrigger> |
🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/left-sidebar.tsx around lines 28 to 33, the
icon-only TabsTrigger elements for value="folder" and value="timeline" lack
accessible names; add an accessible label to each (for example
aria-label="Folders" and aria-label="Timeline") or include visually hidden text
within the trigger so screen readers can announce their purpose, keeping labels
concise and matching the tab intent.
✅ Actions performedReviews paused. |
1 participant
No description provided.