Refactor with note-input and more tabs lifecycle hooks #1573
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR removes the daily tab feature, replaces the session editor header with a new NoteInput tabbed editor (raw/enhanced/transcript), adds an on-empty tab lifecycle hook and notify flow, and hardens date/timeline utilities with nullish guards and minor API edits. Changes
Sequence Diagram(s)sequenceDiagram
participant Layout as Main Layout
participant TabStore as Tab Store
participant Persisted as Persisted Store
participant SessionTab as Session Tab
participant NoteInput as NoteInput
participant Editors as Editors (Raw/Enhanced/Transcript)
Note over Layout: Register empty-state handler on mount
Layout->>TabStore: registerOnEmpty(handler)
TabStore-->>Layout: unregister function
Note over TabStore: When last tab closed
TabStore->>TabStore: notifyEmpty(onEmptyHandlers)
TabStore->>Layout: call onEmpty handler
Layout->>Persisted: create default session row
Persisted-->>Layout: sessionId
Layout->>TabStore: openNew(sessionId)
Note over SessionTab: User views session
SessionTab->>NoteInput: render with tab prop
NoteInput->>TabStore: read/write session editor view state
alt editor view is "raw"
NoteInput->>Editors: render RawEditor(sessionId)
else editor view is "enhanced"
NoteInput->>Editors: render EnhancedEditor(sessionId)
else editor view is "transcript"
NoteInput->>Editors: render TranscriptEditorWrapper(sessionId)
end
Editors->>Persisted: useCell / useSetPartialRowCallback
Persisted-->>Editors: initialContent / ack saves
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
apps/desktop2/src/components/main/body/sessions/note-input/transcript.tsx
Show resolved
Hide resolved
![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/desktop2/src/components/main/body/sessions/index.tsx (1)
51-53: Remove theconsole.logdebug statement; component export is correct.The
FloatingActionButtonnexport name is correct throughout the codebase (floating-action.tsx, import, and usage are all consistent). However, the debugconsole.logat lines 51–53 should be removed to follow coding guidelines that prohibit unnecessary code and debug statements left in production code.
🧹 Nitpick comments (5)
apps/desktop2/src/utils/timeline.ts (2)
131-131: Redundant truthiness check after isNaN validation.The
eventStartTimevariable is guaranteed to be a valid Date object at this point because the code already checksisNaN(eventStartTime.getTime())above (lines 127-129). The additionalif (eventStartTime && ...)check is redundant.Apply this diff to remove the redundant check:
- if (eventStartTime && !isPast(eventStartTime)) { + if (!isPast(eventStartTime)) {
156-163: Redundant truthiness check after isNaN validation.Similar to the event handling above, the
datevariable is guaranteed to be valid after theisNaN(date.getTime())check (lines 152-154), making theif (date)check redundant.Apply this diff to simplify:
- if (date) { - items.push({ - type: "session", - id: sessionId, - date: format(date, "yyyy-MM-dd"), - data: row as unknown as persisted.Session, - }); - } + items.push({ + type: "session", + id: sessionId, + date: format(date, "yyyy-MM-dd"), + data: row as unknown as persisted.Session, + });apps/desktop2/src/store/zustand/tabs/basic.ts (1)
103-118: onEmpty lifecycle: correct placement; minor type-cast nit
- Good: You reset state before invoking notifyEmpty, and early return prevents further history/flag work. The closeAll flow mirrors this correctly.
- Minor: closeAll uses
as unknown as Partial<T>while other setters useas Partial<T>. Consider unifying for consistency unless TS constraints requireunknown.Also applies to: 167-177
apps/desktop2/src/components/main/body/sessions/index.tsx (1)
64-65: Use functional updater to avoid stale togglePrefer functional form to prevent stale closures:
setShowAudioPlayer(v => !v).- onToggleAudioPlayer={() => setShowAudioPlayer(!showAudioPlayer)} + onToggleAudioPlayer={() => setShowAudioPlayer(v => !v)}apps/desktop2/src/store/zustand/tabs/lifecycle.ts (1)
33-44: Consider extracting the registration logic to reduce duplication.The
registerOnEmptyimplementation duplicates the registration pattern fromregisterOnClose(lines 21-32). Both functions follow identical logic: get current handlers, create new Set, add/delete handler, update state, return unregister function.Consider extracting a helper to eliminate the duplication:
+const createRegisterHandler = <T extends LifecycleState, H>( + handlerKey: 'onCloseHandlers' | 'onEmptyHandlers', + get: StoreApi<T>["getState"], + set: StoreApi<T>["setState"], +) => (handler: H) => { + const currentHandlers = get()[handlerKey] as Set<H>; + const nextHandlers = new Set(currentHandlers); + nextHandlers.add(handler); + set({ [handlerKey]: nextHandlers } as Partial<T>); + return () => { + const currentHandlers = get()[handlerKey] as Set<H>; + const nextHandlers = new Set(currentHandlers); + nextHandlers.delete(handler); + set({ [handlerKey]: nextHandlers } as Partial<T>); + }; +}; + export const createLifecycleSlice = <T extends LifecycleState>( set: StoreApi<T>["setState"], get: StoreApi<T>["getState"], ): LifecycleState & LifecycleActions => ({ onCloseHandlers: new Set(), onEmptyHandlers: new Set(), - registerOnClose: (handler) => { - const { onCloseHandlers } = get(); - const nextHandlers = new Set(onCloseHandlers); - nextHandlers.add(handler); - set({ onCloseHandlers: nextHandlers } as Partial<T>); - return () => { - const { onCloseHandlers: currentHandlers } = get(); - const nextHandlers = new Set(currentHandlers); - nextHandlers.delete(handler); - set({ onCloseHandlers: nextHandlers } as Partial<T>); - }; - }, - registerOnEmpty: (handler) => { - const { onEmptyHandlers } = get(); - const nextHandlers = new Set(onEmptyHandlers); - nextHandlers.add(handler); - set({ onEmptyHandlers: nextHandlers } as Partial<T>); - return () => { - const { onEmptyHandlers: currentHandlers } = get(); - const nextHandlers = new Set(currentHandlers); - nextHandlers.delete(handler); - set({ onEmptyHandlers: nextHandlers } as Partial<T>); - }; - }, + registerOnClose: createRegisterHandler<T, (tab: Tab) => void>('onCloseHandlers', get, set), + registerOnEmpty: createRegisterHandler<T, () => void>('onEmptyHandlers', get, set), });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/desktop2/src/components/main/body/daily.tsx(0 hunks)apps/desktop2/src/components/main/body/index.tsx(0 hunks)apps/desktop2/src/components/main/body/sessions/index.tsx(2 hunks)apps/desktop2/src/components/main/body/sessions/inner-header.tsx(0 hunks)apps/desktop2/src/components/main/body/sessions/note-input/enhanced.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/note-input/transcript.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/title-input.tsx(3 hunks)apps/desktop2/src/components/main/sidebar/profile/index.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/timeline/anchor.ts(3 hunks)apps/desktop2/src/components/main/sidebar/timeline/index.tsx(1 hunks)apps/desktop2/src/routes/app/main/_layout.tsx(2 hunks)apps/desktop2/src/store/tinybase/persisted.ts(2 hunks)apps/desktop2/src/store/zustand/tabs/basic.ts(4 hunks)apps/desktop2/src/store/zustand/tabs/lifecycle.ts(2 hunks)apps/desktop2/src/store/zustand/tabs/schema.ts(0 hunks)apps/desktop2/src/store/zustand/tabs/utils.ts(1 hunks)apps/desktop2/src/utils/timeline.ts(2 hunks)packages/tiptap/src/transcript/utils.ts(1 hunks)
💤 Files with no reviewable changes (4)
- apps/desktop2/src/store/zustand/tabs/schema.ts
- apps/desktop2/src/components/main/body/sessions/inner-header.tsx
- apps/desktop2/src/components/main/body/index.tsx
- apps/desktop2/src/components/main/body/daily.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/utils/timeline.tsapps/desktop2/src/store/zustand/tabs/basic.tsapps/desktop2/src/components/main/body/sessions/note-input/index.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/store/zustand/tabs/lifecycle.tsapps/desktop2/src/components/main/body/sessions/note-input/enhanced.tsxapps/desktop2/src/routes/app/main/_layout.tsxapps/desktop2/src/store/zustand/tabs/utils.tspackages/tiptap/src/transcript/utils.tsapps/desktop2/src/components/main/body/sessions/note-input/raw.tsxapps/desktop2/src/store/tinybase/persisted.tsapps/desktop2/src/components/main/body/sessions/title-input.tsxapps/desktop2/src/components/main/body/sessions/note-input/transcript.tsxapps/desktop2/src/components/main/sidebar/profile/index.tsxapps/desktop2/src/components/main/body/sessions/index.tsxapps/desktop2/src/components/main/sidebar/timeline/anchor.ts
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/note-input/index.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/components/main/body/sessions/note-input/enhanced.tsxapps/desktop2/src/routes/app/main/_layout.tsxapps/desktop2/src/components/main/body/sessions/note-input/raw.tsxapps/desktop2/src/components/main/body/sessions/title-input.tsxapps/desktop2/src/components/main/body/sessions/note-input/transcript.tsxapps/desktop2/src/components/main/sidebar/profile/index.tsxapps/desktop2/src/components/main/body/sessions/index.tsx
🧬 Code graph analysis (7)
apps/desktop2/src/utils/timeline.ts (2)
packages/utils/src/datetime.ts (1)
format(4-11)apps/desktop2/src/store/tinybase/persisted.ts (1)
Session(102-102)
apps/desktop2/src/store/zustand/tabs/basic.ts (1)
apps/desktop2/src/store/zustand/tabs/utils.ts (2)
notifyTabClose(10-21)notifyEmpty(30-40)
apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (5)
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/body/sessions/note-input/enhanced.tsx (1)
EnhancedEditor(5-29)apps/desktop2/src/components/main/body/sessions/note-input/raw.tsx (1)
RawEditor(5-29)apps/desktop2/src/components/main/body/sessions/note-input/transcript.tsx (1)
TranscriptEditorWrapper(6-42)
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
apps/desktop2/src/components/main/sidebar/timeline/anchor.ts (1)
useAutoScrollToAnchor(61-110)
apps/desktop2/src/store/zustand/tabs/lifecycle.ts (2)
apps/desktop2/src/store/zustand/tabs/schema.ts (1)
Tab(50-50)apps/desktop2/src/store/zustand/tabs/index.ts (1)
Tab(8-8)
apps/desktop2/src/routes/app/main/_layout.tsx (1)
apps/desktop2/src/store/zustand/tabs/index.ts (1)
useTabs(15-20)
apps/desktop2/src/components/main/body/sessions/index.tsx (2)
apps/desktop2/src/components/main/body/shared.tsx (1)
TabItem(21-21)apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
NoteInput(16-66)
⏰ 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). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: ci (macos, macos-14)
- GitHub Check: zizmor
🔇 Additional comments (14)
packages/tiptap/src/transcript/utils.ts (1)
40-40: Good linter-friendly rename.Renaming index to _index cleanly avoids unused parameter warnings without changing behavior. LGTM.
apps/desktop2/src/components/main/sidebar/profile/index.tsx (1)
2-2: LGTM! Daily note feature cleanly removed.The removal of the FileText import and daily note menu item is consistent with the PR's objective to eliminate the daily tab feature.
Also applies to: 50-55
apps/desktop2/src/store/tinybase/persisted.ts (2)
420-430: LGTM! Proper date validation added.The nullish guards prevent invalid or missing dates from causing issues in the index. This aligns with the PR's broader effort to harden date utilities.
444-454: LGTM! Consistent date validation.Similar guard pattern applied to session dates, ensuring consistency across the codebase.
apps/desktop2/src/components/main/body/sessions/note-input/raw.tsx (2)
21-26: Clarify if empty mention search is intentional.The
handleSearchfunction returns an empty array, effectively disabling mention functionality. If this is a placeholder for future implementation, consider adding a TODO comment or removing the mentionConfig entirely until the feature is ready.
5-29: LGTM! Editor integration is well-structured.The component properly integrates with the persisted store and follows React best practices with the key prop for remounting when sessionId changes.
apps/desktop2/src/routes/app/main/_layout.tsx (2)
37-50: Empty tab lifecycle integration looks good.The logic appropriately creates a default session when no tab exists and registers the handler for future empty-state events.
37-50: The handler registration lifecycle is correct; no duplicates are added.The implementation of
registerOnEmpty(lines 33–43 inlifecycle.ts) correctly registers and cleans up handlers using aSet. When the effect re-runs due to dependency changes, the cleanup function removes the previous handler before the new one is registered, preventing duplicates. Each closure ofcreateDefaultSessionis distinct, and the lifecycle management ensures only the current handler remains active.Likely an incorrect or invalid review comment.
apps/desktop2/src/components/main/body/sessions/title-input.tsx (1)
11-18: LGTM! Clean API simplification.The onChange signature change from
(e: ChangeEvent<HTMLInputElement>) => voidto(value: string) => voidimproves the component's API by passing the value directly to consumers.Also applies to: 51-51
apps/desktop2/src/components/main/sidebar/timeline/anchor.ts (1)
1-1: LGTM! Useful enhancement to scroll behavior.The optional
depsparameter allows external state changes (like Today bucket length) to trigger scroll recalculation, improving the user experience when timeline content changes dynamically.Also applies to: 65-70, 101-109
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
27-30: LGTM! Proper integration with anchor deps.The
todayBucketLengthcomputation and its use as a dependency ensures the timeline auto-scrolls when Today bucket content changes, complementing the new deps parameter inuseAutoScrollToAnchor.Also applies to: 36-36
apps/desktop2/src/store/zustand/tabs/utils.ts (1)
30-40: notifyEmpty mirrors existing pattern — LGTMMatches notifyTabClose’s error-handling approach; straightforward and consistent.
apps/desktop2/src/components/main/body/sessions/note-input/enhanced.tsx (1)
1-29: Enhanced editor wiring looks solidState hookup and NoteEditor config are consistent and minimal.
apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
38-44: cn usage conforms to Tailwind composition guidelines — LGTMArray-based grouping and conditional classes are applied correctly.
No description provided.