-
Notifications
You must be signed in to change notification settings - Fork 0
fix: copy transcript #674
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
base: main
Are you sure you want to change the base?
fix: copy transcript #674
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
| navigator.clipboard | ||
| .writeText(text) | ||
| .then(() => { | ||
| toaster | ||
| .create({ | ||
| placement: "top", | ||
| duration: 2500, | ||
| render: () => ( | ||
| <div className="chakra-ui-light"> | ||
| <div | ||
| style={{ | ||
| background: "#38A169", | ||
| color: "white", | ||
| padding: "8px 12px", | ||
| borderRadius: 6, | ||
| display: "flex", | ||
| alignItems: "center", | ||
| gap: 8, | ||
| boxShadow: "rgba(0,0,0,0.25) 0px 4px 12px", | ||
| }} | ||
| > | ||
| <LuCheck /> Transcript copied | ||
| </div> | ||
| </div> | ||
| ), | ||
| }) | ||
| .then(() => {}); | ||
| }) | ||
| .catch(() => {}); |
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.
Suggestion: Add proper error handling in the clipboard operation's catch block to inform the user when copying fails, rather than silently ignoring errors. [general, importance: 7]
| navigator.clipboard | |
| .writeText(text) | |
| .then(() => { | |
| toaster | |
| .create({ | |
| placement: "top", | |
| duration: 2500, | |
| render: () => ( | |
| <div className="chakra-ui-light"> | |
| <div | |
| style={{ | |
| background: "#38A169", | |
| color: "white", | |
| padding: "8px 12px", | |
| borderRadius: 6, | |
| display: "flex", | |
| alignItems: "center", | |
| gap: 8, | |
| boxShadow: "rgba(0,0,0,0.25) 0px 4px 12px", | |
| }} | |
| > | |
| <LuCheck /> Transcript copied | |
| </div> | |
| </div> | |
| ), | |
| }) | |
| .then(() => {}); | |
| }) | |
| .catch(() => {}); | |
| navigator.clipboard | |
| .writeText(text) | |
| .then(() => { | |
| toaster | |
| .create({ | |
| placement: "top", | |
| duration: 2500, | |
| render: () => ( | |
| <div className="chakra-ui-light"> | |
| <div | |
| style={{ | |
| background: "#38A169", | |
| color: "white", | |
| padding: "8px 12px", | |
| borderRadius: 6, | |
| display: "flex", | |
| alignItems: "center", | |
| gap: 8, | |
| boxShadow: "rgba(0,0,0,0.25) 0px 4px 12px", | |
| }} | |
| > | |
| <LuCheck /> Transcript copied | |
| </div> | |
| </div> | |
| ), | |
| }) | |
| .then(() => {}); | |
| }) | |
| .catch((error) => { | |
| toaster.create({ | |
| placement: "top", | |
| duration: 2500, | |
| render: () => ( | |
| <div className="chakra-ui-light"> | |
| <div | |
| style={{ | |
| background: "#E53E3E", | |
| color: "white", | |
| padding: "8px 12px", | |
| borderRadius: 6, | |
| display: "flex", | |
| alignItems: "center", | |
| gap: 8, | |
| boxShadow: "rgba(0,0,0,0.25) 0px 4px 12px", | |
| }} | |
| > | |
| Failed to copy transcript | |
| </div> | |
| </div> | |
| ), | |
| }); | |
| }); |
| const text = buildTranscriptWithTopics( | ||
| props.topicsResponse || [], | ||
| participantsQuery?.data || null, | ||
| props.transcriptResponse?.title || null, | ||
| ); | ||
| if (!text) return; | ||
| navigator.clipboard |
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.
Suggestion: Check if participantsQuery is still loading before using its data. The current implementation might cause issues if the query hasn't completed when the copy function is triggered. [possible issue, importance: 8]
| const text = buildTranscriptWithTopics( | |
| props.topicsResponse || [], | |
| participantsQuery?.data || null, | |
| props.transcriptResponse?.title || null, | |
| ); | |
| if (!text) return; | |
| navigator.clipboard | |
| if (participantsQuery.isLoading) { | |
| toaster.create({ | |
| placement: "top", | |
| duration: 2500, | |
| render: () => ( | |
| <div className="chakra-ui-light"> | |
| <div style={{ | |
| background: "#ED8936", | |
| color: "white", | |
| padding: "8px 12px", | |
| borderRadius: 6, | |
| display: "flex", | |
| alignItems: "center", | |
| gap: 8, | |
| boxShadow: "rgba(0,0,0,0.25) 0px 4px 12px", | |
| }}> | |
| Please wait for participant data to load | |
| </div> | |
| </div> | |
| ), | |
| }); | |
| return; | |
| } | |
| const text = buildTranscriptWithTopics( | |
| props.topicsResponse || [], | |
| participantsQuery?.data || null, | |
| props.transcriptResponse?.title || null, | |
| ); | |
| if (!text) return; |
| function getSpeakerName( | ||
| speakerNumber: number, | ||
| participants?: Participant[] | null, | ||
| ): string { | ||
| const name = participants?.find((p) => p.speaker === speakerNumber)?.name; | ||
| return name && name.trim().length > 0 ? name : `Speaker ${speakerNumber}`; | ||
| } |
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.
Suggestion: Add a type check for speakerNumber before using it to find a participant. If speakerNumber is undefined or null, the function might return incorrect results. [general, importance: 6]
| function getSpeakerName( | |
| speakerNumber: number, | |
| participants?: Participant[] | null, | |
| ): string { | |
| const name = participants?.find((p) => p.speaker === speakerNumber)?.name; | |
| return name && name.trim().length > 0 ? name : `Speaker ${speakerNumber}`; | |
| } | |
| function getSpeakerName( | |
| speakerNumber: number, | |
| participants?: Participant[] | null, | |
| ): string { | |
| if (speakerNumber === undefined || speakerNumber === null) { | |
| return "Unknown Speaker"; | |
| } | |
| const name = participants?.find((p) => p.speaker === speakerNumber)?.name; | |
| return name && name.trim().length > 0 ? name : `Speaker ${speakerNumber}`; | |
| } |
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.
I think you could have leverage the webvtt field instead of the old topics list, but that works too.
User description
PR Type
Enhancement
Description
Add transcript copy functionality with formatting
Include speaker names and timestamps
Show success notification on copy
Improve transcript formatting with topics
Changes walkthrough 📝
finalSummary.tsx
Add transcript copy button with success notificationwww/app/(app)/transcripts/[transcriptId]/finalSummary.tsx
buildTranscriptWithTopicsbuildTranscriptWithTopics.ts
Create transcript formatting utilitywww/app/(app)/transcripts/buildTranscriptWithTopics.ts
shareCopy.tsx
Update transcript copy with new formatting utilitywww/app/(app)/transcripts/shareCopy.tsx
buildTranscriptWithTopicsutilityuseTranscriptParticipantshook