-
Notifications
You must be signed in to change notification settings - Fork 418
Pre Meeting Note Diff + Export to PDF + New Search/Replace Bar #986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…-0621-merge-main
📝 WalkthroughWalkthroughThis change introduces a diff-based enhancement workflow for meeting notes, refines session store logic to handle pre-meeting notes, and restructures the transcript search UI by moving search-and-replace functionality into a new, reusable Changes
Sequence Diagram(s)Diff-based Enhancement FlowsequenceDiagram
participant User
participant EditorArea
participant SessionStore
participant useEnhanceMutation
participant LanguageModel
User->>EditorArea: Initiate enhancement
EditorArea->>SessionStore: Get preMeetingNote and rawContent
EditorArea->>useEnhanceMutation: Call with preMeetingNote, rawContent
useEnhanceMutation->>useEnhanceMutation: Extract text from HTML
useEnhanceMutation->>useEnhanceMutation: Compute word-level diff
useEnhanceMutation->>LanguageModel: Send only new words (finalInput) for enhancement
LanguageModel-->>useEnhanceMutation: Return enhanced note
useEnhanceMutation-->>EditorArea: Update UI with enhanced note
Transcript Search UI RefactorsequenceDiagram
participant User
participant TranscriptView
participant SearchHeader
participant EditorRef
User->>TranscriptView: Press Ctrl+F or click search icon
TranscriptView->>SearchHeader: Render SearchHeader component
User->>SearchHeader: Enter search/replace terms, navigate results
SearchHeader->>EditorRef: Update search/replace state and results
User->>SearchHeader: Replace all or close search
SearchHeader->>TranscriptView: Notify to close search UI
PDF Export FlowsequenceDiagram
participant User
participant ShareButton
participant pdf-export.ts
participant TauriFS
User->>ShareButton: Click "Export as PDF"
ShareButton->>pdf-export.ts: Call exportToPDF(session)
pdf-export.ts->>TauriFS: Save generated PDF to Downloads
TauriFS-->>pdf-export.ts: Return filename
pdf-export.ts-->>ShareButton: Notify export success/failure
ShareButton-->>User: Show alert or confirmation
Possibly related PRs
✨ Finishing Touches
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. 🪧 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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
apps/desktop/package.json (1)
1-1: Critical: Update pnpm-lock.yaml to fix pipeline failures.The CI pipeline is failing because the pnpm-lock.yaml file is outdated and doesn't match the package.json dependencies. This prevents the build from completing successfully.
Run the following command to update the lockfile:
pnpm installThen commit the updated
pnpm-lock.yamlfile to resolve the pipeline failures.
🧹 Nitpick comments (6)
apps/desktop/.cargo/config.toml (1)
1-3: Enable Apple M1 optimizations
Applying-C target-cpu=apple-m1targets native M1 performance. Consider adding-C opt-level=3in release profiles to maximize optimization.packages/utils/src/stores/ongoing-session.ts (1)
71-71: Use strict equality operator for type safety.Replace
!=with!==to ensure type-safe comparison.Apply this diff:
- if (currentSession.raw_memo_html && currentSession.raw_memo_html != "<p></p>") { + if (currentSession.raw_memo_html && currentSession.raw_memo_html !== "<p></p>") {apps/desktop/src/components/editor-area/index.tsx (3)
198-198: Replacevarwithletfor block-scoped variable.Use
letinstead ofvarto follow modern JavaScript best practices.Apply this diff:
- var finalInput = ""; + let finalInput = "";
202-202: Use strict equality operator or logical NOT operator.Replace
== falsewith=== falseor simply use!diff.removedfor cleaner code.Apply this diff:
- if (diff.added && diff.removed == false) { + if (diff.added && !diff.removed) {
255-257: Remove commented debug statements.Remove unused console.log statements to keep the code clean.
Apply this diff:
- // console.log("systemMessage", systemMessage); - // console.log("userMessage", userMessage); -apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)
75-82: Remove redundant commented block statement.This commented block serves no purpose and should be removed entirely.
Apply this diff to remove the redundant block:
- { - /* - const handleShareToSlack = () => { - setOpen(false); // Close the popover - setSlackModalOpen(true); // Open the modal - }; - */ - }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
apps/desktop/.cargo/config.toml(1 hunks)apps/desktop/package.json(3 hunks)apps/desktop/src-tauri/capabilities/default.json(1 hunks)apps/desktop/src/components/editor-area/index.tsx(4 hunks)apps/desktop/src/components/right-panel/components/search-header.tsx(1 hunks)apps/desktop/src/components/right-panel/views/transcript-view.tsx(6 hunks)apps/desktop/src/components/toolbar/bars/main-toolbar.tsx(2 hunks)apps/desktop/src/components/toolbar/buttons/share-button.tsx(1 hunks)apps/desktop/src/components/toolbar/utils/pdf-export.ts(1 hunks)crates/template/assets/enhance.system.jinja(1 hunks)crates/whisper/Cargo.toml(1 hunks)packages/utils/src/stores/ongoing-session.ts(2 hunks)packages/utils/src/stores/session.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,ts,tsx,rs}`: 1. No error handling. 2. No unused imports, variables, or functions. 3. For comments, keep it minimal. It should be about "Why", not "What".
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
apps/desktop/src/components/toolbar/bars/main-toolbar.tsxpackages/utils/src/stores/session.tspackages/utils/src/stores/ongoing-session.tsapps/desktop/src/components/editor-area/index.tsxapps/desktop/src/components/right-panel/components/search-header.tsxapps/desktop/src/components/toolbar/utils/pdf-export.tsapps/desktop/src/components/right-panel/views/transcript-view.tsxapps/desktop/src/components/toolbar/buttons/share-button.tsx
🪛 GitHub Actions: .github/workflows/app_ci.yaml
apps/desktop/package.json
[error] 1-1: pnpm install failed due to outdated pnpm-lock.yaml. The lockfile is not up to date with package.json dependencies. Use 'pnpm install --no-frozen-lockfile' to bypass this in CI.
🪛 GitHub Actions: .github/workflows/desktop_ci.yaml
apps/desktop/package.json
[error] 1-1: pnpm install failed due to outdated pnpm-lock.yaml. The lockfile is not up to date with package.json. Use 'pnpm install --no-frozen-lockfile' to bypass this in CI.
🪛 Biome (1.9.4)
apps/desktop/src/components/toolbar/buttons/share-button.tsx
[error] 75-82: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (15)
crates/whisper/Cargo.toml (2)
41-43: Removemetalfeature for Apple Silicon
Themetalfeature is dropped for macOS aarch64 builds. Confirm no functionality depends on Metal and that performance remains acceptable.
38-40: Consistent revision bump forwhisper-rs
The gitrevhas been updated toe3d67d5for non-mac and mac x86_64 targets. Verify that this commit exists upstream and includes the intended fixes.#!/bin/bash # Verify that the commit hash exists in the upstream repository git ls-remote https://github.com/tazz4843/whisper-rs e3d67d5Also applies to: 44-46
apps/desktop/.cargo/config.toml (1)
4-6: Default build target locked to Apple Silicon
Specifyingtarget = "aarch64-apple-darwin"may break builds on Intel or CI systems. Ensure multi-architecture support is addressed or document this requirement clearly.apps/desktop/src-tauri/capabilities/default.json (2)
60-66: LGTM! Appropriately scoped file system permissions for PDF export.The new write permissions are correctly limited to the Downloads directory, which is appropriate for PDF export functionality.
75-75: LGTM! Dialog permission enables save functionality.The dialog:allow-save permission is necessary for the PDF export save dialog workflow.
crates/template/assets/enhance.system.jinja (1)
15-15: LGTM! Clarifies input format and improves documentation.The change from "markdown" to "txt" better describes the actual format, and the additional clarification about raw notes being what users write during meetings improves the template's accuracy.
Also applies to: 20-20
apps/desktop/src/components/toolbar/bars/main-toolbar.tsx (1)
7-7: LGTM! Proper integration of ShareButton component.The ShareButton is correctly imported and conditionally rendered alongside other note-related buttons, following the established pattern in the toolbar.
Also applies to: 54-54
packages/utils/src/stores/session.ts (1)
17-17: LGTM! Consistent implementation following established patterns.The
updatePreMeetingNotemethod correctly follows the same pattern as other update methods, using mutative for immutable updates and properly triggering persistence.Also applies to: 53-61
apps/desktop/package.json (1)
51-51: LGTM! Appropriate dependencies for PDF export functionality.The added dependencies (
@react-pdf/renderer,diff,html-pdf,html2canvas,html2pdf.js,jspdf) and their corresponding TypeScript types are appropriate for the PDF export and diffing features.Also applies to: 73-77, 105-107
packages/utils/src/stores/ongoing-session.ts (1)
71-74: Add error handling for updatePreMeetingNote.The
updatePreMeetingNotecall should include error handling to prevent silent failures.Consider wrapping the call in a try-catch block:
if (currentSession.raw_memo_html && currentSession.raw_memo_html !== "<p></p>") { const preMeetingNote = currentSession.raw_memo_html; - sessionStore.getState().updatePreMeetingNote(preMeetingNote); + try { + sessionStore.getState().updatePreMeetingNote(preMeetingNote); + } catch (error) { + console.error("Failed to update pre-meeting note:", error); + } }Likely an incorrect or invalid review comment.
apps/desktop/src/components/editor-area/index.tsx (1)
194-207: Add error handling for text extraction and diffing operations.The text extraction and diffing operations could fail and should be wrapped in error handling.
Consider adding error handling:
- const preMeetingText = extractTextFromHtml(preMeetingNote); - const rawText = extractTextFromHtml(rawContent); - - // finalInput is the text that will be used to enhance the note - let finalInput = ""; - const wordDiff = diffWords(preMeetingText, rawText); - if (wordDiff && wordDiff.length > 0) { - for (const diff of wordDiff) { - if (diff.added && !diff.removed) { - finalInput += " " + diff.value; - } - } - } + let finalInput = ""; + try { + const preMeetingText = extractTextFromHtml(preMeetingNote); + const rawText = extractTextFromHtml(rawContent); + + // finalInput is the text that will be used to enhance the note + const wordDiff = diffWords(preMeetingText, rawText); + if (wordDiff && wordDiff.length > 0) { + for (const diff of wordDiff) { + if (diff.added && !diff.removed) { + finalInput += " " + diff.value; + } + } + } + } catch (error) { + console.error("Failed to process text diff:", error); + // Fallback to using the entire raw content + finalInput = extractTextFromHtml(rawContent); + }Likely an incorrect or invalid review comment.
apps/desktop/src/components/toolbar/utils/pdf-export.ts (1)
1-355: Well-implemented PDF export functionality!The PDF export implementation is comprehensive with:
- Proper error handling throughout
- Clean HTML to text conversion with formatting preservation
- Good PDF layout with pagination support
- Proper file system integration
Great job on the implementation!
apps/desktop/src/components/right-panel/components/search-header.tsx (2)
93-106: Consider using editor API instead of direct DOM queries.Direct DOM manipulation with
querySelectorcan be fragile if class names change. Check if the editor provides an API method for scrolling to search results.If the editor doesn't provide such an API, at least extract the class name to a constant:
+const SEARCH_RESULT_CURRENT_CLASS = "search-result-current"; function scrollCurrentResultIntoView(editorRef: React.RefObject<any>) { if (!editorRef.current) { return; } const editorElement = editorRef.current.editor.view.dom; - const current = editorElement.querySelector(".search-result-current") as HTMLElement | null; + const current = editorElement.querySelector(`.${SEARCH_RESULT_CURRENT_CLASS}`) as HTMLElement | null; if (current) { current.scrollIntoView({ behavior: "smooth", block: "center", inline: "nearest", }); } }
21-36: Fix incorrect dependency array in useDebouncedCallback.The second parameter should be the callback's dependencies directly, not wrapped in an array.
const debouncedSetSearchTerm = useDebouncedCallback( (value: string) => { if (editorRef.current) { editorRef.current.editor.commands.setSearchTerm(value); editorRef.current.editor.commands.resetIndex(); setTimeout(() => { const storage = editorRef.current.editor.storage.searchAndReplace; const results = storage.results || []; setResultCount(results.length); setCurrentIndex((storage.resultIndex ?? 0) + 1); }, 100); } }, - [editorRef], + editorRef, 300, );Likely an incorrect or invalid review comment.
apps/desktop/src/components/right-panel/views/transcript-view.tsx (1)
205-225: Good refactor of width management.Moving the width measurement to the parent component and passing it as a prop improves separation of concerns and component reusability.
| ); | ||
| }, | ||
| start: (sessionId: string) => { | ||
| console.log("start", sessionId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug console.log statement.
Debug logging should not be committed to production code.
Apply this diff to remove the debug statement:
- console.log("start", sessionId);📝 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.
| console.log("start", sessionId); |
🤖 Prompt for AI Agents
In packages/utils/src/stores/ongoing-session.ts at line 58, remove the debug
console.log statement that prints "start" and the sessionId. This debug logging
should not be present in production code, so simply delete the entire
console.log line.
| // Simulate API call | ||
| await new Promise(resolve => setTimeout(resolve, 1500)); | ||
|
|
||
| alert("Successfully shared to Slack!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace alert() with toast notifications for better UX.
Using alert() blocks the UI and provides a poor user experience. Consider using toast notifications instead.
Replace alert calls with toast notifications:
- alert("Successfully shared to Slack!");
+ toast({
+ title: "Success",
+ content: "Successfully shared to Slack!",
+ duration: 5000,
+ });- alert(`PDF saved to Downloads folder: ${filename}`);
+ toast({
+ title: "PDF Exported",
+ content: `PDF saved to Downloads folder: ${filename}`,
+ duration: 5000,
+ });- alert("Failed to export PDF. Please try again.");
+ toast({
+ title: "Export Failed",
+ content: "Failed to export PDF. Please try again.",
+ variant: "destructive",
+ duration: 5000,
+ });Don't forget to import the toast function at the top of the file:
+import { toast } from "@hypr/ui/components/ui/toast";Also applies to: 132-132, 135-135
🤖 Prompt for AI Agents
In apps/desktop/src/components/toolbar/buttons/share-button.tsx at lines 112,
132, and 135, replace the use of alert() with toast notifications to improve
user experience by avoiding UI blocking. Import the toast function from the
appropriate library at the top of the file, then replace each
alert("Successfully shared to Slack!") call with a toast notification call that
displays the same success message non-blockingly.
| useEffect(() => { | ||
| debouncedSetSearchTerm(searchTerm); | ||
| }, [searchTerm]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing dependency to useEffect.
Include debouncedSetSearchTerm in the dependency array to avoid potential stale closure issues.
useEffect(() => {
debouncedSetSearchTerm(searchTerm);
- }, [searchTerm]);
+ }, [searchTerm, debouncedSetSearchTerm]);📝 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.
| useEffect(() => { | |
| debouncedSetSearchTerm(searchTerm); | |
| }, [searchTerm]); | |
| useEffect(() => { | |
| debouncedSetSearchTerm(searchTerm); | |
| }, [searchTerm, debouncedSetSearchTerm]); |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/components/search-header.tsx around
lines 38 to 40, the useEffect hook is missing the debouncedSetSearchTerm
function in its dependency array. Add debouncedSetSearchTerm to the dependency
array to ensure the effect updates correctly when this function changes and to
prevent stale closure issues.
| // Click outside handler | ||
| useEffect(() => { | ||
| const handleClickOutside = (event: MouseEvent) => { | ||
| if (searchHeaderRef.current && !searchHeaderRef.current.contains(event.target as Node)) { | ||
| handleClose(); | ||
| } | ||
| }; | ||
|
|
||
| document.addEventListener("mousedown", handleClickOutside); | ||
| return () => document.removeEventListener("mousedown", handleClickOutside); | ||
| }, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing dependencies to click outside handler.
The effect uses handleClose which is defined later and should be included in dependencies. Consider using useCallback to memoize handleClose.
Add onClose to the dependency array:
document.addEventListener("mousedown", handleClickOutside);
return () => document.removeEventListener("mousedown", handleClickOutside);
- }, []);
+ }, [onClose]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/components/search-header.tsx around
lines 48 to 58, the useEffect hook for the click outside handler is missing
dependencies, specifically the handleClose function. To fix this, memoize
handleClose using useCallback and include it in the dependency array of the
useEffect hook to ensure the effect updates correctly when handleClose changes.
| import { ChevronDownIcon, ChevronUpIcon, ReplaceIcon, XIcon } from "lucide-react"; | ||
| import { useEffect, useRef, useState } from "react"; | ||
| interface SearchHeaderProps { | ||
| editorRef: React.RefObject<any>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type the editorRef prop properly instead of using any.
Using any defeats TypeScript's type safety. Define or import the proper type for the editor reference.
- editorRef: React.RefObject<any>;
+ editorRef: React.RefObject<TranscriptEditorRef>;📝 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.
| editorRef: React.RefObject<any>; | |
| editorRef: React.RefObject<TranscriptEditorRef>; |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/components/search-header.tsx at line
7, the editorRef prop is currently typed as React.RefObject<any>, which bypasses
TypeScript's type safety. Replace the any type with the specific type of the
editor instance that editorRef is expected to reference, either by importing the
editor's type or defining it explicitly, to ensure proper type checking and
safety.
| const handleNext = () => { | ||
| if (editorRef.current?.editor) { | ||
| editorRef.current.editor.commands.nextSearchResult(); | ||
| setTimeout(() => { | ||
| const storage = editorRef.current.editor.storage.searchAndReplace; | ||
| setCurrentIndex((storage.resultIndex ?? 0) + 1); | ||
| scrollCurrentResultIntoView(editorRef); | ||
| }, 100); | ||
| } | ||
| }; | ||
|
|
||
| const handlePrevious = () => { | ||
| if (editorRef.current?.editor) { | ||
| editorRef.current.editor.commands.previousSearchResult(); | ||
| setTimeout(() => { | ||
| const storage = editorRef.current.editor.storage.searchAndReplace; | ||
| setCurrentIndex((storage.resultIndex ?? 0) + 1); | ||
| scrollCurrentResultIntoView(editorRef); | ||
| }, 100); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid using setTimeout for state synchronization.
Multiple setTimeout calls with hardcoded delays can lead to race conditions. Consider using promises or callbacks from the editor API if available, or at least extract the delay to a named constant.
+const EDITOR_UPDATE_DELAY = 100; // ms
const handleNext = () => {
if (editorRef.current?.editor) {
editorRef.current.editor.commands.nextSearchResult();
setTimeout(() => {
const storage = editorRef.current.editor.storage.searchAndReplace;
setCurrentIndex((storage.resultIndex ?? 0) + 1);
scrollCurrentResultIntoView(editorRef);
- }, 100);
+ }, EDITOR_UPDATE_DELAY);
}
};
const handlePrevious = () => {
if (editorRef.current?.editor) {
editorRef.current.editor.commands.previousSearchResult();
setTimeout(() => {
const storage = editorRef.current.editor.storage.searchAndReplace;
setCurrentIndex((storage.resultIndex ?? 0) + 1);
scrollCurrentResultIntoView(editorRef);
- }, 100);
+ }, EDITOR_UPDATE_DELAY);
}
};📝 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.
| const handleNext = () => { | |
| if (editorRef.current?.editor) { | |
| editorRef.current.editor.commands.nextSearchResult(); | |
| setTimeout(() => { | |
| const storage = editorRef.current.editor.storage.searchAndReplace; | |
| setCurrentIndex((storage.resultIndex ?? 0) + 1); | |
| scrollCurrentResultIntoView(editorRef); | |
| }, 100); | |
| } | |
| }; | |
| const handlePrevious = () => { | |
| if (editorRef.current?.editor) { | |
| editorRef.current.editor.commands.previousSearchResult(); | |
| setTimeout(() => { | |
| const storage = editorRef.current.editor.storage.searchAndReplace; | |
| setCurrentIndex((storage.resultIndex ?? 0) + 1); | |
| scrollCurrentResultIntoView(editorRef); | |
| }, 100); | |
| } | |
| }; | |
| // extract magic number for consistency and easy adjustment | |
| const EDITOR_UPDATE_DELAY = 100; // ms | |
| const handleNext = () => { | |
| if (editorRef.current?.editor) { | |
| editorRef.current.editor.commands.nextSearchResult(); | |
| setTimeout(() => { | |
| const storage = editorRef.current.editor.storage.searchAndReplace; | |
| setCurrentIndex((storage.resultIndex ?? 0) + 1); | |
| scrollCurrentResultIntoView(editorRef); | |
| }, EDITOR_UPDATE_DELAY); | |
| } | |
| }; | |
| const handlePrevious = () => { | |
| if (editorRef.current?.editor) { | |
| editorRef.current.editor.commands.previousSearchResult(); | |
| setTimeout(() => { | |
| const storage = editorRef.current.editor.storage.searchAndReplace; | |
| setCurrentIndex((storage.resultIndex ?? 0) + 1); | |
| scrollCurrentResultIntoView(editorRef); | |
| }, EDITOR_UPDATE_DELAY); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/components/search-header.tsx around
lines 71 to 91, avoid using setTimeout with hardcoded delays for state
synchronization as it can cause race conditions. Instead, refactor handleNext
and handlePrevious to use promises or callbacks provided by the editor API to
update the current index and scroll after the search result changes. If the
editor API does not support this, at minimum extract the delay duration into a
named constant for clarity and easier adjustment.
| // Add Ctrl+F keyboard shortcut | ||
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.ctrlKey || e.metaKey) && e.key === "f") { | ||
| const currentShowActions = hasTranscript && sessionId && ongoingSession.isInactive; | ||
| if (currentShowActions) { | ||
| setIsSearchActive(true); | ||
| } | ||
| } | ||
| }; | ||
| document.addEventListener("keydown", handleKeyDown); | ||
| return () => document.removeEventListener("keydown", handleKeyDown); | ||
| }, [hasTranscript, sessionId, ongoingSession.isInactive]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add preventDefault to avoid browser's default find behavior.
The keyboard shortcut should prevent the browser's default Ctrl/Cmd+F behavior to avoid conflicts.
useEffect(() => {
const handleKeyDown = (e: KeyboardEvent) => {
if ((e.ctrlKey || e.metaKey) && e.key === "f") {
+ e.preventDefault();
const currentShowActions = hasTranscript && sessionId && ongoingSession.isInactive;
if (currentShowActions) {
setIsSearchActive(true);
}
}
};
document.addEventListener("keydown", handleKeyDown);
return () => document.removeEventListener("keydown", handleKeyDown);
}, [hasTranscript, sessionId, ongoingSession.isInactive]);📝 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.
| // Add Ctrl+F keyboard shortcut | |
| useEffect(() => { | |
| const handleKeyDown = (e: KeyboardEvent) => { | |
| if ((e.ctrlKey || e.metaKey) && e.key === "f") { | |
| const currentShowActions = hasTranscript && sessionId && ongoingSession.isInactive; | |
| if (currentShowActions) { | |
| setIsSearchActive(true); | |
| } | |
| } | |
| }; | |
| document.addEventListener("keydown", handleKeyDown); | |
| return () => document.removeEventListener("keydown", handleKeyDown); | |
| }, [hasTranscript, sessionId, ongoingSession.isInactive]); | |
| // Add Ctrl+F keyboard shortcut | |
| useEffect(() => { | |
| const handleKeyDown = (e: KeyboardEvent) => { | |
| if ((e.ctrlKey || e.metaKey) && e.key === "f") { | |
| e.preventDefault(); | |
| const currentShowActions = hasTranscript && sessionId && ongoingSession.isInactive; | |
| if (currentShowActions) { | |
| setIsSearchActive(true); | |
| } | |
| } | |
| }; | |
| document.addEventListener("keydown", handleKeyDown); | |
| return () => document.removeEventListener("keydown", handleKeyDown); | |
| }, [hasTranscript, sessionId, ongoingSession.isInactive]); |
🤖 Prompt for AI Agents
In apps/desktop/src/components/right-panel/views/transcript-view.tsx around
lines 83 to 95, the Ctrl+F keyboard shortcut handler does not call
preventDefault, causing the browser's default find dialog to appear and conflict
with the app's search. Add e.preventDefault() inside the if block where the
shortcut is detected to stop the browser's default find behavior when activating
the custom search.
No description provided.