-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Audio recording feature #5263
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?
Audio recording feature #5263
Changes from 9 commits
abcab8d
9027ece
bbc82f9
61a3142
25f1bcf
063128e
02b36d5
1cd7973
6e6ade0
eb7f779
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,128 @@ | ||||||
| import { PauseIcon, PlayIcon } from "lucide-react"; | ||||||
| import { useEffect, useRef, useState } from "react"; | ||||||
| import { Button } from "@/components/ui/button"; | ||||||
|
|
||||||
| interface Props { | ||||||
| src: string; | ||||||
| className?: string; | ||||||
| } | ||||||
|
|
||||||
| const AudioPlayer = ({ src, className = "" }: Props) => { | ||||||
| const audioRef = useRef<HTMLAudioElement>(null); | ||||||
| const [isPlaying, setIsPlaying] = useState(false); | ||||||
| const [currentTime, setCurrentTime] = useState(0); | ||||||
| const [duration, setDuration] = useState(0); | ||||||
| const [isLoading, setIsLoading] = useState(true); | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| const audio = audioRef.current; | ||||||
| if (!audio) return; | ||||||
|
|
||||||
| const handleLoadedMetadata = () => { | ||||||
| if (audio.duration && !isNaN(audio.duration) && isFinite(audio.duration)) { | ||||||
| setDuration(audio.duration); | ||||||
| } | ||||||
| setIsLoading(false); | ||||||
| }; | ||||||
|
|
||||||
| const handleTimeUpdate = () => { | ||||||
| setCurrentTime(audio.currentTime); | ||||||
| if (audio.duration && !isNaN(audio.duration) && isFinite(audio.duration)) { | ||||||
| setDuration((prev) => (prev === 0 ? audio.duration : prev)); | ||||||
| } | ||||||
| }; | ||||||
|
|
||||||
| const handleEnded = () => { | ||||||
| setIsPlaying(false); | ||||||
| setCurrentTime(0); | ||||||
| }; | ||||||
|
|
||||||
| const handleLoadedData = () => { | ||||||
| // For files without proper duration in metadata, | ||||||
| // try to get it after some data is loaded | ||||||
| if (audio.duration && !isNaN(audio.duration) && isFinite(audio.duration)) { | ||||||
| setDuration(audio.duration); | ||||||
| setIsLoading(false); | ||||||
| } | ||||||
| }; | ||||||
|
|
||||||
| audio.addEventListener("loadedmetadata", handleLoadedMetadata); | ||||||
| audio.addEventListener("loadeddata", handleLoadedData); | ||||||
| audio.addEventListener("timeupdate", handleTimeUpdate); | ||||||
| audio.addEventListener("ended", handleEnded); | ||||||
|
|
||||||
| return () => { | ||||||
| audio.removeEventListener("loadedmetadata", handleLoadedMetadata); | ||||||
| audio.removeEventListener("loadeddata", handleLoadedData); | ||||||
| audio.removeEventListener("timeupdate", handleTimeUpdate); | ||||||
| audio.removeEventListener("ended", handleEnded); | ||||||
| }; | ||||||
| }, []); | ||||||
|
||||||
| }, []); | |
| }, [src]); |
Outdated
Copilot
AI
Nov 19, 2025
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.
[nitpick] The range input has extremely long className with complex styling that makes it hard to read and maintain. Consider extracting this to a separate CSS class or using a more maintainable approach with CSS modules or styled components.
Outdated
Copilot
AI
Nov 19, 2025
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.
The audio player controls lack accessible labels. The play/pause button and seek slider need aria-label attributes for screen reader users. Add labels like "Play audio" / "Pause audio" for the button and "Seek audio position" for the range input.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,9 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { LatLng } from "leaflet"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { uniqBy } from "lodash-es"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { FileIcon, LinkIcon, LoaderIcon, MapPinIcon, Maximize2Icon, MoreHorizontalIcon, PlusIcon } from "lucide-react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { FileIcon, LinkIcon, LoaderIcon, MapPinIcon, Maximize2Icon, MicIcon, MoreHorizontalIcon, PlusIcon, XIcon } from "lucide-react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { observer } from "mobx-react-lite"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useContext, useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { toast } from "react-hot-toast"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Button } from "@/components/ui/button"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DropdownMenu, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -13,12 +14,14 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DropdownMenuSubTrigger, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DropdownMenuTrigger, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@/components/ui/dropdown-menu"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { attachmentStore } from "@/store"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Attachment } from "@/types/proto/api/v1/attachment_service"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Location, MemoRelation } from "@/types/proto/api/v1/memo_service"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useTranslate } from "@/utils/i18n"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { MemoEditorContext } from "../types"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { LinkMemoDialog } from "./InsertMenu/LinkMemoDialog"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { LocationDialog } from "./InsertMenu/LocationDialog"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useAudioRecorder } from "./InsertMenu/useAudioRecorder"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useFileUpload } from "./InsertMenu/useFileUpload"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useLinkMemo } from "./InsertMenu/useLinkMemo"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useLocation } from "./InsertMenu/useLocation"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -52,6 +55,7 @@ const InsertMenu = observer((props: Props) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const location = useLocation(props.location); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const audioRecorder = useAudioRecorder(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Cleanup media stream and timer when component unmounts | |
| useEffect(() => { | |
| return () => { | |
| if (audioRecorder && typeof audioRecorder.cancelRecording === "function") { | |
| audioRecorder.cancelRecording(); | |
| } | |
| }; | |
| }, [audioRecorder]); |
Outdated
Copilot
AI
Nov 19, 2025
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.
The error message fallback uses error.details which is not a standard Error property in JavaScript. Standard Error objects have message property. Consider using error.message || "Failed to upload audio recording" for proper error handling.
| toast.error(error.details || "Failed to upload audio recording"); | |
| toast.error(error.message || "Failed to upload audio recording"); |
Outdated
Copilot
AI
Nov 19, 2025
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.
The pause button icon uses a hardcoded double pipe string "||" instead of a proper Pause icon component. This is inconsistent with the rest of the UI which uses Lucide icons. Consider using PauseIcon which is already imported.
Outdated
Copilot
AI
Nov 19, 2025
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.
[nitpick] The cancel recording button lacks clear visual indication of its destructive action. Consider adding text-red-600 or similar styling to match the stop button's indication that it's a cancel/delete action, improving user understanding of the button's purpose.
| <Button variant="ghost" size="icon" onClick={audioRecorder.cancelRecording} className="shrink-0"> | |
| <Button variant="ghost" size="icon" onClick={audioRecorder.cancelRecording} className="shrink-0 text-red-600 hover:text-red-700"> |
Outdated
Copilot
AI
Nov 19, 2025
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.
The recording control buttons lack accessible labels. Screen reader users won't know what the pause, stop, and cancel buttons do. Add aria-label attributes to each button to describe their purpose (e.g., "Pause recording", "Stop and save recording", "Cancel recording").
| <Button variant="outline" size="icon" onClick={audioRecorder.togglePause} className="shrink-0"> | |
| {audioRecorder.isPaused ? <MicIcon className="w-4 h-4" /> : <span className="font-bold text-xs">||</span>} | |
| </Button> | |
| <Button variant="outline" size="icon" onClick={handleStopRecording} className="shrink-0 text-red-600 hover:text-red-700"> | |
| <div className="w-3 h-3 bg-current rounded-sm" /> | |
| </Button> | |
| <Button variant="ghost" size="icon" onClick={audioRecorder.cancelRecording} className="shrink-0"> | |
| <Button | |
| variant="outline" | |
| size="icon" | |
| onClick={audioRecorder.togglePause} | |
| className="shrink-0" | |
| aria-label={audioRecorder.isPaused ? "Resume recording" : "Pause recording"} | |
| > | |
| {audioRecorder.isPaused ? <MicIcon className="w-4 h-4" /> : <span className="font-bold text-xs">||</span>} | |
| </Button> | |
| <Button | |
| variant="outline" | |
| size="icon" | |
| onClick={handleStopRecording} | |
| className="shrink-0 text-red-600 hover:text-red-700" | |
| aria-label="Stop and save recording" | |
| > | |
| <div className="w-3 h-3 bg-current rounded-sm" /> | |
| </Button> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| onClick={audioRecorder.cancelRecording} | |
| className="shrink-0" | |
| aria-label="Cancel recording" | |
| > |
Uh oh!
There was an error while loading. Please reload this page.