yorickr/plezy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(profiles): keep the profile picker highlighted while its list sorts

Browse Source 
The picker resolved StorageService asynchronously and rebuilt its profiles
stream once it landed. Storage is what supplies profile recency, so the second
view arrived re-sorted a microtask after first paint. The sliver children
carried no keys, so that reorder handed each tile's Element the next profile's
focus node; detaching the old node dropped primary focus onto the enclosing
scope and took the D-pad highlight with it. The launch picker has no back
route on tvOS, so a user who can no longer see or move the selection has
nothing useful left to press.

Read StorageService from the provider graph, where it is already resolved
before any route exists, so the stream is built once and the first painted
frame is already recency-sorted. Key the tiles and add findChildIndexCallback
so a later re-sort from a refreshed profile source moves a tile instead of
destroying it: without the lookup the sliver re-inflates the tile, which keeps
primary focus but resets FocusableWrapper's chrome to unfocused.

close #1792
This commit is contained in:
edde746
2026-08-05 06:09:26 +02:00
parent d7c4ea0a8b
commit f36e20bcad
2 changed files with 288 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/screens/profile/profile_switch_screen.dart
+84 -73
View File
@@ -55,32 +55,27 @@ class _ProfileSwitchScreenState extends State<ProfileSwitchScreen> with MountedS
bool _focusRequested = false;
bool _switching = false;
Stream<ProfilesView>? _viewStream;
StorageService? _viewStreamStorage;
StorageService? _storage;
Future<StorageService>? _storageFuture;
@override
void didChangeDependencies() {
super.didChangeDependencies();
_ensureViewStream();
if (_storage == null) {
unawaited(
(_storageFuture ??= StorageService.getInstance()).then((s) {
setStateIfMounted(() => _storage = s);
}),
);
}
}
/// Built exactly once. Resolving [StorageService] asynchronously here used
/// to rebuild the stream a microtask after first paint, and because storage
/// is what supplies profile recency the second view arrived re-sorted — the
/// reorder then rebound the tiles' focus nodes and dropped the D-pad
/// highlight onto the enclosing scope (#1792). Storage is already resolved
/// before any route exists, so read it from the provider graph instead.
void _ensureViewStream() {
if (_viewStream != null && identical(_viewStreamStorage, _storage)) return;
_viewStreamStorage = _storage;
if (_viewStream != null) return;
_viewStream = watchProfilesView(
profiles: context.read<ProfileRegistry>(),
profileConnections: context.read<ProfileConnectionRegistry>(),
connections: context.read<ConnectionRegistry>(),
plexHome: context.read<PlexHomeService>(),
storage: _storage,
storage: context.read<StorageService>(),
);
}
@@ -220,71 +215,87 @@ class _ProfileSwitchScreenState extends State<ProfileSwitchScreen> with MountedS
}
SliverList _profileList(List<Profile> profiles, ProfilesView view, String? activeId, {required bool autofocusFirst}) {
// The tile keys and `findChildIndexCallback` below are one mechanism, not
// two independent safeguards. A refreshed profile source can re-sort this
// list after first paint; the key stops the sliver handing a tile's
// Element the next profile's focus node, and the lookup lets it map that
// key to its new index. Without the lookup the tile is destroyed and
// re-inflated instead of moved, which keeps primary focus but resets
// FocusableWrapper's chrome — a focused tile with no highlight (#1792).
return SliverList(
delegate: SliverChildBuilderDelegate((context, index) {
final profile = profiles[index];
final isActive = profile.id == activeId;
final tokensRef = tokens(context);
final tileRadii = groupItemRadii(context, index, profiles.length);
final isFirstSelectable = autofocusFirst && index == 0;
final profileFocusNode = _profileFocusNode(profile);
final menuFocusNode = _profileMenuFocusNode(profile);
final menuKey = _profileMenuKey(profile);
// All tile actions are disabled while a switch is binding: the
// overlay's barrier blocks pointers but not DPAD key events, and a
// Manage/Delete flow racing the in-flight switch corrupts state
// (e.g. a delete confirmation left open when the switch settles).
final actionsEnabled = !widget.requireSelection && !_switching;
final onManage = actionsEnabled ? () => _manageProfile(profile) : null;
final onDelete = profile.isLocal && actionsEnabled ? () => _deleteProfile(profile) : null;
final onSignOut = profile.isPlexHome && profile.parentConnectionId != null && actionsEnabled
? () => _signOutPlexAccount(profile)
: null;
final hasMenu = onManage != null || onDelete != null || onSignOut != null;
delegate: SliverChildBuilderDelegate(
(context, index) {
final profile = profiles[index];
final isActive = profile.id == activeId;
final tokensRef = tokens(context);
final tileRadii = groupItemRadii(context, index, profiles.length);
final isFirstSelectable = autofocusFirst && index == 0;
final profileFocusNode = _profileFocusNode(profile);
final menuFocusNode = _profileMenuFocusNode(profile);
final menuKey = _profileMenuKey(profile);
// All tile actions are disabled while a switch is binding: the
// overlay's barrier blocks pointers but not DPAD key events, and a
// Manage/Delete flow racing the in-flight switch corrupts state
// (e.g. a delete confirmation left open when the switch settles).
final actionsEnabled = !widget.requireSelection && !_switching;
final onManage = actionsEnabled ? () => _manageProfile(profile) : null;
final onDelete = profile.isLocal && actionsEnabled ? () => _deleteProfile(profile) : null;
final onSignOut = profile.isPlexHome && profile.parentConnectionId != null && actionsEnabled
? () => _signOutPlexAccount(profile)
: null;
final hasMenu = onManage != null || onDelete != null || onSignOut != null;
if (isFirstSelectable && !_focusRequested) {
_focusRequested = true;
WidgetsBinding.instance.addPostFrameCallback((_) {
if (mounted) profileFocusNode.requestFocus();
});
}
if (isFirstSelectable && !_focusRequested) {
_focusRequested = true;
WidgetsBinding.instance.addPostFrameCallback((_) {
if (mounted) profileFocusNode.requestFocus();
});
}
return Padding(
padding: EdgeInsets.fromLTRB(16, index == 0 ? 4 : tokensRef.groupGap, 16, 0),
child: FocusableWrapper(
autofocus: isFirstSelectable,
focusNode: profileFocusNode,
disableScale: true,
borderRadii: tileRadii,
enableLongPress: hasMenu,
onLongPress: hasMenu ? () => _openProfileMenu(profile) : null,
onNavigateRight: hasMenu ? () => menuFocusNode.requestFocus() : null,
onSelect: _switching || (isActive && !widget.requireSelection) ? null : () => _switchTo(profile),
child: Card(
shape: RoundedRectangleBorder(borderRadius: tileRadii),
clipBehavior: Clip.antiAlias,
child: _ProfileTile(
borderRadius: tileRadii,
profile: profile,
avatarUrl: view.avatarUrlByProfile[profile.id],
isActive: isActive && !widget.requireSelection,
chips: _chipsFor(profile, view),
onTap: () => _switchTo(profile),
onLongPress: hasMenu ? () => _openProfileMenu(profile) : null,
// Manage available for any profile — adding/removing
// borrowed connections is supported on plex_home too. Delete
// stays local-only (Plex Home users are owned by Plex).
onManage: onManage,
onDelete: onDelete,
onSignOut: onSignOut,
menuFocusNode: menuFocusNode,
menuKey: menuKey,
onMenuNavigateLeft: () => profileFocusNode.requestFocus(),
return Padding(
key: ValueKey(profile.id),
padding: EdgeInsets.fromLTRB(16, index == 0 ? 4 : tokensRef.groupGap, 16, 0),
child: FocusableWrapper(
autofocus: isFirstSelectable,
focusNode: profileFocusNode,
disableScale: true,
borderRadii: tileRadii,
enableLongPress: hasMenu,
onLongPress: hasMenu ? () => _openProfileMenu(profile) : null,
onNavigateRight: hasMenu ? () => menuFocusNode.requestFocus() : null,
onSelect: _switching || (isActive && !widget.requireSelection) ? null : () => _switchTo(profile),
child: Card(
shape: RoundedRectangleBorder(borderRadius: tileRadii),
clipBehavior: Clip.antiAlias,
child: _ProfileTile(
borderRadius: tileRadii,
profile: profile,
avatarUrl: view.avatarUrlByProfile[profile.id],
isActive: isActive && !widget.requireSelection,
chips: _chipsFor(profile, view),
onTap: () => _switchTo(profile),
onLongPress: hasMenu ? () => _openProfileMenu(profile) : null,
// Manage available for any profile — adding/removing
// borrowed connections is supported on plex_home too. Delete
// stays local-only (Plex Home users are owned by Plex).
onManage: onManage,
onDelete: onDelete,
onSignOut: onSignOut,
menuFocusNode: menuFocusNode,
menuKey: menuKey,
onMenuNavigateLeft: () => profileFocusNode.requestFocus(),
),
),
),
),
);
}, childCount: profiles.length),
);
},
childCount: profiles.length,
findChildIndexCallback: (key) {
final id = (key as ValueKey<String>).value;
final index = profiles.indexWhere((profile) => profile.id == id);
return index < 0 ? null : index;
},
),
);
}
test/screens/profile/profile_switch_screen_test.dart
+204
View File
@@ -1,3 +1,5 @@
import 'dart:async';
import 'package:drift/native.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
@@ -5,6 +7,8 @@ import 'package:flutter_test/flutter_test.dart';
import 'package:plezy/connection/connection.dart';
import 'package:plezy/connection/connection_registry.dart';
import 'package:plezy/database/app_database.dart';
import 'package:plezy/focus/focusable_wrapper.dart';
import 'package:plezy/focus/input_mode_tracker.dart';
import 'package:plezy/i18n/strings.g.dart';
import 'package:plezy/profiles/active_profile_provider.dart';
import 'package:plezy/profiles/plex_home_service.dart';
@@ -16,6 +20,7 @@ import 'package:plezy/profiles/profile_registry.dart';
import 'package:plezy/screens/profile/profile_switch_screen.dart';
import 'package:plezy/services/storage_service.dart';
import 'package:plezy/theme/mono_theme.dart';
import 'package:plezy/utils/platform_detector.dart';
import 'package:provider/provider.dart';
import '../../test_helpers/prefs.dart';
@@ -58,6 +63,7 @@ void main() {
TranslationProvider(
child: MultiProvider(
providers: [
Provider<StorageService>.value(value: storage),
Provider<ProfileRegistry>.value(value: profiles),
Provider<ProfileConnectionRegistry>.value(value: profileConnections),
Provider<ConnectionRegistry>.value(value: connections),
@@ -121,6 +127,7 @@ void main() {
TranslationProvider(
child: MultiProvider(
providers: [
Provider<StorageService>.value(value: storage),
Provider<ProfileRegistry>.value(value: profiles),
Provider<ProfileConnectionRegistry>.value(value: profileConnections),
Provider<ConnectionRegistry>.value(value: connections),
@@ -188,6 +195,7 @@ void main() {
TranslationProvider(
child: MultiProvider(
providers: [
Provider<StorageService>.value(value: storage),
Provider<ProfileRegistry>.value(value: profiles),
Provider<ProfileConnectionRegistry>.value(value: profileConnections),
Provider<ConnectionRegistry>.value(value: connections),
@@ -216,6 +224,177 @@ void main() {
await tester.pumpWidget(const SizedBox.shrink());
await tester.pump();
});
testWidgets('paints the recency order on the first frame that shows profiles', (tester) async {
final db = AppDatabase.forTesting(NativeDatabase.memory());
final profiles = _FakeProfileRegistry(db, [
Profile.local(id: 'local-owner', displayName: 'Owner', createdAt: DateTime(2026, 1, 1)),
Profile.local(id: 'local-kids', displayName: 'Kids', createdAt: DateTime(2026, 1, 2)),
]);
final connections = _FakeConnectionRegistry(db);
final profileConnections = _FakeProfileConnectionRegistry(db);
final storage = await StorageService.getInstance();
await storage.markProfileUsed('local-kids', DateTime(2026, 1, 3));
final plexHome = PlexHomeService(
connections: connections,
profileConnections: profileConnections,
storage: storage,
plexHomeUserFetcher: (_) async => const [],
);
final activeProfile = ActiveProfileProvider(
registry: profiles,
plexHome: plexHome,
connections: connections,
profileConnections: profileConnections,
storage: storage,
);
addTearDown(() async {
activeProfile.dispose();
await plexHome.dispose();
await db.close();
});
await tester.pumpWidget(
TranslationProvider(
child: MultiProvider(
providers: [
Provider<StorageService>.value(value: storage),
Provider<ProfileRegistry>.value(value: profiles),
Provider<ProfileConnectionRegistry>.value(value: profileConnections),
Provider<ConnectionRegistry>.value(value: connections),
Provider<PlexHomeService>.value(value: plexHome),
ChangeNotifierProvider<ActiveProfileProvider>.value(value: activeProfile),
],
child: MaterialApp(theme: monoTheme(dark: true), home: const ProfileSwitchScreen()),
),
),
);
// Frame-by-frame, not pumpAndSettle: the regression was an intermediate
// order that only existed for one frame, which a settled assertion cannot
// see (#1792).
final paintedOrders = <String>[];
for (var frame = 0; frame < 6; frame++) {
await tester.pump(const Duration(milliseconds: 16));
final order = _visibleProfileNames(tester);
if (order.isNotEmpty) paintedOrders.add(order.join(','));
}
expect(paintedOrders, isNotEmpty, reason: 'the picker never painted a profile');
expect(paintedOrders.toSet(), {'Kids,Owner'}, reason: 'the recency sort must not arrive a frame late');
expect(FocusManager.instance.primaryFocus?.debugLabel, 'ProfileTile:local-kids');
await tester.pumpWidget(const SizedBox.shrink());
await tester.pump();
});
testWidgets('keeps the focused tile highlighted when the list re-sorts after first paint', (tester) async {
TvDetectionService.debugSetAppleTVOverride(true);
PlatformDetector.debugSetIsDesktopOSOverride(false);
addTearDown(() {
TvDetectionService.debugSetAppleTVOverride(null);
PlatformDetector.debugSetIsDesktopOSOverride(null);
});
final db = AppDatabase.forTesting(NativeDatabase.memory());
final owner = Profile.local(id: 'local-owner', displayName: 'Owner', createdAt: DateTime(2026, 1, 1));
final kids = Profile.local(id: 'local-kids', displayName: 'Kids', createdAt: DateTime(2026, 1, 2));
final profiles = _MutableProfileRegistry(db, [owner, kids]);
final connections = _FakeConnectionRegistry(db);
final profileConnections = _FakeProfileConnectionRegistry(db);
final storage = await StorageService.getInstance();
final plexHome = PlexHomeService(
connections: connections,
profileConnections: profileConnections,
storage: storage,
plexHomeUserFetcher: (_) async => const [],
);
final activeProfile = ActiveProfileProvider(
registry: profiles,
plexHome: plexHome,
connections: connections,
profileConnections: profileConnections,
storage: storage,
);
addTearDown(() async {
activeProfile.dispose();
await plexHome.dispose();
await profiles.close();
await db.close();
});
await tester.pumpWidget(
InputModeTracker(
child: TranslationProvider(
child: MultiProvider(
providers: [
Provider<StorageService>.value(value: storage),
Provider<ProfileRegistry>.value(value: profiles),
Provider<ProfileConnectionRegistry>.value(value: profileConnections),
Provider<ConnectionRegistry>.value(value: connections),
Provider<PlexHomeService>.value(value: plexHome),
ChangeNotifierProvider<ActiveProfileProvider>.value(value: activeProfile),
],
child: MaterialApp(theme: monoTheme(dark: true), home: const ProfileSwitchScreen()),
),
),
),
);
await tester.pumpAndSettle();
expect(_visibleProfileNames(tester), ['Owner', 'Kids']);
final focused = FocusManager.instance.primaryFocus;
expect(focused?.debugLabel, 'ProfileTile:local-owner');
expect(_tileIsHighlighted(tester, 'Owner'), isTrue, reason: 'the focused tile starts highlighted');
// A refreshed profile source can re-sort the list after first paint. The
// tile must move with its focus node instead of the node being rebound to
// whatever profile now occupies its index.
profiles.emit([
owner,
Profile.local(
id: 'local-kids',
displayName: 'Kids',
createdAt: DateTime(2026, 1, 2),
lastUsedAt: DateTime(2026, 1, 3),
),
]);
await tester.pumpAndSettle();
expect(_visibleProfileNames(tester), ['Kids', 'Owner'], reason: 'the emission should have re-sorted the list');
expect(FocusManager.instance.primaryFocus, same(focused), reason: 'the reorder must not move focus off the tile');
expect(_tileIsHighlighted(tester, 'Owner'), isTrue, reason: 'the focused tile must still draw focus chrome');
await tester.pumpWidget(const SizedBox.shrink());
await tester.pump();
});
}
/// Tile labels in painted order. Only the two names the reorder tests seed are
/// considered, so surrounding chrome text cannot pollute the sequence.
List<String> _visibleProfileNames(WidgetTester tester) {
return tester
.widgetList<Text>(find.byType(Text))
.map((text) => text.data)
.where((label) => label == 'Owner' || label == 'Kids')
.cast<String>()
.toList();
}
/// Whether the tile showing [name] draws its focus border.
///
/// [FocusableWrapper] paints an opaque border only while it believes it holds
/// focus, so this catches the state where primary focus is correct but the
/// wrapper's chrome was reset by a rebuilt element.
bool _tileIsHighlighted(WidgetTester tester, String name) {
final wrapper = find.ancestor(of: find.text(name), matching: find.byType(FocusableWrapper));
final containers = tester.widgetList<AnimatedContainer>(
find.descendant(of: wrapper, matching: find.byType(AnimatedContainer)),
);
return containers.any((container) {
final border = (container.decoration as BoxDecoration?)?.border?.top;
return border != null && border.style != BorderStyle.none && border.color.a > 0;
});
}
class _FakeProfileRegistry extends ProfileRegistry {
@@ -230,6 +409,31 @@ class _FakeProfileRegistry extends ProfileRegistry {
Future<List<Profile>> list() async => _profiles;
}
/// Profile registry whose stream keeps emitting, so a test can re-sort the
/// list after first paint the way a refreshed profile source does.
class _MutableProfileRegistry extends ProfileRegistry {
_MutableProfileRegistry(super.db, this._profiles);
List<Profile> _profiles;
final StreamController<List<Profile>> _controller = StreamController<List<Profile>>.broadcast();
@override
Stream<List<Profile>> watchProfiles() async* {
yield _profiles;
yield* _controller.stream;
}
@override
Future<List<Profile>> list() async => _profiles;
void emit(List<Profile> profiles) {
_profiles = profiles;
_controller.add(profiles);
}
Future<void> close() => _controller.close();
}
class _FakeConnectionRegistry extends ConnectionRegistry {
final List<Connection> _connections;