From 09bc2b6f3c7446283e29cfd997ac6ecd8dee06b8 Mon Sep 17 00:00:00 2001 From: Ghbarker Date: Mon, 7 Sep 2026 23:08:00 -0400 Subject: [PATCH] fix(tv-motion): route the pre-#276 scroll-follow sites through the TV motion profile The Smooth/Snappy profile (PR #276) and its shared AppMotion.tvFocus / tvScroll wiring are correct end to end: the settings row persists the choice, TvMotionController.select republishes it synchronously, and main.dart's explicit addListener/setState rebuilds TvMotionScope so every AppMotion.of(context) consumer retargets on the next frame (test/theme/tv_motion_profile_test.dart and test/settings_tv_motion_row_test.dart already pin this). The reason Smooth read as unchanged on a real Shield is that several DPAD scroll-follow sites bypass that wiring entirely. Two came before the motion profile existed: - board_cell.dart's board card (_StremioCard) and favourite_art_cell's ArtPoster hardcoded a flat 140ms/260ms glide with no profile branch at all (predates PR #276 by two days). - episodes_panel.dart's _CompactEpisodeRow hardcoded 220ms, unlike its sibling EpisodeTile which already reads AppMotion.tvScroll. Three more were added six minutes AFTER #276 merged, by PR #281 (the rec-rail/grid scroll-follow for Marquee, Dossier and Console), which branched off before #276 landed and so never got the memo: all three hardcoded `duration: Duration.zero` unconditionally. Every site now resolves its duration through AppMotion.tvScroll / AppMotion.scrollTempo, matching the pattern catalog_item_tile.dart and detail_identity.dart already used. Snappy's figures are unchanged (140/220ms glides, Duration.zero jumps); Smooth now actually reaches the profile's 260ms glide on all of them. Added regression tests that fail on the pre-fix duration and pass after: tv_motion_profile_scroll_sites_test.dart pins the board card, detail_layout_rec_scroll_follow_motion_profile_test.dart pins the Marquee rail (Console/Dossier share the identical fix, exercised by the existing detail_layout_rec_scroll_follow_test.dart). --- lib/screens/search/board_cell.dart | 17 +- lib/screens/search/favourite_art_cell.dart | 18 +- lib/widgets/detail/detail_layout_console.dart | 12 +- lib/widgets/detail/detail_layout_dossier.dart | 13 +- lib/widgets/detail/detail_layout_marquee.dart | 14 +- lib/widgets/episodes_panel.dart | 10 +- ...rec_scroll_follow_motion_profile_test.dart | 189 ++++++++++++++++++ .../tv_motion_profile_scroll_sites_test.dart | 148 ++++++++++++++ 8 files changed, 406 insertions(+), 15 deletions(-) create mode 100644 test/detail_layout_rec_scroll_follow_motion_profile_test.dart create mode 100644 test/theme/tv_motion_profile_scroll_sites_test.dart diff --git a/lib/screens/search/board_cell.dart b/lib/screens/search/board_cell.dart index 234d390c8..827f9aa8c 100644 --- a/lib/screens/search/board_cell.dart +++ b/lib/screens/search/board_cell.dart @@ -4,6 +4,7 @@ import 'package:flutter/services.dart'; import '../../models/stremio_addon.dart'; import '../../services/main_page_bridge.dart'; +import '../../theme/app_motion.dart'; import '../../theme/app_theme_scope.dart'; import '../../utils/dialog_tap_guard.dart'; import '../../utils/tv_keys.dart'; @@ -341,6 +342,7 @@ class _StremioCardState extends State<_StremioCard> @override Widget build(BuildContext context) { final app = AppThemeScope.of(context); + final motion = AppMotion.of(context); final item = widget.item; final wide = widget.aspectRatio > 1; final poster = widget.artUrl ?? item.poster; @@ -544,11 +546,16 @@ class _StremioCardState extends State<_StremioCard> // repeat retargets the in-flight scroll from the CURRENT offset, // and a short glide converges on the focused card fast enough // that motion never reads as trailing the keypress (200ms felt - // laggy on-device). - duration: widget.isTelevision - ? const Duration(milliseconds: 140) - : const Duration(milliseconds: 260), - curve: Curves.easeOutCubic, + // laggy on-device). That figure is the SNAPPY profile's; under + // smooth this follows `AppMotion.tvScroll` instead, like every + // other TV scroll-follow — this predates the motion profile + // (added before PR #276) and was never routed through it. + duration: motion.scrollTempo( + widget.isTelevision, + const Duration(milliseconds: 260), + tvSnappy: const Duration(milliseconds: 140), + ), + curve: motion.tvScrollCurve, ); }); } diff --git a/lib/screens/search/favourite_art_cell.dart b/lib/screens/search/favourite_art_cell.dart index 5aed89492..c9a67eda1 100644 --- a/lib/screens/search/favourite_art_cell.dart +++ b/lib/screens/search/favourite_art_cell.dart @@ -3,6 +3,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import '../../services/main_page_bridge.dart'; +import '../../theme/app_motion.dart'; import '../../theme/app_theme_scope.dart'; import '../../utils/tv_keys.dart'; import '../../widgets/home/card_focus_rise.dart'; @@ -197,6 +198,7 @@ class _ArtPosterState extends State { @override Widget build(BuildContext context) { final app = AppThemeScope.of(context); + final motion = AppMotion.of(context); final url = widget.imageUrl; final hasImage = url != null && url.isNotEmpty; @@ -329,11 +331,17 @@ class _ArtPosterState extends State { // TV glides too (was a hard jump) — see _StremioCard: repeated // DPAD moves retarget the in-flight scroll, so held browsing // stays one continuous motion. Short on purpose; 200ms trailed - // the keypress on-device. - duration: widget.isTelevision - ? const Duration(milliseconds: 140) - : const Duration(milliseconds: 260), - curve: Curves.easeOutCubic, + // the keypress on-device. That figure is the SNAPPY profile's; + // under smooth this follows `AppMotion.tvScroll` instead, like + // every other TV scroll-follow — this predates the motion + // profile (added before PR #276) and was never routed through + // it. + duration: motion.scrollTempo( + widget.isTelevision, + const Duration(milliseconds: 260), + tvSnappy: const Duration(milliseconds: 140), + ), + curve: motion.tvScrollCurve, ); }); } diff --git a/lib/widgets/detail/detail_layout_console.dart b/lib/widgets/detail/detail_layout_console.dart index cba235b42..f35623a8c 100644 --- a/lib/widgets/detail/detail_layout_console.dart +++ b/lib/widgets/detail/detail_layout_console.dart @@ -6,6 +6,7 @@ import 'package:flutter/services.dart'; import '../../services/debrify_image_cache.dart'; import '../../services/imdb_enrichment_service.dart'; import '../../services/storage_service.dart'; +import '../../theme/app_motion.dart'; import '../../utils/platform_util.dart'; import '../episodes_panel.dart'; import '../parents_guide_section.dart'; @@ -1021,14 +1022,23 @@ class _ConsolePosterState extends State<_ConsolePoster> { // have moved focus programmatically instead of via the // framework's own key-driven traversal). Follow explicitly so // the cursor is never invisible. + // + // This predates the TV motion profile (PR #281 landed before + // #276) and was left on a bare snap. Route it through + // `AppMotion.tvScroll` like every other TV scroll-follow: zero + // under snappy (unchanged), the profile's glide under smooth. + // Off TV the jump is untouched. if (f) { + final tv = PlatformUtil.isTelevision; + final motion = AppMotion.of(context); WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted || !context.mounted) return; Scrollable.ensureVisible( context, alignment: 0.5, alignmentPolicy: ScrollPositionAlignmentPolicy.explicit, - duration: Duration.zero, + duration: tv ? motion.tvScroll : Duration.zero, + curve: motion.tvScrollCurve, ); }); } diff --git a/lib/widgets/detail/detail_layout_dossier.dart b/lib/widgets/detail/detail_layout_dossier.dart index 70b37ffc3..2ac35a6be 100644 --- a/lib/widgets/detail/detail_layout_dossier.dart +++ b/lib/widgets/detail/detail_layout_dossier.dart @@ -4,6 +4,8 @@ import 'package:flutter/services.dart'; import '../../services/debrify_image_cache.dart'; import '../../services/imdb_enrichment_service.dart'; +import '../../theme/app_motion.dart'; +import '../../utils/platform_util.dart'; import '../episodes_panel.dart'; import '../horizontal_mouse_wheel.dart'; import '../parents_guide_section.dart'; @@ -674,14 +676,23 @@ class _RecPosterState extends State<_RecPoster> { // `_rightKey` — may have moved focus programmatically instead // of via the framework's own key-driven traversal). Follow // explicitly so the cursor is never invisible. + // + // This predates the TV motion profile (PR #281 landed before + // #276) and was left on a bare snap. Route it through + // `AppMotion.tvScroll` like every other TV scroll-follow: zero + // under snappy (unchanged), the profile's glide under smooth. + // Off TV the jump is untouched. if (f) { + final tv = PlatformUtil.isTelevision; + final motion = AppMotion.of(context); WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted || !context.mounted) return; Scrollable.ensureVisible( context, alignment: 0.5, alignmentPolicy: ScrollPositionAlignmentPolicy.explicit, - duration: Duration.zero, + duration: tv ? motion.tvScroll : Duration.zero, + curve: motion.tvScrollCurve, ); }); } diff --git a/lib/widgets/detail/detail_layout_marquee.dart b/lib/widgets/detail/detail_layout_marquee.dart index 8be445812..dbe064255 100644 --- a/lib/widgets/detail/detail_layout_marquee.dart +++ b/lib/widgets/detail/detail_layout_marquee.dart @@ -3,6 +3,8 @@ import 'package:flutter/material.dart'; import '../../models/stremio_addon.dart'; import '../../services/debrify_image_cache.dart'; +import '../../theme/app_motion.dart'; +import '../../utils/platform_util.dart'; import '../episodes_panel.dart'; import '../horizontal_mouse_wheel.dart'; import 'detail_episode_cells.dart'; @@ -440,14 +442,24 @@ class _RecCardState extends State<_RecCard> { // ancestor onKeyEvent may have moved focus programmatically // instead of via the framework's own key-driven traversal). // Follow explicitly so the cursor is never invisible. + // + // This predates the TV motion profile (PR #281 landed + // before #276) and was left on a bare snap. Route it + // through `AppMotion.tvScroll` like every other TV + // scroll-follow: zero under snappy (unchanged), the + // profile's glide under smooth. Off TV the jump is + // untouched. if (f) { + final tv = PlatformUtil.isTelevision; + final motion = AppMotion.of(context); WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted || !context.mounted) return; Scrollable.ensureVisible( context, alignment: 0.5, alignmentPolicy: ScrollPositionAlignmentPolicy.explicit, - duration: Duration.zero, + duration: tv ? motion.tvScroll : Duration.zero, + curve: motion.tvScrollCurve, ); }); } diff --git a/lib/widgets/episodes_panel.dart b/lib/widgets/episodes_panel.dart index cd8c81ef0..9d0883002 100644 --- a/lib/widgets/episodes_panel.dart +++ b/lib/widgets/episodes_panel.dart @@ -17,6 +17,7 @@ import '../utils/platform_util.dart'; import '../utils/episode_progress_merge.dart'; import '../utils/tv_keys.dart'; import 'tv_focus_scroll_wrapper.dart'; +import '../theme/app_motion.dart'; import '../theme/app_theme_scope.dart'; import 'detail/detail_style.dart'; import 'detail/theme/detail_theme.dart'; @@ -2575,12 +2576,17 @@ class _CompactEpisodeRowState extends State<_CompactEpisodeRow> { onFocusChange: (f) { if (mounted) setState(() => _focused = f); if (f && widget.isTelevision && context.mounted) { + // TV: `AppMotion.tvScroll` — the snap under snappy, the profile's + // glide under smooth. This row's own hardcoded 220ms predated the + // motion profile and was never routed through it, unlike the + // sibling `EpisodeTile.onFocusChange`. + final motion = AppMotion.of(context); Scrollable.ensureVisible( context, alignment: 0.5, alignmentPolicy: ScrollPositionAlignmentPolicy.explicit, - duration: const Duration(milliseconds: 220), - curve: Curves.easeOutCubic, + duration: motion.tvScroll, + curve: motion.tvScrollCurve, ); } }, diff --git a/test/detail_layout_rec_scroll_follow_motion_profile_test.dart b/test/detail_layout_rec_scroll_follow_motion_profile_test.dart new file mode 100644 index 000000000..71711aa68 --- /dev/null +++ b/test/detail_layout_rec_scroll_follow_motion_profile_test.dart @@ -0,0 +1,189 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:debrify/models/stremio_addon.dart'; +import 'package:debrify/services/tv_motion_profile.dart'; +import 'package:debrify/utils/platform_util.dart'; +import 'package:debrify/widgets/detail/detail_layout_marquee.dart'; +import 'package:debrify/widgets/detail/detail_model.dart'; +import 'package:debrify/widgets/detail/theme/detail_theme.dart'; +import 'package:debrify/widgets/detail/theme/detail_themes.dart'; + +/// Pins the fix for `DetailMarquee`'s rec-rail scroll-follow (mirrored +/// identically in `DetailConsole`/`DetailDossier` — see +/// `detail_layout_rec_scroll_follow_test.dart` for the shared fixture that +/// proves the scroll itself still lands, unaffected by this change). +/// +/// The rail's `onFocusChange` hardcoded `duration: Duration.zero` — a bare +/// jump — regardless of the TV motion profile. PR #281 (this scroll-follow) +/// landed at 2026-09-07 21:39 and PR #276 (the motion profile) landed six +/// minutes later at 21:44, so #281 simply predates the profile and was never +/// migrated to read it, unlike every other TV scroll-follow site added +/// before #276. +/// +/// Before the fix, both profiles jump instantly (`Duration.zero`), settling +/// in the same one or two `pumpAndSettle` frames. After the fix, snappy +/// keeps the instant jump but smooth now animates over `AppMotion.tvScroll` +/// (260ms), needing measurably more frames to land. +const _tv = Size(960, 540); + +List _recs(int count) => [ + for (var i = 0; i < count; i++) + StremioMeta( + id: 'tt300$i', + imdbId: 'tt300$i', + type: 'movie', + name: 'Rec $i', + poster: null, + background: null, + description: null, + year: '2020', + genres: const [], + ), +]; + +DetailModel _movieModel({required List recs}) { + final item = StremioMeta( + id: 'tt0000002', + imdbId: 'tt0000002', + type: 'movie', + name: 'A Movie', + poster: null, + background: null, + description: null, + year: '2020', + genres: const [], + ); + return DetailModel( + item: item, + isMovie: true, + isTelevision: true, + accent: const Color(0xFFABA124), + imdbExtra: null, + parentsGuide: null, + recommendations: recs, + primaryLabel: 'Play', + sourceCount: 2, + hasTrailer: false, + trailerBusy: false, + trailerPlaying: false, + hasTrakt: false, + traktTracked: false, + traktLabel: 'Watchlist', + traktRating: null, + hasSimkl: false, + simklTracked: false, + simklLabel: 'Watching', + simklRating: null, + showPrimary: true, + onPrimary: () {}, + onBrowse: null, + onTrailer: () {}, + onSelectSource: () {}, + onAppMenu: () {}, + onTraktMenu: () {}, + onSimklMenu: () {}, + onRecommendationTap: (_) {}, + onAmbientStill: (_) {}, + focus: DetailFocusCoordinator( + backNode: FocusNode(debugLabel: 'test-back'), + primaryEntry: FocusNode(debugLabel: 'test-primary'), + ), + ); +} + +Future _pump(WidgetTester tester, Widget child) async { + tester.view.physicalSize = _tv; + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + await tester.pumpWidget( + MediaQuery( + data: const MediaQueryData(size: _tv, devicePixelRatio: 1.0), + child: MaterialApp( + home: Scaffold( + backgroundColor: DetailThemes.signal.ground, + body: DetailThemeScope(theme: DetailThemes.signal, child: child), + ), + ), + ), + ); + await tester.pump(const Duration(milliseconds: 400)); +} + +Future _focusCardAndSettle(WidgetTester tester, Finder list, int index) async { + final cards = find.descendant(of: list, matching: find.byType(InkWell)); + final leaf = find + .descendant(of: cards.at(index), matching: find.byType(ColoredBox)) + .first; + final node = Focus.of(tester.element(leaf), createDependency: false); + node.requestFocus(); + return tester.pumpAndSettle(); +} + +void main() { + setUp(() { + TvMotionController.debugReset(); + // The scroll-follow's own TV gate reads the global platform flag, not + // the fixture's DetailModel.isTelevision — force it on so the fixed + // branch (`tv ? motion.tvScroll : Duration.zero`) actually engages. + PlatformUtil.debugSetAndroidTvCached(true); + }); + + tearDown(() { + TvMotionController.debugReset(); + PlatformUtil.debugSetAndroidTvCached(null); + }); + + // The snappy-profile ceiling: how many `pumpAndSettle` frames the + // unchanged instant jump needs. + const snappyCeilingPumps = 3; + + + testWidgets( + 'Marquee snappy: the rec rail still jumps instantly, within a couple of ' + 'frames', + (tester) async { + final model = _movieModel(recs: _recs(20)); + await _pump(tester, DetailMarquee(model: model, episodesHost: null)); + + final list = find.byType(ListView); + final pumps = await _focusCardAndSettle(tester, list, 8); + + expect( + pumps, + lessThanOrEqualTo(snappyCeilingPumps), + reason: 'snappy keeps the shipped instant jump', + ); + }, + ); + + testWidgets( + 'Marquee smooth: the rec rail now glides on AppMotion.tvScroll instead ' + 'of the old unconditional instant jump', + (tester) async { + TvMotionController.select(TvMotionProfile.smooth); + final model = _movieModel(recs: _recs(20)); + await _pump(tester, DetailMarquee(model: model, episodesHost: null)); + + final list = find.byType(ListView); + final pumps = await _focusCardAndSettle(tester, list, 8); + + // A generous margin above snappyCeilingPumps rather than a bare + // greaterThan: the unconditional pre-fix Duration.zero already varies + // by a pump or two run to run (observed 3-4), so a one-pump margin is + // not a reliable signal. The real 260ms glide this fix adds needs + // several more settle iterations (observed 6) — comfortably clear of + // that noise floor. + expect( + pumps, + greaterThanOrEqualTo(snappyCeilingPumps + 2), + reason: + 'the smooth profile should glide over AppMotion.tvScroll ' + '(260ms), needing measurably more frames than the snappy ' + 'instant jump. A pump count within a pump or two of the snappy ' + 'ceiling means the rail is still on the old unconditional ' + 'Duration.zero.', + ); + }, + ); +} diff --git a/test/theme/tv_motion_profile_scroll_sites_test.dart b/test/theme/tv_motion_profile_scroll_sites_test.dart new file mode 100644 index 000000000..4c0077f7b --- /dev/null +++ b/test/theme/tv_motion_profile_scroll_sites_test.dart @@ -0,0 +1,148 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:debrify/models/stremio_addon.dart'; +import 'package:debrify/screens/search/board_cell.dart'; +import 'package:debrify/services/tv_motion_profile.dart'; + +/// Pins the fix for the board's own DPAD scroll-follow (`_StremioCard` in +/// `board_cell.dart`), which predates the TV motion profile (added +/// 2026-09-05, PR #276 landed 2026-09-07) and hardcoded a flat 140ms glide +/// on every TV regardless of which profile was selected — so choosing +/// "Smooth" in Appearance never touched the single most visible TV scroll +/// interaction, the Home/Discover board. +/// +/// Before the fix both profiles glide at the identical, unconditional 140ms +/// literal, so focusing an off-screen card settles within the same handful +/// of `pumpAndSettle` frames under either profile (observed: 5). After the +/// fix, snappy keeps that 140ms figure — still settling within 5 frames — +/// but smooth resolves to `AppMotion.tvScroll` (260ms) via +/// `AppMotion.scrollTempo`, a longer glide that needs more frames to land +/// (observed: 6). Comparing frame counts rather than sampling a fixed +/// elapsed time avoids coupling this test to exactly when the focus-driven +/// post-frame callback happens to start ticking; each profile runs in its +/// own `testWidgets` so neither test's fake clock leaks into the other's. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + setUp(() { + TvMotionController.debugReset(); + }); + + tearDown(() { + TvMotionController.debugReset(); + }); + + StremioMeta item(int i) => StremioMeta( + id: 'tt200$i', + imdbId: 'tt200$i', + type: 'movie', + name: 'Board $i', + poster: null, + background: null, + description: null, + year: '2020', + genres: const [], + ); + + Future> pumpBoard(WidgetTester tester) async { + final nodes = List.generate(20, (i) => FocusNode(debugLabel: 'board-$i')); + addTearDown(() { + for (final n in nodes) { + n.dispose(); + } + }); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 220, + child: ListView.builder( + scrollDirection: Axis.horizontal, + itemCount: nodes.length, + itemBuilder: (context, i) => SizedBox( + width: 150, + child: BoardCell( + item: item(i), + isTelevision: true, + focusNode: nodes[i], + column: i, + rowNodes: nodes, + hasBoundSource: false, + onFocused: () {}, + onUp: () {}, + onDown: () {}, + onOpen: () {}, + ), + ), + ), + ), + ), + ), + ); + await tester.pump(); + return nodes; + } + + /// Focuses the built-but-partially-off-screen card 5 (inside the default + /// cache extent, but only half inside the 800-wide test viewport, so + /// centring it under alignment 0.5 is a real, non-zero scroll) and settles + /// the resulting glide. + Future<(int pumps, double offset)> focusAndSettle( + WidgetTester tester, + Finder scrollable, + List nodes, + ) async { + nodes[5].requestFocus(); + final pumps = await tester.pumpAndSettle(); + final offset = tester.state(scrollable).position.pixels; + return (pumps, offset); + } + + // The shared ceiling the smooth test compares against — the number of + // `pumpAndSettle` frames the snappy profile's shipped 140ms glide needs. + const snappyCeilingPumps = 5; + + testWidgets( + 'snappy: keeps the shipped 140ms glide, settling within a handful of ' + 'frames', + (tester) async { + final nodes = await pumpBoard(tester); + final scrollable = find.byType(Scrollable); + final (pumps, offset) = await focusAndSettle(tester, scrollable, nodes); + + expect( + offset, + isNot(0.0), + reason: 'the board should have scrolled to reveal the focused card', + ); + expect( + pumps, + lessThanOrEqualTo(snappyCeilingPumps), + reason: 'the snappy profile keeps its shipped 140ms glide', + ); + }, + ); + + testWidgets( + 'smooth: the board glide is the profile figure (260ms), not the old ' + 'flat 140ms — takes strictly more frames to settle than snappy', + (tester) async { + TvMotionController.select(TvMotionProfile.smooth); + final nodes = await pumpBoard(tester); + final scrollable = find.byType(Scrollable); + final (pumps, offset) = await focusAndSettle(tester, scrollable, nodes); + + expect(offset, isNot(0.0)); + expect( + pumps, + greaterThan(snappyCeilingPumps), + reason: + 'the smooth profile (260ms) should need more pumped frames to ' + "settle than snappy's shipped 140ms glide. A flat, " + 'profile-blind 140ms (the pre-fix behaviour) would settle in ' + 'the same handful of frames as snappy.', + ); + }, + ); +}