From 5e2bd4efa8cf4d89fe838eee07c081a3d6e64afd Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sun, 14 Jun 2026 18:01:06 +0200 Subject: [PATCH] fix(video): release scrub chrome hold close #1340 --- .../desktop_video_controls.dart | 6 ++ .../video_controls/mobile_video_controls.dart | 6 ++ .../video_controls/parts/navigation.dart | 2 + .../video_controls/parts/playback_input.dart | 10 ++- .../video_controls/video_controls.dart | 2 + .../widgets/timeline_slider.dart | 17 +++- .../widgets/video_timeline_bar.dart | 6 ++ test/widgets/video_controls_test.dart | 78 ++++++++++++++++++- 8 files changed, 122 insertions(+), 5 deletions(-) diff --git a/lib/widgets/video_controls/desktop_video_controls.dart b/lib/widgets/video_controls/desktop_video_controls.dart index b3e90128..9ba5058f 100644 --- a/lib/widgets/video_controls/desktop_video_controls.dart +++ b/lib/widgets/video_controls/desktop_video_controls.dart @@ -44,6 +44,8 @@ class DesktopVideoControls extends StatefulWidget { final VoidCallback? onSeekForward; final ValueChanged onSeek; final ValueChanged onSeekEnd; + final VoidCallback? onScrubStart; + final VoidCallback? onScrubEnd; final IconData Function(int) getReplayIcon; final IconData Function(int) getForwardIcon; @@ -122,6 +124,8 @@ class DesktopVideoControls extends StatefulWidget { this.onSeekForward, required this.onSeek, required this.onSeekEnd, + this.onScrubStart, + this.onScrubEnd, required this.getReplayIcon, required this.getForwardIcon, this.onFocusActivity, @@ -717,6 +721,8 @@ class DesktopVideoControlsState extends State { showChapterMarkersOnTimeline: widget.showChapterMarkersOnTimeline, onSeek: widget.onSeek, onSeekEnd: widget.onSeekEnd, + onScrubStart: widget.onScrubStart, + onScrubEnd: widget.onScrubEnd, horizontalLayout: true, focusNode: _timelineFocusNode, onKeyEvent: _handleTimelineKeyEvent, diff --git a/lib/widgets/video_controls/mobile_video_controls.dart b/lib/widgets/video_controls/mobile_video_controls.dart index 0db026de..2ea2350a 100644 --- a/lib/widgets/video_controls/mobile_video_controls.dart +++ b/lib/widgets/video_controls/mobile_video_controls.dart @@ -37,6 +37,8 @@ class MobileVideoControls extends StatefulWidget { final Widget trackChapterControls; final Function(Duration) onSeek; final Function(Duration) onSeekEnd; + final VoidCallback? onScrubStart; + final VoidCallback? onScrubEnd; final Future Function(Duration position)? onSeekRequested; final Function(Duration)? onSeekCompleted; final VoidCallback onPlayPause; @@ -96,6 +98,8 @@ class MobileVideoControls extends StatefulWidget { required this.trackChapterControls, required this.onSeek, required this.onSeekEnd, + this.onScrubStart, + this.onScrubEnd, required this.onPlayPause, this.onSeekRequested, this.onSeekCompleted, @@ -453,6 +457,8 @@ class _MobileVideoControlsState extends State with SingleTi showChapterMarkersOnTimeline: widget.showChapterMarkersOnTimeline, onSeek: widget.onSeek, onSeekEnd: widget.onSeekEnd, + onScrubStart: widget.onScrubStart, + onScrubEnd: widget.onScrubEnd, horizontalLayout: false, enabled: widget.canControl, showFinishTime: true, diff --git a/lib/widgets/video_controls/parts/navigation.dart b/lib/widgets/video_controls/parts/navigation.dart index 9fc37429..fa11b2ee 100644 --- a/lib/widgets/video_controls/parts/navigation.dart +++ b/lib/widgets/video_controls/parts/navigation.dart @@ -28,6 +28,8 @@ extension _PlexVideoControlsNavigationMethods on _PlexVideoControlsState { onSeekForward: () => unawaited(_seekByTime(forward: true)), onSeek: _throttledSeek, onSeekEnd: _finalizeSeek, + onScrubStart: _holdTimelineScrub, + onScrubEnd: _releaseTimelineScrub, onSeekRequested: widget.onSeekRequested, getReplayIcon: getReplayIcon, getForwardIcon: getForwardIcon, diff --git a/lib/widgets/video_controls/parts/playback_input.dart b/lib/widgets/video_controls/parts/playback_input.dart index 7f41a373..45dd572c 100644 --- a/lib/widgets/video_controls/parts/playback_input.dart +++ b/lib/widgets/video_controls/parts/playback_input.dart @@ -89,9 +89,6 @@ extension _PlexVideoControlsPlaybackInputMethods on _PlexVideoControlsState { /// Throttled seek for timeline slider - executes immediately then throttles to 200ms void _throttledSeek(Duration position) { - // Hold before the transcoding early-return so a slow scrub never loses - // the controls to auto-hide mid-drag (idempotent while held). - widget.chromeController.hold(PlayerChromeHold.scrub); if (widget.isTranscoding) return; _seekThrottle([position]); } @@ -100,6 +97,13 @@ extension _PlexVideoControlsPlaybackInputMethods on _PlexVideoControlsState { void _finalizeSeek(Duration position) { _seekThrottle.cancel(); unawaited(_seekToPosition(position)); + } + + void _holdTimelineScrub() { + widget.chromeController.hold(PlayerChromeHold.scrub); + } + + void _releaseTimelineScrub() { widget.chromeController.release(PlayerChromeHold.scrub); } diff --git a/lib/widgets/video_controls/video_controls.dart b/lib/widgets/video_controls/video_controls.dart index f6468afc..366bf5a5 100644 --- a/lib/widgets/video_controls/video_controls.dart +++ b/lib/widgets/video_controls/video_controls.dart @@ -802,6 +802,8 @@ class _PlexVideoControlsState extends State ), onSeek: _throttledSeek, onSeekEnd: _finalizeSeek, + onScrubStart: _holdTimelineScrub, + onScrubEnd: _releaseTimelineScrub, onSeekRequested: widget.onSeekRequested, onSeekCompleted: widget.onSeekCompleted, // ignore: no-empty-block - play/pause handled by parent VideoControlsState diff --git a/lib/widgets/video_controls/widgets/timeline_slider.dart b/lib/widgets/video_controls/widgets/timeline_slider.dart index 0c9d7a8b..b48bfc2e 100644 --- a/lib/widgets/video_controls/widgets/timeline_slider.dart +++ b/lib/widgets/video_controls/widgets/timeline_slider.dart @@ -23,6 +23,8 @@ class TimelineSlider extends StatefulWidget { final bool showChapterMarkersOnTimeline; final ValueChanged onSeek; final ValueChanged onSeekEnd; + final VoidCallback? onScrubStart; + final VoidCallback? onScrubEnd; /// Optional FocusNode for D-pad/keyboard navigation. final FocusNode? focusNode; @@ -56,6 +58,8 @@ class TimelineSlider extends StatefulWidget { this.showChapterMarkersOnTimeline = true, required this.onSeek, required this.onSeekEnd, + this.onScrubStart, + this.onScrubEnd, this.focusNode, this.onKeyEvent, this.onFocusChange, @@ -84,6 +88,12 @@ class _TimelineSliderState extends State { static const _thumbWidth = 160.0; + @override + void dispose() { + if (_scrubbing) widget.onScrubEnd?.call(); + super.dispose(); + } + @override void didUpdateWidget(TimelineSlider oldWidget) { super.didUpdateWidget(oldWidget); @@ -100,6 +110,7 @@ class _TimelineSliderState extends State { void _handleScrubStart(DragStartDetails details, BuildContext sliderContext) { if (widget.duration.inMilliseconds <= 0) return; _scrubbing = true; + widget.onScrubStart?.call(); _applyScrub(details.localPosition.dx, sliderContext); } @@ -125,7 +136,11 @@ class _TimelineSliderState extends State { _scrubbing = false; final value = _dragValue; setState(() => _dragValue = null); - if (value != null) widget.onSeekEnd(Duration(milliseconds: value.round())); + try { + if (value != null) widget.onSeekEnd(Duration(milliseconds: value.round())); + } finally { + widget.onScrubEnd?.call(); + } } /// Discrete a11y step (VoiceOver/TalkBack swipe): a complete seek. diff --git a/lib/widgets/video_controls/widgets/video_timeline_bar.dart b/lib/widgets/video_controls/widgets/video_timeline_bar.dart index b636cbaf..989e0fb0 100644 --- a/lib/widgets/video_controls/widgets/video_timeline_bar.dart +++ b/lib/widgets/video_controls/widgets/video_timeline_bar.dart @@ -18,6 +18,8 @@ class VideoTimelineBar extends StatelessWidget { final bool showChapterMarkersOnTimeline; final ValueChanged onSeek; final ValueChanged onSeekEnd; + final VoidCallback? onScrubStart; + final VoidCallback? onScrubEnd; /// If true, timestamps are shown in a row beside the slider (desktop layout). /// If false, timestamps are shown in a row below the slider (mobile layout). @@ -53,6 +55,8 @@ class VideoTimelineBar extends StatelessWidget { this.showChapterMarkersOnTimeline = true, required this.onSeek, required this.onSeekEnd, + this.onScrubStart, + this.onScrubEnd, this.horizontalLayout = true, this.focusNode, this.onKeyEvent, @@ -166,6 +170,8 @@ class VideoTimelineBar extends StatelessWidget { showChapterMarkersOnTimeline: showChapterMarkersOnTimeline, onSeek: onSeek, onSeekEnd: onSeekEnd, + onScrubStart: onScrubStart, + onScrubEnd: onScrubEnd, focusNode: focusNode, onKeyEvent: onKeyEvent, onFocusChange: onFocusChange, diff --git a/test/widgets/video_controls_test.dart b/test/widgets/video_controls_test.dart index 59af229b..d0cf8c83 100644 --- a/test/widgets/video_controls_test.dart +++ b/test/widgets/video_controls_test.dart @@ -425,6 +425,8 @@ void main() { required List seekEnds, Duration duration = const Duration(minutes: 10), bool enabled = true, + VoidCallback? onScrubStart, + VoidCallback? onScrubEnd, Widget Function(Widget child)? wrap, }) async { Widget slider = SizedBox( @@ -437,6 +439,8 @@ void main() { enabled: enabled, onSeek: seeks.add, onSeekEnd: seekEnds.add, + onScrubStart: onScrubStart, + onScrubEnd: onScrubEnd, ), ); if (wrap != null) slider = wrap(slider); @@ -450,7 +454,15 @@ void main() { testWidgets('touch drag survives tooltip appearance and finalizes once', (tester) async { final seeks = []; final seekEnds = []; - await pumpScrubSlider(tester, seeks: seeks, seekEnds: seekEnds); + var scrubStarts = 0; + var scrubEnds = 0; + await pumpScrubSlider( + tester, + seeks: seeks, + seekEnds: seekEnds, + onScrubStart: () => scrubStarts++, + onScrubEnd: () => scrubEnds++, + ); // Down at the center (200/400 → 5min), drag +100px (→ 7.5min). The // first scrub event makes the tooltip appear; the drag must keep @@ -466,9 +478,73 @@ void main() { expect(seeks, isNotEmpty); expect(seekEnds, hasLength(1)); + expect(scrubStarts, 1); + expect(scrubEnds, 1); expect(seekEnds.single.inMilliseconds, closeTo(const Duration(minutes: 7, seconds: 30).inMilliseconds, 2000)); }); + testWidgets('keyboard input does not start a scrub lifecycle', (tester) async { + final focusNode = FocusNode(); + addTearDown(focusNode.dispose); + var scrubStarts = 0; + var scrubEnds = 0; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + width: 400, + child: TimelineSlider( + position: const Duration(minutes: 1), + duration: const Duration(minutes: 10), + chapters: const [], + chaptersLoaded: true, + focusNode: focusNode, + onKeyEvent: (_, event) => KeyEventResult.handled, + onSeek: (_) {}, + onSeekEnd: (_) {}, + onScrubStart: () => scrubStarts++, + onScrubEnd: () => scrubEnds++, + ), + ), + ), + ), + ); + + focusNode.requestFocus(); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + + expect(scrubStarts, 0); + expect(scrubEnds, 0); + }); + + testWidgets('disposing mid-drag ends the scrub lifecycle', (tester) async { + final seeks = []; + final seekEnds = []; + var scrubStarts = 0; + var scrubEnds = 0; + await pumpScrubSlider( + tester, + seeks: seeks, + seekEnds: seekEnds, + onScrubStart: () => scrubStarts++, + onScrubEnd: () => scrubEnds++, + ); + + final gesture = await tester.startGesture(tester.getCenter(find.byType(TimelineSlider))); + await tester.pump(); + expect(scrubStarts, 1); + expect(scrubEnds, 0); + + await tester.pumpWidget(const SizedBox.shrink()); + await tester.pump(); + await gesture.cancel(); + + expect(scrubEnds, 1); + }); + testWidgets('tap seeks to the tapped position', (tester) async { final seeks = []; final seekEnds = [];