fix(settings): dismiss Manage Libraries without leaving Settings
On phone layouts main_screen pushes SettingsScreen as its own route, and that route carried no OverlaySheetHost. showAdaptive could not find one from the tile's context, so Manage Libraries fell back to showModalBottomSheet. The sheet also owns a focused Back handler, so a single Android Back arrived twice — once as a key event, once as popRoute — and the two route-based paths raced, tearing down Settings along with the sheet. Installs one route-local host when no enclosing host exists, and opens the sheet from a context below it. OverlaySheetHost then holds the route while a sheet is open and deduplicates the key path, so one Back closes only the sheet.
This commit is contained in:
@@ -39,6 +39,7 @@ import '../../widgets/desktop_app_bar.dart';
|
|||||||
import '../../widgets/dialog_action_button.dart';
|
import '../../widgets/dialog_action_button.dart';
|
||||||
import '../../widgets/focusable_list_tile.dart';
|
import '../../widgets/focusable_list_tile.dart';
|
||||||
import '../../widgets/library_management_sheet.dart';
|
import '../../widgets/library_management_sheet.dart';
|
||||||
|
import '../../widgets/overlay_sheet.dart';
|
||||||
import '../../widgets/setting_tile.dart';
|
import '../../widgets/setting_tile.dart';
|
||||||
import '../../widgets/settings_builder.dart';
|
import '../../widgets/settings_builder.dart';
|
||||||
import '../../widgets/settings_section.dart';
|
import '../../widgets/settings_section.dart';
|
||||||
@@ -159,6 +160,21 @@ class _SettingsScreenState extends State<SettingsScreen> with FocusableTab, Moun
|
|||||||
Widget build(BuildContext context) {
|
Widget build(BuildContext context) {
|
||||||
final hasLibraries = context.select<LibrariesProvider, bool>((p) => p.libraries.isNotEmpty);
|
final hasLibraries = context.select<LibrariesProvider, bool>((p) => p.libraries.isNotEmpty);
|
||||||
|
|
||||||
|
if (OverlaySheetController.maybeOf(context) != null) {
|
||||||
|
return _buildContent(context, hasLibraries: hasLibraries);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Settings is hosted by MainScreen on side-navigation layouts, but it is a
|
||||||
|
// separate pushed route on phones. Add a route-local host only for the
|
||||||
|
// latter, and build its content from below the host so adaptive sheets do
|
||||||
|
// not fall back to modal routes with a competing Android back path.
|
||||||
|
return OverlaySheetHost(
|
||||||
|
canPop: true,
|
||||||
|
child: Builder(builder: (hostContext) => _buildContent(hostContext, hasLibraries: hasLibraries)),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
Widget _buildContent(BuildContext sheetContext, {required bool hasLibraries}) {
|
||||||
return Scaffold(
|
return Scaffold(
|
||||||
body: Focus(
|
body: Focus(
|
||||||
onKeyEvent: _handleKeyEvent,
|
onKeyEvent: _handleKeyEvent,
|
||||||
@@ -174,12 +190,12 @@ class _SettingsScreenState extends State<SettingsScreen> with FocusableTab, Moun
|
|||||||
if (DonationService.isEnabled) _buildDonateTile(),
|
if (DonationService.isEnabled) _buildDonateTile(),
|
||||||
_buildAppearanceTile(),
|
_buildAppearanceTile(),
|
||||||
_buildPlaybackTile(),
|
_buildPlaybackTile(),
|
||||||
if (hasLibraries) _buildManageLibrariesTile(),
|
if (hasLibraries) _buildManageLibrariesTile(sheetContext),
|
||||||
_buildServicesTile(),
|
_buildServicesTile(),
|
||||||
],
|
],
|
||||||
),
|
),
|
||||||
|
|
||||||
_buildConnectionsSection(),
|
_buildConnectionsSection(sheetContext),
|
||||||
|
|
||||||
if (!PlatformDetector.isAppleTV()) _buildDownloadsSection(),
|
if (!PlatformDetector.isAppleTV()) _buildDownloadsSection(),
|
||||||
|
|
||||||
@@ -258,7 +274,7 @@ class _SettingsScreenState extends State<SettingsScreen> with FocusableTab, Moun
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
Widget _buildManageLibrariesTile() {
|
Widget _buildManageLibrariesTile(BuildContext context) {
|
||||||
return SettingNavigationTile(
|
return SettingNavigationTile(
|
||||||
focusNode: _focusTracker.get(_kManageLibraries),
|
focusNode: _focusTracker.get(_kManageLibraries),
|
||||||
icon: Symbols.video_library_rounded,
|
icon: Symbols.video_library_rounded,
|
||||||
@@ -289,7 +305,7 @@ class _SettingsScreenState extends State<SettingsScreen> with FocusableTab, Moun
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
Widget _buildConnectionsSection() {
|
Widget _buildConnectionsSection(BuildContext context) {
|
||||||
final active = context.select<ActiveProfileProvider, Profile?>((p) => p.active);
|
final active = context.select<ActiveProfileProvider, Profile?>((p) => p.active);
|
||||||
final subtitle = active == null
|
final subtitle = active == null
|
||||||
? t.connections.addConnectionSubtitleNoProfile
|
? t.connections.addConnectionSubtitleNoProfile
|
||||||
@@ -311,12 +327,12 @@ class _SettingsScreenState extends State<SettingsScreen> with FocusableTab, Moun
|
|||||||
Navigator.push(context, MaterialPageRoute(builder: (_) => AddConnectionScreen(targetProfile: active)));
|
Navigator.push(context, MaterialPageRoute(builder: (_) => AddConnectionScreen(targetProfile: active)));
|
||||||
},
|
},
|
||||||
),
|
),
|
||||||
_buildProfilesTile(),
|
_buildProfilesTile(context),
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
Widget _buildProfilesTile() {
|
Widget _buildProfilesTile(BuildContext context) {
|
||||||
// ActiveProfileProvider already merges local rows with virtual Plex
|
// ActiveProfileProvider already merges local rows with virtual Plex
|
||||||
// Home profiles — counting only the local DB rows made every Plex Home
|
// Home profiles — counting only the local DB rows made every Plex Home
|
||||||
// household read as a single profile here. `context.select` keeps
|
// household read as a single profile here. `context.select` keeps
|
||||||
|
|||||||
@@ -87,10 +87,9 @@ Future<void> showLibraryManagementSheet(
|
|||||||
if (PlatformDetector.isTV()) {
|
if (PlatformDetector.isTV()) {
|
||||||
return showScopedDialog<void>(context: context, builder: (context) => buildSheet(isDialog: true));
|
return showScopedDialog<void>(context: context, builder: (context) => buildSheet(isDialog: true));
|
||||||
}
|
}
|
||||||
// showAdaptive rather than of(context).show: on mobile, settings is a pushed
|
// Use the host supplied by the calling screen when available while keeping
|
||||||
// route outside the OverlaySheetHost subtree, so only the fallback path is
|
// this reusable entry point safe for routes without one. isScrollControlled
|
||||||
// available there. isScrollControlled keeps the fallback modal from capping
|
// keeps that modal fallback from capping the sheet at ~9/16 of the screen.
|
||||||
// the sheet at ~9/16 of the screen.
|
|
||||||
return OverlaySheetController.showAdaptive<void>(
|
return OverlaySheetController.showAdaptive<void>(
|
||||||
context,
|
context,
|
||||||
showDragHandle: true,
|
showDragHandle: true,
|
||||||
|
|||||||
@@ -10,10 +10,14 @@ import 'package:path_provider_platform_interface/path_provider_platform_interfac
|
|||||||
import 'package:plezy/connection/connection_registry.dart';
|
import 'package:plezy/connection/connection_registry.dart';
|
||||||
import 'package:plezy/database/app_database.dart';
|
import 'package:plezy/database/app_database.dart';
|
||||||
import 'package:plezy/i18n/strings.g.dart';
|
import 'package:plezy/i18n/strings.g.dart';
|
||||||
|
import 'package:plezy/media/media_backend.dart';
|
||||||
|
import 'package:plezy/media/media_kind.dart';
|
||||||
|
import 'package:plezy/media/media_library.dart';
|
||||||
import 'package:plezy/profiles/active_profile_provider.dart';
|
import 'package:plezy/profiles/active_profile_provider.dart';
|
||||||
import 'package:plezy/profiles/plex_home_service.dart';
|
import 'package:plezy/profiles/plex_home_service.dart';
|
||||||
import 'package:plezy/profiles/profile_connection_registry.dart';
|
import 'package:plezy/profiles/profile_connection_registry.dart';
|
||||||
import 'package:plezy/profiles/profile_registry.dart';
|
import 'package:plezy/profiles/profile_registry.dart';
|
||||||
|
import 'package:plezy/providers/hidden_libraries_provider.dart';
|
||||||
import 'package:plezy/providers/libraries_provider.dart';
|
import 'package:plezy/providers/libraries_provider.dart';
|
||||||
import 'package:plezy/providers/download_provider.dart';
|
import 'package:plezy/providers/download_provider.dart';
|
||||||
import 'package:plezy/providers/seerr_account_provider.dart';
|
import 'package:plezy/providers/seerr_account_provider.dart';
|
||||||
@@ -28,6 +32,7 @@ import 'package:plezy/services/download_manager_service.dart';
|
|||||||
import 'package:plezy/services/file_picker_service.dart';
|
import 'package:plezy/services/file_picker_service.dart';
|
||||||
import 'package:plezy/services/settings_export_service.dart';
|
import 'package:plezy/services/settings_export_service.dart';
|
||||||
import 'package:plezy/services/settings_service.dart';
|
import 'package:plezy/services/settings_service.dart';
|
||||||
|
import 'package:plezy/services/storage_service.dart';
|
||||||
import 'package:plezy/services/update_service.dart';
|
import 'package:plezy/services/update_service.dart';
|
||||||
import 'package:plezy/theme/mono_theme.dart';
|
import 'package:plezy/theme/mono_theme.dart';
|
||||||
import 'package:plezy/utils/platform_detector.dart';
|
import 'package:plezy/utils/platform_detector.dart';
|
||||||
@@ -78,6 +83,39 @@ void main() {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
testWidgets('system back closes Manage Libraries without popping pushed settings', (tester) async {
|
||||||
|
final harness = await _pumpSettingsScreen(tester, pushSettingsRoute: true);
|
||||||
|
addTearDown(() => harness.dispose(tester));
|
||||||
|
unawaited(
|
||||||
|
harness.libraries.updateLibraryOrder([
|
||||||
|
const MediaLibrary(
|
||||||
|
id: 'maestro-movies',
|
||||||
|
backend: MediaBackend.jellyfin,
|
||||||
|
title: 'Maestro Movies',
|
||||||
|
kind: MediaKind.movie,
|
||||||
|
),
|
||||||
|
]),
|
||||||
|
);
|
||||||
|
await _pumpUi(tester);
|
||||||
|
|
||||||
|
await tester.tap(find.text(t.libraries.manageLibraries));
|
||||||
|
await _pumpUi(tester);
|
||||||
|
expect(find.text('Maestro Movies'), findsOneWidget);
|
||||||
|
|
||||||
|
// Android can dispatch one physical Back through both the focused key
|
||||||
|
// path and Navigator.popRoute. The old modal fallback consumed one path
|
||||||
|
// and let the other pop Settings itself.
|
||||||
|
await tester.sendKeyDownEvent(LogicalKeyboardKey.escape);
|
||||||
|
await tester.binding.handlePopRoute();
|
||||||
|
await tester.pump();
|
||||||
|
await tester.sendKeyUpEvent(LogicalKeyboardKey.escape);
|
||||||
|
await _pumpUi(tester);
|
||||||
|
|
||||||
|
expect(find.text('Maestro Movies'), findsNothing, reason: 'system back dismisses the sheet');
|
||||||
|
expect(find.text(t.settings.services), findsOneWidget, reason: 'the settings route remains current');
|
||||||
|
expect(find.text('Settings launcher'), findsNothing, reason: 'system back must not pop the settings route');
|
||||||
|
});
|
||||||
|
|
||||||
testWidgets('migrated rows retain the shared compact row geometry and activation', (tester) async {
|
testWidgets('migrated rows retain the shared compact row geometry and activation', (tester) async {
|
||||||
final harness = await _pumpSettingsScreen(tester);
|
final harness = await _pumpSettingsScreen(tester);
|
||||||
addTearDown(() => harness.dispose(tester));
|
addTearDown(() => harness.dispose(tester));
|
||||||
@@ -539,6 +577,7 @@ class _SettingsHarness {
|
|||||||
required this.plexHome,
|
required this.plexHome,
|
||||||
required this.activeProfile,
|
required this.activeProfile,
|
||||||
required this.libraries,
|
required this.libraries,
|
||||||
|
required this.hiddenLibraries,
|
||||||
required this.theme,
|
required this.theme,
|
||||||
required this.trakt,
|
required this.trakt,
|
||||||
required this.trackers,
|
required this.trackers,
|
||||||
@@ -553,6 +592,7 @@ class _SettingsHarness {
|
|||||||
final PlexHomeService plexHome;
|
final PlexHomeService plexHome;
|
||||||
final ActiveProfileProvider activeProfile;
|
final ActiveProfileProvider activeProfile;
|
||||||
final LibrariesProvider libraries;
|
final LibrariesProvider libraries;
|
||||||
|
final HiddenLibrariesProvider hiddenLibraries;
|
||||||
final ThemeProvider theme;
|
final ThemeProvider theme;
|
||||||
final TraktAccountProvider trakt;
|
final TraktAccountProvider trakt;
|
||||||
final TrackersProvider trackers;
|
final TrackersProvider trackers;
|
||||||
@@ -567,6 +607,7 @@ class _SettingsHarness {
|
|||||||
await tester.pump();
|
await tester.pump();
|
||||||
downloadProvider.dispose();
|
downloadProvider.dispose();
|
||||||
downloadManager.dispose();
|
downloadManager.dispose();
|
||||||
|
hiddenLibraries.dispose();
|
||||||
libraries.dispose();
|
libraries.dispose();
|
||||||
theme.dispose();
|
theme.dispose();
|
||||||
trakt.dispose();
|
trakt.dispose();
|
||||||
@@ -589,6 +630,7 @@ Future<_SettingsHarness> _pumpSettingsScreen(
|
|||||||
Future<String?> Function()? settingsExporter,
|
Future<String?> Function()? settingsExporter,
|
||||||
Future<ImportResult?> Function()? settingsImporter,
|
Future<ImportResult?> Function()? settingsImporter,
|
||||||
BackgroundWorkDiagnosticsService? backgroundWorkDiagnosticsService,
|
BackgroundWorkDiagnosticsService? backgroundWorkDiagnosticsService,
|
||||||
|
bool pushSettingsRoute = false,
|
||||||
}) async {
|
}) async {
|
||||||
tester.view.physicalSize = const Size(1800, 3200);
|
tester.view.physicalSize = const Size(1800, 3200);
|
||||||
tester.view.devicePixelRatio = 1;
|
tester.view.devicePixelRatio = 1;
|
||||||
@@ -606,6 +648,8 @@ Future<_SettingsHarness> _pumpSettingsScreen(
|
|||||||
);
|
);
|
||||||
final activeProfile = ActiveProfileProvider(registry: profiles, plexHome: plexHome, connections: connections);
|
final activeProfile = ActiveProfileProvider(registry: profiles, plexHome: plexHome, connections: connections);
|
||||||
final libraries = LibrariesProvider();
|
final libraries = LibrariesProvider();
|
||||||
|
final hiddenLibraries = HiddenLibrariesProvider(storageService: _FakeHiddenLibrariesStorage());
|
||||||
|
await hiddenLibraries.ensureInitialized();
|
||||||
final theme = ThemeProvider();
|
final theme = ThemeProvider();
|
||||||
final trackerHttpClients = <FakeHttpClient>[];
|
final trackerHttpClients = <FakeHttpClient>[];
|
||||||
FakeHttpClient trackerHttpClientFactory() {
|
FakeHttpClient trackerHttpClientFactory() {
|
||||||
@@ -648,6 +692,7 @@ Future<_SettingsHarness> _pumpSettingsScreen(
|
|||||||
plexHome: plexHome,
|
plexHome: plexHome,
|
||||||
activeProfile: activeProfile,
|
activeProfile: activeProfile,
|
||||||
libraries: libraries,
|
libraries: libraries,
|
||||||
|
hiddenLibraries: hiddenLibraries,
|
||||||
theme: theme,
|
theme: theme,
|
||||||
trakt: trakt,
|
trakt: trakt,
|
||||||
trackers: trackers,
|
trackers: trackers,
|
||||||
@@ -664,6 +709,7 @@ Future<_SettingsHarness> _pumpSettingsScreen(
|
|||||||
providers: [
|
providers: [
|
||||||
ChangeNotifierProvider<ActiveProfileProvider>.value(value: activeProfile),
|
ChangeNotifierProvider<ActiveProfileProvider>.value(value: activeProfile),
|
||||||
ChangeNotifierProvider<LibrariesProvider>.value(value: libraries),
|
ChangeNotifierProvider<LibrariesProvider>.value(value: libraries),
|
||||||
|
ChangeNotifierProvider<HiddenLibrariesProvider>.value(value: hiddenLibraries),
|
||||||
ChangeNotifierProvider<ThemeProvider>.value(value: theme),
|
ChangeNotifierProvider<ThemeProvider>.value(value: theme),
|
||||||
ChangeNotifierProvider<TraktAccountProvider>.value(value: trakt),
|
ChangeNotifierProvider<TraktAccountProvider>.value(value: trakt),
|
||||||
ChangeNotifierProvider<TrackersProvider>.value(value: trackers),
|
ChangeNotifierProvider<TrackersProvider>.value(value: trackers),
|
||||||
@@ -672,21 +718,53 @@ Future<_SettingsHarness> _pumpSettingsScreen(
|
|||||||
],
|
],
|
||||||
child: MaterialApp(
|
child: MaterialApp(
|
||||||
theme: monoTheme(dark: true).copyWith(platform: TargetPlatform.android),
|
theme: monoTheme(dark: true).copyWith(platform: TargetPlatform.android),
|
||||||
home: SettingsScreen(
|
home: pushSettingsRoute
|
||||||
downloadDirectoryWritableChecker: writableChecker ?? (_) async => true,
|
? Builder(
|
||||||
settingsExporter: settingsExporter,
|
builder: (context) => Scaffold(
|
||||||
settingsImporter: settingsImporter,
|
body: Center(
|
||||||
backgroundWorkDiagnosticsService: backgroundWorkDiagnosticsService,
|
child: ElevatedButton(
|
||||||
),
|
onPressed: () => Navigator.of(context).push(
|
||||||
|
MaterialPageRoute<void>(
|
||||||
|
builder: (_) => SettingsScreen(
|
||||||
|
downloadDirectoryWritableChecker: writableChecker ?? (_) async => true,
|
||||||
|
settingsExporter: settingsExporter,
|
||||||
|
settingsImporter: settingsImporter,
|
||||||
|
backgroundWorkDiagnosticsService: backgroundWorkDiagnosticsService,
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
child: const Text('Settings launcher'),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
: SettingsScreen(
|
||||||
|
downloadDirectoryWritableChecker: writableChecker ?? (_) async => true,
|
||||||
|
settingsExporter: settingsExporter,
|
||||||
|
settingsImporter: settingsImporter,
|
||||||
|
backgroundWorkDiagnosticsService: backgroundWorkDiagnosticsService,
|
||||||
|
),
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
await tester.pump();
|
await tester.pump();
|
||||||
await tester.pump(const Duration(milliseconds: 300));
|
await tester.pump(const Duration(milliseconds: 300));
|
||||||
|
if (pushSettingsRoute) {
|
||||||
|
await tester.tap(find.text('Settings launcher'));
|
||||||
|
await _pumpUi(tester);
|
||||||
|
}
|
||||||
return harness;
|
return harness;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
class _FakeHiddenLibrariesStorage implements StorageService {
|
||||||
|
@override
|
||||||
|
Set<String> getHiddenLibraries() => {};
|
||||||
|
|
||||||
|
@override
|
||||||
|
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
|
||||||
|
}
|
||||||
|
|
||||||
class _FakeDirectoryPicker implements FilePickerDelegate {
|
class _FakeDirectoryPicker implements FilePickerDelegate {
|
||||||
@override
|
@override
|
||||||
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
|
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
|
||||||
|
|||||||
Reference in New Issue
Block a user