From 25e5ea042f6e6333d219a4ef290f49aed49f2d28 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 26 Jun 2026 08:34:54 +0200 Subject: [PATCH] fix: server-qualify TvBrowseRail hub keys to avoid duplicate GlobalKey --- lib/widgets/tv_browse_rail.dart | 65 ++++++++++++++++----------- test/widgets/tv_browse_rail_test.dart | 46 ++++++++++++++++++- 2 files changed, 83 insertions(+), 28 deletions(-) diff --git a/lib/widgets/tv_browse_rail.dart b/lib/widgets/tv_browse_rail.dart index 02b0662d..6e40f886 100644 --- a/lib/widgets/tv_browse_rail.dart +++ b/lib/widgets/tv_browse_rail.dart @@ -397,7 +397,7 @@ class TvBrowseRailState extends State { final Map> _mediaCardKeys = {}; final Map _metricsByHub = {}; final Map _scaleByHub = {}; - final Map _lastTrailingByHubId = {}; + final Map _lastTrailingByHubKey = {}; int _hubIndex = 0; int _itemIndex = 0; @@ -490,9 +490,9 @@ class TvBrowseRailState extends State { return; } - final oldActiveHubId = oldWidget.hubs.isEmpty + final oldActiveHubKey = oldWidget.hubs.isEmpty ? null - : oldWidget.hubs[_hubIndex.clamp(0, oldWidget.hubs.length - 1)].id; + : _hubKey(oldWidget.hubs[_hubIndex.clamp(0, oldWidget.hubs.length - 1)]); if (widget.hubs.isEmpty) { _hubIndex = 0; @@ -503,8 +503,8 @@ class TvBrowseRailState extends State { } final selectedInitialHub = _selectInitialHubIfPossible(); - if (!selectedInitialHub && oldActiveHubId != null) { - final preservedIndex = widget.hubs.indexWhere((hub) => hub.id == oldActiveHubId); + if (!selectedInitialHub && oldActiveHubKey != null) { + final preservedIndex = widget.hubs.indexWhere((hub) => _hubKey(hub) == oldActiveHubKey); if (preservedIndex != -1) { _hubIndex = preservedIndex; } else { @@ -521,7 +521,8 @@ class TvBrowseRailState extends State { // notify:false — this runs during the build phase and the enclosing // rebuild already refreshes every selector. _focusPosition.set(_hubIndex, _itemIndex, notify: false); - final activeHubChanged = oldActiveHubId != _activeHub?.id; + final newActiveHub = _activeHub; + final activeHubChanged = oldActiveHubKey != (newActiveHub == null ? null : _hubKey(newActiveHub)); final activeHubStateChanged = _hubStateChanged(oldWidget.hubs, widget.hubs, _hubIndex) || (_activeHub != null && _trailingStateChanged(_activeHub!)); @@ -554,7 +555,9 @@ class TvBrowseRailState extends State { if (index < 0 || index >= oldHubs.length || index >= newHubs.length) return true; final oldHub = oldHubs[index]; final newHub = newHubs[index]; - if (oldHub.id != newHub.id || oldHub.more != newHub.more || oldHub.items.length != newHub.items.length) { + if (_hubKey(oldHub) != _hubKey(newHub) || + oldHub.more != newHub.more || + oldHub.items.length != newHub.items.length) { return true; } for (var j = 0; j < oldHub.items.length; j++) { @@ -571,14 +574,14 @@ class TvBrowseRailState extends State { } bool _trailingStateChanged(MediaHub hub) { - final previous = _lastTrailingByHubId[hub.id]; + final previous = _lastTrailingByHubKey[_hubKey(hub)]; return previous != null && previous != _trailingFor(hub); } void _rememberTrailingStates() { - _lastTrailingByHubId + _lastTrailingByHubKey ..clear() - ..addEntries(widget.hubs.map((hub) => MapEntry(hub.id, _trailingFor(hub)))); + ..addEntries(widget.hubs.map((hub) => MapEntry(_hubKey(hub), _trailingFor(hub)))); } @override @@ -649,6 +652,8 @@ class TvBrowseRailState extends State { bool _selectInitialHubIfPossible() { final initialHubId = widget.initialHubId; if (_hasUserChangedHub || initialHubId == null || widget.hubs.isEmpty) return false; + // External contract: `initialHubId` is a bare `hub.id` supplied by the + // single-server media-detail caller, so match on `hub.id` (not `_hubKey`). final initialIndex = widget.hubs.indexWhere((hub) => hub.id == initialHubId); if (initialIndex == -1) return false; if (initialIndex != _hubIndex) { @@ -770,7 +775,7 @@ class TvBrowseRailState extends State { final currentHub = _activeHub; if (currentHub != null) _rememberFocus(currentHub); final nextHub = widget.hubs[next]; - final remembered = HubFocusMemory.getForHubOnly(nextHub.id, _totalItemCount(nextHub)); + final remembered = HubFocusMemory.getForHubOnly(_hubKey(nextHub), _totalItemCount(nextHub)); setState(() { _hubIndex = next; _itemIndex = remembered.clamp(0, _totalItemCount(nextHub) == 0 ? 0 : _totalItemCount(nextHub) - 1); @@ -818,7 +823,10 @@ class TvBrowseRailState extends State { } void _setHoveredItem(MediaHub hub, int index) { - if (_activeHub?.id != hub.id || index >= hub.items.length || _itemIndex == index) return; + final active = _activeHub; + if (active == null || _hubKey(active) != _hubKey(hub) || index >= hub.items.length || _itemIndex == index) { + return; + } _itemIndex = index; _hasUserChangedItem = true; _focusPosition.set(_hubIndex, _itemIndex); @@ -849,18 +857,18 @@ class TvBrowseRailState extends State { } void _rememberFocus(MediaHub hub) { - HubFocusMemory.setForHub(hub.id, _itemIndex); + HubFocusMemory.setForHub(_hubKey(hub), _itemIndex); } void _scrollToItem({bool animate = true, Duration duration = _navigationScrollDuration}) { final hub = _activeHub; if (hub == null) return; - final controller = _scrollControllers[hub.id]; + final controller = _scrollControllers[_hubKey(hub)]; if (controller == null) return; if (controller.positions.length != 1) return; - final metrics = _metricsByHub[hub.id]; + final metrics = _metricsByHub[_hubKey(hub)]; if (metrics == null) return; - final scale = _scaleByHub[hub.id] ?? 1.0; + final scale = _scaleByHub[_hubKey(hub)] ?? 1.0; final position = controller.position; final viewportWidth = position.viewportDimension; final maxScrollExtent = position.maxScrollExtent; @@ -898,7 +906,7 @@ class TvBrowseRailState extends State { double scale, int initialItemIndex, ) { - return _scrollControllers.putIfAbsent(hub.id, () { + return _scrollControllers.putIfAbsent(_hubKey(hub), () { final maxScrollExtent = TvBrowseRailLayout.estimatedMaxScrollExtent( hub: hub, metrics: metrics, @@ -919,8 +927,14 @@ class TvBrowseRailState extends State { }); } + /// Stable, collision-free per-hub key. `hub.id` (the backend hub key) is only + /// unique within one server; Discover aggregates hubs from several servers, so + /// prefix the server id to keep two same-id hubs from sharing rail state + /// (scroll position, metrics, card GlobalKeys, focus memory). + String _hubKey(MediaHub hub) => '${hub.serverId ?? ''}:${hub.id}'; + GlobalKey _cardKeyFor(MediaHub hub, int itemIndex) { - return _mediaCardKeys.putIfAbsent('${hub.id}:$itemIndex', () => GlobalKey()); + return _mediaCardKeys.putIfAbsent('${_hubKey(hub)}:$itemIndex', () => GlobalKey()); } bool _isContinueWatchingHub(MediaHub hub) => widget.isContinueWatchingHub?.call(hub) ?? false; @@ -1182,12 +1196,9 @@ class TvBrowseRailState extends State { final iconColor = isActive ? colorScheme.onSurface : colorScheme.onSurface.withValues(alpha: 0.42); final showServerName = widget.showServerName && hub.serverName != null; final serverColor = colorScheme.primary.withValues(alpha: isActive ? 0.7 : 0.4); - final serverStyle = Theme.of(context).textTheme.titleMedium?.copyWith( - color: serverColor, - fontSize: 15 * scale, - height: 1, - fontWeight: FontWeight.w700, - ); + final serverStyle = Theme.of( + context, + ).textTheme.titleMedium?.copyWith(color: serverColor, fontSize: 15 * scale, height: 1, fontWeight: FontWeight.w700); return SizedBox( height: TvBrowseRailLayout.hubStripHeightForScale(scale), @@ -1251,11 +1262,11 @@ class TvBrowseRailState extends State { }) { final isActiveHub = hubIndex == _hubIndex; final totalCount = _totalItemCount(hub); - final inactiveIndex = HubFocusMemory.getForHubOnly(hub.id, totalCount); + final inactiveIndex = HubFocusMemory.getForHubOnly(_hubKey(hub), totalCount); final focusedIndex = isActiveHub ? _itemIndex : inactiveIndex; final scrollController = _scrollControllerForHub(hub, metrics, railViewportWidth, scale, focusedIndex); - _metricsByHub[hub.id] = metrics; - _scaleByHub[hub.id] = scale; + _metricsByHub[_hubKey(hub)] = metrics; + _scaleByHub[_hubKey(hub)] = scale; return Transform.translate( offset: Offset(-interactionExpansion, 0), diff --git a/test/widgets/tv_browse_rail_test.dart b/test/widgets/tv_browse_rail_test.dart index 95dee89a..2e79bdb6 100644 --- a/test/widgets/tv_browse_rail_test.dart +++ b/test/widgets/tv_browse_rail_test.dart @@ -350,6 +350,47 @@ void main() { expect(headerText.style?.color, theme.colorScheme.onSurface); }); + testWidgets('two hubs sharing a backend id across servers do not collide on card GlobalKeys', (tester) async { + final serverManager = MultiServerManager(); + // Multi-server aggregation: two servers return a hub with the SAME backend + // id ('recently_added'). Per-card GlobalKeys must be qualified by serverId, + // or both rows hand the same GlobalKey to different cards + // and finalizeTree throws "Duplicate GlobalKey". + MediaHub hubFor(String serverId) => MediaHub( + id: 'recently_added', + title: 'Recently Added ($serverId)', + type: 'movie', + serverId: serverId, + size: 2, + items: [ + MediaItem(id: 'movie_1', backend: MediaBackend.plex, kind: MediaKind.movie, title: 'A', serverId: serverId), + MediaItem(id: 'movie_2', backend: MediaBackend.plex, kind: MediaKind.movie, title: 'B', serverId: serverId), + ], + ); + + await tester.pumpWidget( + ChangeNotifierProvider( + create: (_) => MultiServerProvider(serverManager, DataAggregationService(serverManager)), + child: MaterialApp( + theme: monoTheme(dark: true), + home: Scaffold( + body: SizedBox( + width: 1280, + height: 720, + child: TvBrowseRail( + hubs: [hubFor('serverA'), hubFor('serverB')], + iconForHub: (_, _) => Icons.movie_rounded, + ), + ), + ), + ), + ), + ); + await tester.pump(); + + expect(tester.takeException(), isNull); + }); + testWidgets('full card layout hides media text and overlays actor text when enabled', (tester) async { await SettingsService.instanceOrNull!.write(SettingsService.tvFullCardLayout, true); @@ -986,7 +1027,10 @@ void main() { final serverManager = MultiServerManager(); final activeHubIds = []; var parentRebuilds = 0; - HubFocusMemory.setForHub(episodeHub.id, 5); + // Seed remembered focus under the rail's server-qualified hub key (mirrors + // _TvBrowseRailState._hubKey: ':'), so the multi-server keying + // resolves it the same way the rail does. + HubFocusMemory.setForHub('${episodeHub.serverId ?? ''}:${episodeHub.id}', 5); await tester.pumpWidget( ChangeNotifierProvider(