fix(mobile): convert video controls from hook to stateful widget (#27514)

We are generally looking to move away from hooks as they are hard to
reason about and have weird bugs. In particular, the timer did not
properly capture the ref of the callback, and so it would execute on old
state. A standard stateful widget does not have this problem, and is
easier to organise.

Co-authored-by: Alex <alex.tran1502@gmail.com>
This commit is contained in:
Thomas 2026-04-06 16:13:45 +01:00 committed by GitHub
parent 2143a0c935
commit fbe631fe91
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 63 additions and 29 deletions

View File

@ -74,7 +74,7 @@ class ViewerBottomBar extends ConsumerWidget {
child: Column(
mainAxisSize: MainAxisSize.min,
children: [
if (asset.isVideo) VideoControls(key: ValueKey(asset.heroTag), videoPlayerName: asset.heroTag),
if (asset.isVideo) VideoControls(videoPlayerName: asset.heroTag),
if (!isReadonlyModeEnabled)
Row(mainAxisAlignment: MainAxisAlignment.spaceEvenly, children: actions),
],

View File

@ -1,5 +1,6 @@
import 'dart:math';
import 'package:async/async.dart';
import 'package:flutter/material.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:immich_mobile/constants/colors.dart';
@ -7,26 +8,63 @@ import 'package:immich_mobile/models/cast/cast_manager_state.dart';
import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart';
import 'package:immich_mobile/providers/asset_viewer/video_player_provider.dart';
import 'package:immich_mobile/providers/cast.provider.dart';
import 'package:immich_mobile/utils/hooks/timer_hook.dart';
import 'package:immich_mobile/extensions/duration_extensions.dart';
import 'package:immich_mobile/widgets/asset_viewer/animated_play_pause.dart';
class VideoControls extends HookConsumerWidget {
class VideoControls extends ConsumerStatefulWidget {
final String videoPlayerName;
static const List<Shadow> _controlShadows = [Shadow(color: Colors.black87, blurRadius: 6, offset: Offset(0, 1))];
const VideoControls({super.key, required this.videoPlayerName});
void _toggle(WidgetRef ref, bool isCasting) {
if (isCasting) {
ref.read(castProvider.notifier).toggle();
} else {
ref.read(videoPlayerProvider(videoPlayerName).notifier).toggle();
@override
ConsumerState<VideoControls> createState() => _VideoControlsState();
}
class _VideoControlsState extends ConsumerState<VideoControls> {
late final RestartableTimer _hideTimer;
AutoDisposeStateNotifierProvider<VideoPlayerNotifier, VideoPlayerState> get _provider =>
videoPlayerProvider(widget.videoPlayerName);
@override
void initState() {
super.initState();
_hideTimer = RestartableTimer(const Duration(seconds: 5), _onHideTimer);
}
@override
void didUpdateWidget(covariant VideoControls oldWidget) {
super.didUpdateWidget(oldWidget);
if (oldWidget.videoPlayerName != widget.videoPlayerName) {
_hideTimer.reset();
}
}
void _onSeek(WidgetRef ref, bool isCasting, double value) {
@override
void dispose() {
_hideTimer.cancel();
super.dispose();
}
void _onHideTimer() {
if (!mounted) return;
if (ref.read(_provider).status == VideoPlaybackStatus.playing) {
ref.read(assetViewerProvider.notifier).setControls(false);
}
}
void _toggle(bool isCasting) {
if (isCasting) {
ref.read(castProvider.notifier).toggle();
return;
}
ref.read(_provider.notifier).toggle();
}
void _onSeek(bool isCasting, double value) {
final seekTo = Duration(microseconds: value.toInt());
if (isCasting) {
@ -34,38 +72,30 @@ class VideoControls extends HookConsumerWidget {
return;
}
ref.read(videoPlayerProvider(videoPlayerName).notifier).seekTo(seekTo);
ref.read(_provider.notifier).seekTo(seekTo);
}
@override
Widget build(BuildContext context, WidgetRef ref) {
final provider = videoPlayerProvider(videoPlayerName);
Widget build(BuildContext context) {
final cast = ref.watch(castProvider);
final isCasting = cast.isCasting;
final (position, duration) = isCasting
? ref.watch(castProvider.select((c) => (c.currentTime, c.duration)))
: ref.watch(provider.select((v) => (v.position, v.duration)));
: ref.watch(_provider.select((v) => (v.position, v.duration)));
final videoStatus = ref.watch(provider.select((v) => v.status));
final videoStatus = ref.watch(_provider.select((v) => v.status));
final isPlaying = isCasting
? cast.castState == CastState.playing
: videoStatus == VideoPlaybackStatus.playing || videoStatus == VideoPlaybackStatus.buffering;
final isFinished = !isCasting && videoStatus == VideoPlaybackStatus.completed;
final hideTimer = useTimer(const Duration(seconds: 5), () {
if (!context.mounted) return;
if (ref.read(provider).status == VideoPlaybackStatus.playing) {
ref.read(assetViewerProvider.notifier).setControls(false);
}
});
ref.listen(assetViewerProvider.select((v) => v.showingControls), (prev, showing) {
if (showing && prev != showing) hideTimer.reset();
if (showing && prev != showing) _hideTimer.reset();
});
ref.listen(provider.select((v) => v.status), (_, __) => hideTimer.reset());
ref.listen(_provider.select((v) => v.status), (_, __) => _hideTimer.reset());
final notifier = ref.read(provider.notifier);
final notifier = ref.read(_provider.notifier);
final isLoaded = duration != Duration.zero;
return Padding(
@ -80,9 +110,13 @@ class VideoControls extends HookConsumerWidget {
padding: const EdgeInsets.all(12),
constraints: const BoxConstraints(),
icon: isFinished
? const Icon(Icons.replay, color: Colors.white, shadows: _controlShadows)
: AnimatedPlayPause(color: Colors.white, playing: isPlaying, shadows: _controlShadows),
onPressed: () => _toggle(ref, isCasting),
? const Icon(Icons.replay, color: Colors.white, shadows: VideoControls._controlShadows)
: AnimatedPlayPause(
color: Colors.white,
playing: isPlaying,
shadows: VideoControls._controlShadows,
),
onPressed: () => _toggle(isCasting),
),
const Spacer(),
Text(
@ -91,7 +125,7 @@ class VideoControls extends HookConsumerWidget {
color: Colors.white,
fontWeight: FontWeight.w500,
fontFeatures: [FontFeature.tabularFigures()],
shadows: _controlShadows,
shadows: VideoControls._controlShadows,
),
),
const SizedBox(width: 12),
@ -107,7 +141,7 @@ class VideoControls extends HookConsumerWidget {
padding: EdgeInsets.zero,
onChangeStart: (_) => notifier.hold(),
onChangeEnd: (_) => notifier.release(),
onChanged: isLoaded ? (value) => _onSeek(ref, isCasting, value) : null,
onChanged: isLoaded ? (value) => _onSeek(isCasting, value) : null,
),
],
),