From db3235fcc52d49ec03e01a8c75fbcad7afa5f45c Mon Sep 17 00:00:00 2001 From: Amitoj Singh Date: Mon, 5 Jan 2026 10:46:32 -0700 Subject: [PATCH 1/3] Refactor AudioPlayer and AudioControlBar for improved seek handling and UI responsiveness - Updated handleSeek function in AudioPlayer to use useCallback for better performance. - Enhanced AudioControlBar with new slider handling logic, including optimistic updates during dragging. - Implemented state management for slider value and seek loading to improve user experience. - Added scroll state reset functionality in useAudioSyncScroll to handle seeking more effectively. --- .../components/AudioControlBar/index.jsx | 66 +++++++++++++++++-- .../hooks/useAudioSyncScroll/index.js | 38 +++++++++-- .../components/AudioPlayer/index.jsx | 21 +++--- 3 files changed, 107 insertions(+), 18 deletions(-) diff --git a/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx b/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx index c90ecfd4..b3225327 100644 --- a/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx +++ b/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useMemo, useRef } from "react"; +import React, { useState, useEffect, useMemo, useRef, useCallback } from "react"; import { View, Pressable, Animated, Platform, ActivityIndicator } from "react-native"; import { useDispatch, useSelector } from "react-redux"; import { Slider } from "@miblanchard/react-native-slider"; @@ -68,6 +68,9 @@ const AudioControlBar = ({ const currentPlayingRef = useRef(currentPlaying); const audioProgress = useSelector((state) => state.audioProgress); const [isSeekLoading, setIsSeekLoading] = useState(false); + const [isSliding, setIsSliding] = useState(false); + const [sliderValue, setSliderValue] = useState(progress.position); + const isSeekingRef = useRef(false); const { modalHeight, modalOpacity } = useAnimation(isSettingsModalOpen, isMoreTracksModalOpen); const { isDownloading, isDownloaded } = useDownloadManager( currentPlaying, @@ -97,6 +100,13 @@ const AudioControlBar = ({ currentPlayingRef.current = currentPlaying; }, [currentPlaying]); + // Sync slider value with progress when not sliding and not seeking + useEffect(() => { + if (!isSliding && !isSeekingRef.current) { + setSliderValue(progress.position); + } + }, [progress.position, isSliding]); + // Save progress when user closes the modal const handleClose = async () => { const currentProgress = progressRef.current; @@ -276,6 +286,52 @@ const AudioControlBar = ({ return unsubscribe; }, [navigation]); + // Handle slider value change during dragging (optimistic update) + const handleSliderValueChange = useCallback( + (value) => { + if (isSliding) { + setSliderValue(value[0]); + } + }, + [isSliding] + ); + + // Handle slider drag start + const handleSlidingStart = useCallback(() => { + setIsSliding(true); + }, []); + + // Handle seek completion - non-blocking for smooth UI + const handleSeekComplete = useCallback( + (value) => { + // Prevent concurrent seeks + if (isSeekingRef.current || !isAudioEnabled) { + setIsSliding(false); + return; + } + + // Update UI immediately (optimistic update) + setIsSliding(false); + setSliderValue(value); + + // Perform seek operation asynchronously without blocking UI + (async () => { + try { + isSeekingRef.current = true; + setIsSeekLoading(true); + await handleSeek(value); + } catch (error) { + // Error is already handled in handleSeek + logError("Error in handleSeekComplete:", error); + } finally { + isSeekingRef.current = false; + setIsSeekLoading(false); + } + })(); + }, + [handleSeek, isAudioEnabled] + ); + return ( {isDownloading && !isDownloaded && } @@ -359,13 +415,15 @@ const AudioControlBar = ({ )} - {formatTime(progress.position)} + {formatTime(sliderValue)} handleSeek(v)} + onSlidingStart={handleSlidingStart} + onValueChange={handleSliderValueChange} + onSlidingComplete={([v]) => handleSeekComplete(v)} minimumTrackTintColor={sliderMinTrackColor} maximumTrackTintColor={theme.staticColors.SLIDER_TRACK_COLOR} disabled={!isAudioEnabled || isSeekLoading} diff --git a/src/ReaderScreen/components/AudioPlayer/hooks/useAudioSyncScroll/index.js b/src/ReaderScreen/components/AudioPlayer/hooks/useAudioSyncScroll/index.js index 2e94a59a..7ccf9e8e 100644 --- a/src/ReaderScreen/components/AudioPlayer/hooks/useAudioSyncScroll/index.js +++ b/src/ReaderScreen/components/AudioPlayer/hooks/useAudioSyncScroll/index.js @@ -8,6 +8,7 @@ const useAudioSyncScroll = (progress, isPlaying, webViewRef, lyricsUrl) => { const lastSequenceRef = useRef(null); const isScrollingRef = useRef(false); const scrollTimeoutRef = useRef(null); + const previousPositionRef = useRef(null); const [baniLRC, setBaniLRC] = useState(null); // Load LRC data when audioUrl changes @@ -47,6 +48,16 @@ const useAudioSyncScroll = (progress, isPlaying, webViewRef, lyricsUrl) => { }; }; + // Reset scroll state (used when seeking) + const resetScrollState = () => { + lastSequenceRef.current = null; + isScrollingRef.current = false; + if (scrollTimeoutRef.current) { + clearTimeout(scrollTimeoutRef.current); + scrollTimeoutRef.current = null; + } + }; + // Scroll to specific sequence in WebView const scrollToSequence = (sequence, timeOut) => { if (!webViewRef?.current?.postMessage || !sequence) { @@ -66,9 +77,8 @@ const useAudioSyncScroll = (progress, isPlaying, webViewRef, lyricsUrl) => { clearTimeout(scrollTimeoutRef.current); } - // Prevent multiple rapid scroll calls - if (isScrollingRef.current) return; - isScrollingRef.current = true; + // Reset scrolling flag to allow new scrolls (important for rapid seeks) + isScrollingRef.current = false; const scrollMessage = { action: "scrollToSequence", @@ -80,7 +90,8 @@ const useAudioSyncScroll = (progress, isPlaying, webViewRef, lyricsUrl) => { webViewRef.current.postMessage(JSON.stringify(scrollMessage)); - // Reset scrolling flag after a short delay + // Set scrolling flag and reset after a short delay + isScrollingRef.current = true; scrollTimeoutRef.current = setTimeout(() => { isScrollingRef.current = false; scrollTimeoutRef.current = null; @@ -100,7 +111,24 @@ const useAudioSyncScroll = (progress, isPlaying, webViewRef, lyricsUrl) => { if (!isAudioSyncScroll || !isPlaying || !progress?.position || !baniLRC) { return; } - const { currentSequence, timeOut } = findCurrentSequence(progress.position); + + const currentPosition = progress.position; + const previousPosition = previousPositionRef.current; + + // Detect seek: if position jumps more than 2 seconds, it's likely a seek + // Normal playback won't jump more than ~1 second per update + if (previousPosition !== null) { + const positionDiff = Math.abs(currentPosition - previousPosition); + if (positionDiff > 2) { + // Reset scroll state when seek is detected + resetScrollState(); + } + } + + // Update previous position + previousPositionRef.current = currentPosition; + + const { currentSequence, timeOut } = findCurrentSequence(currentPosition); // Only scroll if sequence changed and we have a valid sequence if (currentSequence !== null && currentSequence !== lastSequenceRef.current) { diff --git a/src/ReaderScreen/components/AudioPlayer/index.jsx b/src/ReaderScreen/components/AudioPlayer/index.jsx index 00c36193..74d87bc6 100644 --- a/src/ReaderScreen/components/AudioPlayer/index.jsx +++ b/src/ReaderScreen/components/AudioPlayer/index.jsx @@ -116,15 +116,18 @@ const AudioPlayer = ({ baniID, title, webViewRef }) => { } }, [currentPlaying, defaultAudio, baniID]); - const handleSeek = async (value) => { - if (!isAudioEnabled || !isInitialized) return; - try { - await seekTo(value); - } catch (error) { - logError("Error seeking:", error); - showErrorToast(`${STRINGS.UNABLE_TO_SEEK} ${STRINGS.PLEASE_TRY_AGAIN}`); - } - }; + const handleSeek = useCallback( + async (value) => { + if (!isAudioEnabled || !isInitialized) return; + try { + await seekTo(value); + } catch (error) { + logError("Error seeking:", error); + showErrorToast(`${STRINGS.UNABLE_TO_SEEK} ${STRINGS.PLEASE_TRY_AGAIN}`); + } + }, + [isAudioEnabled, isInitialized, seekTo] + ); const handleTrackSelect = useCallback( async (selectedTrack) => { From e1899ba9b78597d11be8926a1ee3e3d6ff23fcd2 Mon Sep 17 00:00:00 2001 From: Amitoj Singh Date: Mon, 23 Feb 2026 15:24:21 -0700 Subject: [PATCH 2/3] fix seeking --- .../components/AudioControlBar/index.jsx | 80 +++++++++++-------- .../hooks/useDownloadManager/index.js | 2 + .../AudioPlayer/hooks/useTrackPlayer/index.js | 5 ++ src/services/audioApi.js | 1 - 4 files changed, 52 insertions(+), 36 deletions(-) diff --git a/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx b/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx index b3225327..75bac8f6 100644 --- a/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx +++ b/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx @@ -54,6 +54,7 @@ const AudioControlBar = ({ isInitialized, addAndPlayTrack, play, + isBufferingOrLoading, }) => { const dispatch = useDispatch(); const { theme } = useTheme(); @@ -67,7 +68,9 @@ const AudioControlBar = ({ const progressRef = useRef(progress); const currentPlayingRef = useRef(currentPlaying); const audioProgress = useSelector((state) => state.audioProgress); - const [isSeekLoading, setIsSeekLoading] = useState(false); + const [isSeekLoading, setIsSeekLoading] = useState(() => { + return !!(isInitialized && currentPlaying?.id && currentPlaying?.audioUrl); + }); const [isSliding, setIsSliding] = useState(false); const [sliderValue, setSliderValue] = useState(progress.position); const isSeekingRef = useRef(false); @@ -100,12 +103,33 @@ const AudioControlBar = ({ currentPlayingRef.current = currentPlaying; }, [currentPlaying]); - // Sync slider value with progress when not sliding and not seeking useEffect(() => { - if (!isSliding && !isSeekingRef.current) { + if (currentPlaying?.id) { + setIsSeekLoading(true); // start loading immediately on track change + } else { + setIsSeekLoading(false); + } + setSliderValue(0); + }, [currentPlaying?.id]); + + // Clear loading once TrackPlayer reports duration and buffering is done + useEffect(() => { + if ( + currentPlaying?.id && + progress.duration > 0 && + !isBufferingOrLoading && + !isSeekingRef.current + ) { + setIsSeekLoading(false); + } + }, [currentPlaying?.id, progress.duration, isBufferingOrLoading]); + + // Sync slider value with progress when not sliding and not seeking (and not loading) + useEffect(() => { + if (!isSliding && !isSeekingRef.current && !isSeekLoading) { setSliderValue(progress.position); } - }, [progress.position, isSliding]); + }, [progress.position, isSliding, isSeekLoading]); // Save progress when user closes the modal const handleClose = async () => { @@ -188,10 +212,6 @@ const AudioControlBar = ({ // Load the active track when component mounts or currentPlaying changes useEffect(() => { const loadActiveTrack = async () => { - if (!isInitialized || !currentPlaying?.id || !currentPlaying?.audioUrl) { - return; - } - try { setIsSeekLoading(true); // Load the track (will seek to saved position if available) @@ -222,7 +242,6 @@ const AudioControlBar = ({ if (isAudioAutoPlay) { await play(); } - setIsSeekLoading(false); return; } } @@ -236,23 +255,16 @@ const AudioControlBar = ({ if (isAudioAutoPlay) { await play(); } - setIsSeekLoading(false); } catch (error) { logError("Error loading active track:", error); setIsSeekLoading(false); } }; - loadActiveTrack(); - }, [ - isInitialized, - currentPlaying?.id, - currentPlaying?.audioUrl, - currentPlaying?.displayName, - currentPlaying?.lyricsUrl, - audioProgress, - baniID, - ]); + if (isInitialized && currentPlaying?.id && currentPlaying?.audioUrl) { + loadActiveTrack(); + } + }, [isInitialized, currentPlaying?.id, currentPlaying?.audioUrl, audioProgress, baniID]); // Save audio progress when component unmounts or user leaves the screen useEffect(() => { @@ -304,14 +316,13 @@ const AudioControlBar = ({ // Handle seek completion - non-blocking for smooth UI const handleSeekComplete = useCallback( (value) => { + setIsSliding(false); // Prevent concurrent seeks if (isSeekingRef.current || !isAudioEnabled) { - setIsSliding(false); return; } // Update UI immediately (optimistic update) - setIsSliding(false); setSliderValue(value); // Perform seek operation asynchronously without blocking UI @@ -399,21 +410,19 @@ const AudioControlBar = ({ - - {isPlaying ? ( - - ) : ( - - )} - - + {isSeekLoading || isBufferingOrLoading || !currentPlaying?.id ? ( + + ) : ( + + {isPlaying ? ( + + ) : ( + + )} + + )} - {isSeekLoading && ( - - - - )} {formatTime(sliderValue)} @@ -485,6 +494,7 @@ AudioControlBar.propTypes = { isInitialized: PropTypes.bool.isRequired, addAndPlayTrack: PropTypes.func.isRequired, play: PropTypes.func.isRequired, + isBufferingOrLoading: PropTypes.bool.isRequired, }; export default AudioControlBar; diff --git a/src/ReaderScreen/components/AudioPlayer/hooks/useDownloadManager/index.js b/src/ReaderScreen/components/AudioPlayer/hooks/useDownloadManager/index.js index 99f09008..a7373407 100644 --- a/src/ReaderScreen/components/AudioPlayer/hooks/useDownloadManager/index.js +++ b/src/ReaderScreen/components/AudioPlayer/hooks/useDownloadManager/index.js @@ -27,6 +27,7 @@ const useDownloadManager = (currentPlaying, addTrackToManifest, isTrackDownloade try { const downloaded = checkDownloadStatus(); if (downloaded) { + setIsDownloaded(true); return; } @@ -49,6 +50,7 @@ const useDownloadManager = (currentPlaying, addTrackToManifest, isTrackDownloade const isDownloadedStatus = checkDownloadStatus(); if (currentPlaying?.audioUrl && !isDownloadedStatus) { + setIsDownloaded(false); await handleDownload(); } else { setIsDownloaded(true); diff --git a/src/ReaderScreen/components/AudioPlayer/hooks/useTrackPlayer/index.js b/src/ReaderScreen/components/AudioPlayer/hooks/useTrackPlayer/index.js index 86dd3aa6..bd5516c7 100644 --- a/src/ReaderScreen/components/AudioPlayer/hooks/useTrackPlayer/index.js +++ b/src/ReaderScreen/components/AudioPlayer/hooks/useTrackPlayer/index.js @@ -21,6 +21,8 @@ const useTrackPlayer = () => { const playbackState = usePlaybackState(); const progress = useProgress(); const [isPlaying, setIsPlaying] = useState(false); + const [isBuffering, setIsBuffering] = useState(false); + const [isLoading, setIsLoading] = useState(false); const isAudio = useSelector((state) => state.isAudio); const configurePlayer = useCallback(async () => { @@ -72,6 +74,8 @@ const useTrackPlayer = () => { useEffect(() => { if (!isInitialized) return; setIsPlaying(playbackState?.state === State.Playing); + setIsBuffering(playbackState?.state === State.Buffering); + setIsLoading(playbackState?.state === State.Loading); }, [playbackState, isInitialized]); const play = async () => { @@ -204,6 +208,7 @@ const useTrackPlayer = () => { isAudioEnabled: isAudio && isInitialized, isInitialized, setIsPlaying, + isBufferingOrLoading: isBuffering || isLoading, isInitializing, initializationError, retryInitialization, diff --git a/src/services/audioApi.js b/src/services/audioApi.js index 9e23d7db..1108bbf7 100644 --- a/src/services/audioApi.js +++ b/src/services/audioApi.js @@ -34,7 +34,6 @@ const makeApiRequest = async (endpoint, options = {}) => { return data; } catch (error) { // Network error - show toast and continue without audio features - showErrorToast(STRINGS.NETWORK_ERROR); return null; } }; From 7160ea64136f4810cc6159d98484c0748b925f60 Mon Sep 17 00:00:00 2001 From: Amitoj Singh Date: Mon, 23 Feb 2026 16:55:45 -0700 Subject: [PATCH 3/3] Fix seek loading state management in AudioControlBar - Added setIsSeekLoading(false) to ensure loading state is reset after seeking operations. - Updated test timeouts for better reliability in asynchronous tests. - Improved test assertions for slider state and function calls to enhance clarity and maintainability. --- .../components/AudioControlBar/index.jsx | 2 ++ .../components/AudioControlBar/index.test.jsx | 14 +++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx b/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx index 75bac8f6..e7e7c873 100644 --- a/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx +++ b/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.jsx @@ -242,6 +242,7 @@ const AudioControlBar = ({ if (isAudioAutoPlay) { await play(); } + setIsSeekLoading(false); return; } } @@ -255,6 +256,7 @@ const AudioControlBar = ({ if (isAudioAutoPlay) { await play(); } + setIsSeekLoading(false); } catch (error) { logError("Error loading active track:", error); setIsSeekLoading(false); diff --git a/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.test.jsx b/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.test.jsx index 959fea8f..21e23209 100644 --- a/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.test.jsx +++ b/src/ReaderScreen/components/AudioPlayer/components/AudioControlBar/index.test.jsx @@ -375,7 +375,7 @@ describe("AudioControlBar", () => { const slider = getByTestId("slider"); expect(slider.props.disabled).toBe(true); }, - { timeout: 100 } + { timeout: 100 }, ); // Resolve the seek promise to complete the loading @@ -387,7 +387,7 @@ describe("AudioControlBar", () => { const slider = getByTestId("slider"); expect(slider.props.disabled).toBe(false); }, - { timeout: 500 } + { timeout: 500 }, ); }); @@ -435,7 +435,7 @@ describe("AudioControlBar", () => { await waitFor(() => { expect(mockGetSequenceFromPosition).toHaveBeenCalledWith( defaultCurrentTrack.lyricsUrl, - defaultProgress.position + defaultProgress.position, ); expect(mockSetAudioProgress).toHaveBeenCalledWith("bani-1", "track-1", 10, null); expect(props.reset).toHaveBeenCalledTimes(1); @@ -478,7 +478,7 @@ describe("AudioControlBar", () => { defaultCurrentTrack.trackLengthSec, defaultCurrentTrack.trackSizeMB, false, - defaultCurrentTrack.remoteUrl || defaultCurrentTrack.audioUrl + defaultCurrentTrack.remoteUrl || defaultCurrentTrack.audioUrl, ); }); @@ -539,17 +539,17 @@ describe("AudioControlBar", () => { () => { expect(mockCheckLyricsFileAvailable).toHaveBeenCalledWith(defaultCurrentTrack.lyricsUrl); }, - { timeout: 1000 } + { timeout: 1000 }, ); await waitFor( () => { expect(mockToggleAudioSyncScroll).toHaveBeenCalledWith(true); expect(mockDispatch).toHaveBeenCalledWith( - expect.objectContaining({ type: "TOGGLE_AUDIO_SYNC_SCROLL" }) + expect.objectContaining({ type: "TOGGLE_AUDIO_SYNC_SCROLL" }), ); }, - { timeout: 1000 } + { timeout: 1000 }, ); });