Conversation
yujonglee
force-pushed
the
transcript-editor-2
branch
from
d898681 to
7f158f7
Compare
WalkthroughThe changes remove the TinyBase state management integration from the desktop app, including its provider, context, and dependencies. Participant and transcript components are refactored for improved data flow and UI, with new hooks and components introduced for participant grouping and speaker selection. The Tiptap transcript editor is enhanced with richer speaker node handling, a more flexible API, and improved content synchronization. CSS and UI components receive styling and accessibility updates. Localization files and database seeding are updated for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TranscriptView
participant TranscriptEditor
participant SpeakerSelector
User->>TranscriptView: Opens transcript
TranscriptView->>TranscriptEditor: Mounts with initialWords, speaker view component, onUpdate
TranscriptEditor->>SpeakerSelector: Renders speaker popover for each speaker node
User->>SpeakerSelector: Selects/changing speaker
SpeakerSelector->>TranscriptEditor: Updates speaker node attributes
TranscriptEditor->>TranscriptView: Calls onUpdate with new words
TranscriptView->>Session: Saves updated words
sequenceDiagram
participant ParticipantsChip
participant useParticipantsWithOrg
participant ParticipantsChipInner
ParticipantsChip->>useParticipantsWithOrg: Fetch grouped participants by org
useParticipantsWithOrg-->>ParticipantsChip: Returns grouped data
ParticipantsChip->>ParticipantsChipInner: Passes grouped participants and click handler
ParticipantsChipInner->>User: Renders participant groups and handles UI events
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (9)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🔭 Outside diff range comments (1)
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (1)
154-162: 🛠️ Refactor suggestionClickable
<div>should be a<button>for a11y & semantics.The whole row is interactive but rendered as a generic
div.
Screen-reader users won’t be informed that it is clickable, and keyboard users cannot activate it via Space/Enter (except through nested hacks).Replace with a
<button>or addrole="button"and key handlers for Enter/Space on the container.-<div +<button className="flex items-center …" onClick={() => handleClickHuman(member)} + onKeyDown={(e) => { if (['Enter', ' '].includes(e.key)) handleClickHuman(member) }} + role="button" + tabIndex={0} >
🧹 Nitpick comments (7)
apps/desktop/src/components/right-panel/views/exp.tsx (1)
33-35: Consider adding a more informative empty stateThe current implementation returns an empty paragraph when no session ID is found. Consider providing a more informative empty state or error message to improve user experience.
if (!sessionId) { - return <p></p>; + return <p className="text-neutral-400 text-sm">No active session</p>; }packages/tiptap/src/transcript/extensions.ts (2)
4-4: Preferimport typeto avoid clashing with the DOM-globalNode.
Nodeis already declared in the DOM lib. Importing the same identifier as a value pollutes the bundle and can confuse tooling.
Switch to a type-only import and give it an alias for clarity.-import { Node } from "prosemirror-model"; +import type { Node as PMNode } from "prosemirror-model";Then replace occurrences of
Nodein this file withPMNode.🧰 Tools
🪛 GitHub Actions: .github/workflows/desktop_ci.yaml
[error] 4-4: TypeScript error TS2307: Cannot find module 'prosemirror-model' or its corresponding type declarations.
185-349: Consider splitting the monolithicSpeakerCommandsextension for maintainability.The extension now hosts 5 fairly large command bodies (≈160 lines). Future updates will be cumbersome and the file is already dense.
Suggested plan:
- Move pure helpers (e.g., the descendant traversal & update pattern) into a util:
updateSpeakerNodes(tr, predicate, updater).- Keep each high-level command in its own file or at least its own object literal to improve discoverability.
- Re-export a composed extension from
index.ts.Not a blocker but will pay off quickly as the command surface grows.
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (1)
29-35:reduceon a plain object risks prototype leakage – useMap.Using
{}as an accumulator withoutObject.create(null)inheritsObject.prototypekeys (toString, etc.) which could collide with organisation IDs like"toString".
AMap<string, Human[]>is safer and semantically clearer.packages/tiptap/src/transcript/utils.ts (1)
78-110: Type guard can be simplified & made safer withinoperator.Manually indexing
attrs[SPEAKER_ID_ATTR]and coercing may return''(empty string) which is falsy but still truthy for identity existence.
Using anincheck prevents accidental empty-string acceptance.-if (attrs[SPEAKER_ID_ATTR]) { +if (SPEAKER_ID_ATTR in attrs && attrs[SPEAKER_ID_ATTR] !== null) {Also add similar guards for the index attr.
packages/tiptap/src/transcript/nodes.ts (2)
115-136: Traversal logic scans the whole document even when only “before position” nodes are relevant
descendantskeeps walking siblings afterreturn false; it merely skips the current node’s children.
For large docs this is wasted work.A cheaper alternative:
tr.doc.nodesBetween(0, position, (node, pos) => { /* same update logic */ });This bounds the traversal to exactly the range you care about.
Consider applying the same optimisation toreplaceSpeakerIdsAfter.
216-218: AvoidanyinaddCommandsCasting the whole object defeats the purpose of the strongly-typed command interface you extended above.
Expose the commands with proper generics instead:- addCommands() { - return implementCommands as any; - }, + addCommands() { + return implementCommands as typeof implementCommands; + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/desktop/package.json(0 hunks)apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx(3 hunks)apps/desktop/src/components/right-panel/views/exp.tsx(1 hunks)apps/desktop/src/components/right-panel/views/transcript-view.tsx(6 hunks)apps/desktop/src/contexts/index.ts(0 hunks)apps/desktop/src/contexts/tinybase.tsx(0 hunks)apps/desktop/src/locales/en/messages.po(3 hunks)apps/desktop/src/locales/ko/messages.po(3 hunks)apps/desktop/src/routes/app.tsx(1 hunks)packages/tiptap/package.json(1 hunks)packages/tiptap/src/styles/transcript.css(2 hunks)packages/tiptap/src/transcript/extensions.ts(2 hunks)packages/tiptap/src/transcript/index.tsx(3 hunks)packages/tiptap/src/transcript/nodes.ts(1 hunks)packages/tiptap/src/transcript/utils.test.ts(1 hunks)packages/tiptap/src/transcript/utils.ts(6 hunks)packages/tiptap/src/transcript/views.tsx(1 hunks)packages/ui/src/components/ui/popover.tsx(2 hunks)
💤 Files with no reviewable changes (3)
- apps/desktop/package.json
- apps/desktop/src/contexts/index.ts
- apps/desktop/src/contexts/tinybase.tsx
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/tiptap/src/transcript/utils.test.ts (1)
packages/tiptap/src/transcript/utils.ts (2)
fromWordsToEditor(35-76)fromEditorToWords(78-126)
packages/ui/src/components/ui/popover.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
packages/tiptap/src/transcript/views.tsx (2)
packages/tiptap/src/transcript/index.tsx (1)
SpeakerViewInnerProps(17-17)packages/tiptap/src/editor/index.tsx (1)
TiptapEditor(10-10)
apps/desktop/src/components/right-panel/views/exp.tsx (5)
packages/tiptap/src/transcript/index.tsx (1)
SpeakerViewInnerProps(17-17)packages/tiptap/src/transcript/views.tsx (1)
SpeakerViewInnerProps(31-37)plugins/db/js/bindings.gen.ts (1)
Human(152-152)packages/ui/src/components/ui/popover.tsx (3)
Popover(85-85)PopoverTrigger(85-85)PopoverContent(85-85)apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (1)
ParticipantsChipInner(91-126)
packages/tiptap/src/transcript/index.tsx (3)
packages/tiptap/src/transcript/utils.ts (3)
Word(4-4)fromEditorToWords(78-126)fromWordsToEditor(35-76)packages/tiptap/src/transcript/views.tsx (1)
SpeakerViewInnerComponent(39-39)packages/tiptap/src/transcript/nodes.ts (1)
SpeakerNode(172-220)
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (3)
plugins/db/js/bindings.gen.ts (1)
Human(152-152)apps/desktop/src/contexts/hypr.tsx (1)
useHypr(48-54)packages/ui/src/components/ui/popover.tsx (2)
PopoverContent(85-85)Popover(85-85)
🪛 GitHub Actions: .github/workflows/desktop_ci.yaml
packages/tiptap/src/transcript/extensions.ts
[error] 4-4: TypeScript error TS2307: Cannot find module 'prosemirror-model' or its corresponding type declarations.
🔇 Additional comments (28)
apps/desktop/src/routes/app.tsx (1)
44-88: TinyBaseProvider successfully removedThe TinyBase context provider has been removed from the component tree, aligning with the broader effort to refactor state management by removing TinyBase integration. The component hierarchy remains properly nested with no regressions in the provider structure.
packages/tiptap/package.json (3)
20-20: Added workspace dependency for utils packageThe addition of
@hypr/utilsas a workspace dependency provides access to shared utilities needed for the refactored transcript editing flow.
24-24: Added React Router dependencyAdding
@tanstack/react-routersupports improved navigation capabilities in the editor components. This is a modern, type-safe router implementation appropriate for the project.
26-26: Added Tiptap bubble menu extensionThe
@tiptap/extension-bubble-menudependency enables the creation of context-sensitive floating menus in the editor, which is essential for the modular speaker node/view system mentioned in the PR summary.apps/desktop/src/locales/ko/messages.po (3)
314-316: Updated source reference for localization entryThe line number reference for "Add participant" has been updated from line 341 to 311 to reflect changes in the participants-chip.tsx component structure.
471-473: Updated source reference for localization entryThe line number reference for "Create" has been updated from line 429 to 399 to reflect changes in the participants-chip.tsx component structure.
872-875: Removed obsolete localization entryThe "Remove {0} from list" string has been marked as obsolete, indicating this tooltip is no longer used in the refactored participants component. This aligns with the UI changes mentioned in the PR summary.
apps/desktop/src/locales/en/messages.po (3)
314-316: Updated source reference for localization entryThe line number reference for "Add participant" has been updated from line 341 to 311 to reflect the refactored component structure in participants-chip.tsx.
471-473: Updated source reference for localization entryThe line number reference for "Create" has been updated from line 429 to 399 to reflect the refactored component structure in participants-chip.tsx.
872-875: Removed obsolete localization entryThe "Remove {0} from list" localization entry has been properly marked as obsolete (commented out), ensuring consistency with the Korean localization and reflecting the UI changes in the participant management component.
packages/tiptap/src/transcript/utils.test.ts (1)
21-51: Improved test with detailed structure validationThe test has been enhanced to validate the exact structure of the editor content after converting from words. This additional assertion is valuable as it ensures the intermediate representation matches the expected format with all required attributes and nesting.
This change aligns with the broader refactoring of the transcript utility functions and supports the new speaker node model.
packages/tiptap/src/styles/transcript.css (3)
40-40: LGTM: Line height adjustment for transcript wordsSetting a consistent line height for transcript words improves text alignment.
52-52: LGTM: Adjusted hover background colorThe background color change for hover state provides better visual feedback.
56-60: LGTM: Improved non-editable word stylingThese changes appropriately disable interactive styling in non-editable contexts by removing hover effects and setting the cursor to default, which provides better UX by clearly indicating when words aren't interactive.
packages/ui/src/components/ui/popover.tsx (2)
8-18: Enhanced PopoverTrigger with ref forwarding and consistent focus stylesRefactoring PopoverTrigger to use forwardRef properly passes DOM refs to the underlying component and applies consistent outline styles. This change improves component reusability and accessibility.
71-71: LGTM: Simplified focus styling for PopoverContentRemoving the focus-visible ring in favor of consistent outline-none styling aligns with the trigger's styling approach.
apps/desktop/src/components/right-panel/views/exp.tsx (2)
1-49: Well-implemented SpeakerSelector component with clean separation of concernsThis new component effectively handles speaker selection within a note session. It properly:
- Extracts session ID from the URL using React Router
- Fetches participants using React Query with appropriate query key and enabling condition
- Maintains local state for popover visibility
- Provides fallback display names when speaker data is unavailable
- Uses the shared ParticipantsChipInner component for consistent UI
The component aligns well with the refactored transcript editing flow and modular speaker node/view system mentioned in the PR summary.
20-24:Details
✅ Verification successful
Verify proper query invalidation for participants data
Ensure there's appropriate query invalidation logic elsewhere in the codebase when participants are added, removed, or modified, to keep this component's data in sync.
🏁 Script executed:
#!/bin/bash # Check for query invalidation of participants data echo "Searching for participant query invalidation..." rg -A 3 "invalidateQueries.*participants" --glob "*.{ts,tsx}"Length of output: 168
🏁 Script executed:
#!/bin/bash # Broad search for any query invalidations in the codebase echo "Searching for all invalidateQueries usage across .ts/.tsx files..." rg -A2 "invalidateQueries" --glob "*.{ts,tsx}"Length of output: 4927
Participants query invalidation is implemented
The participants query is invalidated in the chip component whenever participants change, so no further invalidation hooks are required at this time.• apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx – calls
queryClient.invalidateQueries({ predicate: (query) =>
(query.queryKey[0] as string).includes("participants") && query.queryKey[1] === sessionId
})packages/tiptap/src/transcript/index.tsx (4)
19-24: Use a more descriptive prop name thanc
cdoes not convey intent. A future reader has to navigate the code to learn that it is the speaker view component. Consider something likespeakerVieworrenderSpeakerto improve self-documentation of the public API.-interface TranscriptEditorProps { - editable?: boolean; - onBlur?: (words: Word[]) => void; - c: SpeakerViewInnerComponent; -} +interface TranscriptEditorProps { + editable?: boolean; + onBlur?: (words: Word[]) => void; + speakerView: SpeakerViewInnerComponent; +}[ suggest_nitpick ]
25-29: Expose the imperative API withuseImperativeHandleDirectly mutating
ref.currentinside an effect bypasses React’s recommended pattern and can lead to stale closures when the object is recreated.useImperativeHandleguarantees the reference is kept in sync without extra effects.-const TranscriptEditor = forwardRef<TranscriptEditorRef, TranscriptEditorProps>( - ({ editable = true, c, onBlur }, ref) => { +const TranscriptEditor = forwardRef<TranscriptEditorRef, TranscriptEditorProps>( + ({ editable = true, speakerView, onBlur }, ref) => { … - const editor = useEditor({ … }); + const editor = useEditor({ … }); + + useImperativeHandle(ref, () => ({ + editor, + getWords: () => (editor ? fromEditorToWords(editor.getJSON() as any) : null), + setWords: (words: Word[]) => { + if (editor) { + editor.commands.setContent(fromWordsToEditor(words)); + } + }, + }), [editor]);[ suggest_essential_refactor ]
68-74: Avoid resetting the entire document when words haven’t changed
setWordscurrently callseditor.commands.setContentunconditionally.
If the caller feeds the same words repeatedly (as happens inTranscriptViewwhenever the query refetches), the cursor position, selection, and undo history are lost each time.Consider short-circuiting when the incoming
wordsserialise to the same JSON as the current document:const setWords = (words: Word[]) => { if (!editor) return; - const content = fromWordsToEditor(words); - editor.commands.setContent(content); + const content = fromWordsToEditor(words); + if (JSON.stringify(content) !== JSON.stringify(editor.getJSON())) { + editor.commands.setContent(content, false); // preserve history flag + } };[ suggest_optional_refactor ]
51-56: Verify that theonBlur/setWordsround-trip cannot create an infinite update loop
onBlur⇒ database update ⇒ react-query refetch ⇒ parentwordsupdate ⇒setWords⇒editor.commands.setContent
If the DB returns the same words array, the optimisation above will mitigate churn; if it returns a different ordering or attribute defaults, the editor will keep losing focus selection.Please confirm this flow or add debouncing / diffing at the persistence layer.
[ request_verification ]
packages/tiptap/src/transcript/views.tsx (2)
7-14: Attribute typing mismatch forspeaker-index
node.attrs?.["speaker-index"]is a string (becausegetAttributereturns one), yet the prop is typed asnumber | undefined. Consumers relying on a numeric index may break.-const speakerIndex = node.attrs?.["speaker-index"] ?? undefined; +const rawIndex = node.attrs?.["speaker-index"]; +const speakerIndex = rawIndex !== undefined ? Number(rawIndex) : undefined;Consider performing the same coercion when reading
"speaker-id"if you expect numeric IDs later.[ raise_critical_issue ]
31-38: Provide a way to update the speaker label alongside the ID
onSpeakerIdChangemutates only"speaker-id". If the label is stored on the node (for tooltips / UX), it will remain stale after the user selects a different participant.Add an optional
speakerLabelparameter or derive the label in the parent callback and update both attributes:updateAttributes({ "speaker-id": newId, "speaker-label": newLabel });[ suggest_optional_refactor ]
apps/desktop/src/components/right-panel/views/transcript-view.tsx (3)
30-37: Guard againsteditorRef.currentbeing null on first render
wordsis often populated by a query before the editor ref is ready.
The existingif (editorRef.current) { … }guard covers this, but when the ref eventually becomes non-null, the latestwordsvalue may already be stale (missed by the dependency array).Add
editorRef.currentto the dependency list or, better, move the initial content load to the editor’sonCreatecallback exposed viaTranscriptEditor.[ request_verification ]
61-67: Handle DB failures when persisting on blur
handleBlursilently ignores rejection fromupsertSession.
A network or filesystem error will therefore drop user edits without feedback.- dbCommands.getSession({ id: sessionId! }).then((session) => { - if (session) { - dbCommands.upsertSession({ ...session, words }); - } - }); + dbCommands.getSession({ id: sessionId! }) + .then((session) => session && dbCommands.upsertSession({ ...session, words })) + .catch(console.error); // or report to the user / telemetry[ suggest_optional_refactor ]
125-136: Potential UX issue: editor content resets on everywordschangeBecause
wordsis updated frequently (e.g. live transcription),setWordsin the previoususeEffectwill overwrite any transient user selection or undo stack on every tick.After applying the
setWordsshort-circuit optimisation inTranscriptEditorthis will improve, but you may still want to pause updates while the user is actively editing (e.g. track focus).[ offer_architecture_advice ]
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (1)
145-147: Query-key shape assumption may break future refactors.
predicate: (query) => (query.queryKey[0] as string).includes("participants") && query.queryKey[1] === sessionId
relies on:
queryKey[0]being astring.- The string containing
"participants".If
queryKeybecomes an object/tuple this filter starts to silently fail.
Safer: use an explicit tag in the key (['participants', { sessionId }]) or partial matcher (Array.isArray(k) && k[0] === 'participants').
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/tiptap/src/transcript/nodes.ts (2)
53-60:speaker-indexcomparison is type-unsafe and should be updatedThe comparison between
node.attrs["speaker-index"]andspeakerIndexis type-unsafe. While line 183 correctly parsesspeaker-indexas a number, here you're doing a direct comparison without conversion, which can fail if they're of different types.Also, setting
"speaker-index": nullwill still render the attribute in the DOM (line 185), potentially as the string "null".Apply this fix to address both issues:
-if (node.type.name === "speaker" && node.attrs["speaker-index"] === speakerIndex) { +const indexAttr = node.attrs["speaker-index"]; +if ( + node.type.name === "speaker" && + indexAttr != null && + Number(indexAttr) === speakerIndex +) { tr.setNodeMarkup(pos, undefined, { ...node.attrs, - "speaker-index": null, + "speaker-index": undefined, "speaker-id": speakerId, "speaker-label": speakerLabel, });
179-186:speaker-indexattribute rendering should be consistent with other attributesThe
renderHTMLfunction forspeaker-indexdoesn't handle the null case properly, which will cause "null" to appear in the DOM. Other attributes likestart_msinWordNode(line 242) properly omit the attribute when it's null/undefined.Apply this fix:
"speaker-index": { default: null, parseHTML: element => { const v = element.getAttribute("data-speaker-index"); return v !== null ? Number(v) : null; }, - renderHTML: attributes => ({ "data-speaker-index": attributes["speaker-index"] }), + renderHTML: attributes => + attributes["speaker-index"] != null + ? { "data-speaker-index": attributes["speaker-index"] } + : {}, },
🧹 Nitpick comments (2)
packages/tiptap/src/transcript/nodes.ts (2)
187-196: Apply consistent attribute handling for all speaker attributesFor consistency with
speaker-indexandWordNodeattributes, thespeaker-idandspeaker-labelattributes should also conditionally render and properly handle null values."speaker-id": { default: null, parseHTML: element => element.getAttribute("data-speaker-id"), - renderHTML: attributes => ({ "data-speaker-id": attributes["speaker-id"] }), + renderHTML: attributes => + attributes["speaker-id"] != null + ? { "data-speaker-id": attributes["speaker-id"] } + : {}, }, "speaker-label": { default: null, parseHTML: element => element.getAttribute("data-speaker-label"), - renderHTML: attributes => ({ "data-speaker-label": attributes["speaker-label"] }), + renderHTML: attributes => + attributes["speaker-label"] != null + ? { "data-speaker-label": attributes["speaker-label"] } + : {}, },
223-223: Avoid usinganytype castingUsing
as anybypasses TypeScript's type checking, which can lead to potential runtime errors. Consider using a more specific type or implementing the interface correctly.-return implementCommands as any; // casting because util object is compatible +// Define a type that represents the structure of the commands +type SpeakerCommands = { + [K in keyof Commands<any>['speaker']]: Commands<any>['speaker'][K] +}; +return implementCommands as SpeakerCommands;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/tiptap/src/transcript/nodes.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (3)
packages/tiptap/src/transcript/nodes.ts (3)
172-226: Well-designed factory pattern for SpeakerNodeThe factory pattern implemented for
SpeakerNodeis a great approach that allows for component injection and flexibility. This makes the node more reusable and testable while separating concerns between data and presentation.
8-44: Good use of TypeScript module augmentationThe module augmentation for
@tiptap/coreis a clean way to extend the Commands interface with speaker-specific commands. This ensures proper type checking and autocompletion for consumers of your API.
46-170: Command implementations provide a comprehensive APIThe command implementations provide a rich API for manipulating speaker nodes in different contexts. The structure with explicit checks for
dispatchand returning appropriate boolean values follows Tiptap's command pattern correctly.A minor suggestion to improve code consistency:
- Consider extracting the common logic for checking and updating nodes into helper functions
- Standardize the return flow pattern across all commands
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/desktop/src/components/right-panel/views/transcript-view.tsx (3)
138-138: Use a more descriptive prop name.The prop name
cis not descriptive of what it represents. Consider using a more meaningful name likespeakerComponentorspeakerSelectorto improve code readability.- c={SpeakerSelector} + speakerComponent={SpeakerSelector}
197-242: Well-implemented speaker selection component with minor accessibility improvement.The SpeakerSelector component provides a clean interface for changing the speaker associated with transcript segments. It correctly handles different states and integrates well with the ParticipantsChipInner component.
Add an aria-label to improve accessibility:
- <PopoverTrigger> + <PopoverTrigger aria-label={`Change speaker: ${displayName}`}> <span className="underline py-1 font-semibold">{displayName}</span> </PopoverTrigger>
220-220: Consider externalizing the speaker naming convention.The fallback naming convention for speakers (
Speaker ${speakerIndex}) is hardcoded. Consider extracting this to a constant or utility function for consistent naming across the application.- const displayName = foundSpeaker?.full_name ?? `Speaker ${speakerIndex}`; + const displayName = foundSpeaker?.full_name ?? getDefaultSpeakerName(speakerIndex);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/components/right-panel/views/transcript-view.tsx(7 hunks)crates/db-user/src/init.rs(1 hunks)packages/tiptap/src/transcript/index.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/db-user/src/init.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/tiptap/src/transcript/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (3)
apps/desktop/src/components/right-panel/views/transcript-view.tsx (3)
1-16: Well-organized imports with proper type usage.The import statements are well-organized with clear separation between external and internal dependencies. The explicit import of types like
TranscriptEditorRefis good practice for maintaining type safety.
35-39: Good implementation of editor synchronization.The useEffect hook correctly synchronizes the words state with the editor when the words prop changes. This ensures the editor always reflects the latest data without causing unnecessary re-renders.
129-141: Clean implementation of the transcript editor integration.The code cleanly integrates the TranscriptEditor component and handles the empty state appropriately. The editor is only editable when the session is inactive, which is a good constraint to prevent editing during recording.
1 participant
Summary by CodeRabbit
New Features
Refactor
Style
Bug Fixes
Chores
Tests
Documentation