diff options
| -rw-r--r-- | player-android/lib/main.dart | 14 | ||||
| -rw-r--r-- | player-android/lib/providers/theme_provider.dart | 28 | ||||
| -rw-r--r-- | player-android/lib/screens/settings_screen.dart | 14 | ||||
| -rw-r--r-- | player-android/test/screens/settings_screen_test.dart | 110 |
4 files changed, 143 insertions, 23 deletions
diff --git a/player-android/lib/main.dart b/player-android/lib/main.dart index 2c1b193..3358383 100644 --- a/player-android/lib/main.dart +++ b/player-android/lib/main.dart @@ -64,6 +64,16 @@ void main() async { } // --------------------------------------------------------------------------- +// Module-level theme constants +// --------------------------------------------------------------------------- + +// Built once at startup rather than on every rebuild of PlayerAndroidApp. +// ThemeData construction is not cheap, and the colour tokens never change +// at runtime — only the active ThemeMode (light/dark/system) does. +final _lightTheme = buildLightTheme(); +final _darkTheme = buildDarkTheme(); + +// --------------------------------------------------------------------------- // Root widget // --------------------------------------------------------------------------- @@ -92,8 +102,8 @@ class PlayerAndroidApp extends ConsumerWidget { title: 'Player', routerConfig: router, // Material 3 is enabled in both ThemeData instances; see theme_provider.dart. - theme: buildLightTheme(), - darkTheme: buildDarkTheme(), + theme: _lightTheme, + darkTheme: _darkTheme, themeMode: themeMode, ); } diff --git a/player-android/lib/providers/theme_provider.dart b/player-android/lib/providers/theme_provider.dart index 7ebaa89..29161ec 100644 --- a/player-android/lib/providers/theme_provider.dart +++ b/player-android/lib/providers/theme_provider.dart @@ -30,15 +30,15 @@ String _themeModeToString(ThemeMode mode) => switch (mode) { }; // --------------------------------------------------------------------------- -// Color schemes derived from player-server/docs/theming.md +// Color schemes derived from player-server/web/css/theme.css // // Dark palette mirrors the CSS :root block; light palette mirrors // [data-theme="light"]. Material 3 ColorScheme is built from the key tokens: -// primary ← --accent -// onPrimary ← --text-inverse / white -// surface ← --bg-surface -// background ← --bg-body -// error ← --danger +// primary ← --accent +// onPrimary ← --text-inverse / white +// surface ← --bg-surface +// scaffoldBackgroundColor ← --bg-body +// error ← --danger // --------------------------------------------------------------------------- /// Material 3 dark [ColorScheme] matching the server's default dark palette. @@ -151,12 +151,20 @@ class ThemeNotifier extends AsyncNotifier<ThemeMode> { /// Updates the active [ThemeMode] and persists the choice to disk. /// - /// The in-memory state is updated first so that [MaterialApp.themeMode] - /// changes immediately; the disk write follows asynchronously. + /// In-memory state is updated first so [MaterialApp.themeMode] changes + /// immediately without blocking on I/O. If the disk write fails, the + /// previous state is restored so in-memory and disk stay in sync. Future<void> setThemeMode(ThemeMode mode) async { + final previous = state; state = AsyncData(mode); - final prefs = await SharedPreferences.getInstance(); - await prefs.setString(_kThemeModeKey, _themeModeToString(mode)); + try { + final prefs = await SharedPreferences.getInstance(); + await prefs.setString(_kThemeModeKey, _themeModeToString(mode)); + } catch (_) { + // Roll back so in-memory and disk stay in sync. + state = previous; + rethrow; + } } } diff --git a/player-android/lib/screens/settings_screen.dart b/player-android/lib/screens/settings_screen.dart index 25bb6bc..f7b38bf 100644 --- a/player-android/lib/screens/settings_screen.dart +++ b/player-android/lib/screens/settings_screen.dart @@ -222,7 +222,7 @@ class _SettingsScreenState extends ConsumerState<SettingsScreen> { ), const SizedBox(height: 12), - _ThemeToggle(), + const _ThemeToggle(), const SizedBox(height: 32), const Divider(), @@ -265,8 +265,7 @@ class _SettingsScreenState extends ConsumerState<SettingsScreen> { /// Kept as a separate [ConsumerWidget] (SRP) so [_SettingsScreenState] does /// not need to know about [themeProvider] — it only needs to place the widget. class _ThemeToggle extends ConsumerWidget { - // ignore: prefer_const_constructors_in_immutables — private widget, not const - _ThemeToggle(); + const _ThemeToggle(); @override Widget build(BuildContext context, WidgetRef ref) { @@ -274,14 +273,6 @@ class _ThemeToggle extends ConsumerWidget { // immediately rather than showing an empty state. final current = ref.watch(themeProvider).valueOrNull ?? ThemeMode.system; - return _buildSegmentedButton(context, ref, current); - } - - Widget _buildSegmentedButton( - BuildContext context, - WidgetRef ref, - ThemeMode current, - ) { return SegmentedButton<ThemeMode>( key: const Key('settings_theme_toggle'), segments: const [ @@ -305,6 +296,7 @@ class _ThemeToggle extends ConsumerWidget { // Allow only single selection — the user always has exactly one mode active. multiSelectionEnabled: false, onSelectionChanged: (selection) { + // emptySelectionAllowed defaults to false, but guard defensively against future API changes. if (selection.isNotEmpty) { ref.read(themeProvider.notifier).setThemeMode(selection.first); } diff --git a/player-android/test/screens/settings_screen_test.dart b/player-android/test/screens/settings_screen_test.dart index 71c2d7d..ecbc94d 100644 --- a/player-android/test/screens/settings_screen_test.dart +++ b/player-android/test/screens/settings_screen_test.dart @@ -21,6 +21,7 @@ import 'package:player_android/api/dio_client.dart'; import 'package:player_android/providers/api_client_provider.dart'; import 'package:player_android/providers/auth_state_provider.dart'; import 'package:player_android/providers/settings_provider.dart'; +import 'package:player_android/providers/theme_provider.dart'; import 'package:player_android/screens/settings_screen.dart'; // --------------------------------------------------------------------------- @@ -65,6 +66,25 @@ class _FakeSettingsNotifier extends SettingsNotifier { } } +/// In-memory [ThemeNotifier] that bypasses [SharedPreferences] in tests. +/// +/// Starts at [ThemeMode.system] (the default) and records the last mode passed +/// to [setThemeMode] so tests can assert on the captured value. +class _FakeThemeNotifier extends ThemeNotifier { + // Tracks the last mode applied via setThemeMode for test assertions. + ThemeMode? capturedMode; + + @override + Future<ThemeMode> build() async => ThemeMode.system; + + @override + Future<void> setThemeMode(ThemeMode mode) async { + capturedMode = mode; + // Mirror the production behaviour: update in-memory state immediately. + state = AsyncData(mode); + } +} + // --------------------------------------------------------------------------- // Helper: pump SettingsScreen inside a minimal ProviderScope. // --------------------------------------------------------------------------- @@ -260,6 +280,96 @@ void main() { }); // -------------------------------------------------------------------------- + // Theme toggle + // -------------------------------------------------------------------------- + + group('theme toggle', () { + testWidgets('renders with system segment selected by default', + (tester) async { + final fakeStorage = _FakeTokenStorage().._token = 'alice'; + final fakeSettings = _FakeSettingsNotifier('http://10.0.2.2:8080'); + final fakeTheme = _FakeThemeNotifier(); + + final router = GoRouter( + initialLocation: '/', + routes: [ + GoRoute( + path: '/', + builder: (_, __) => const SettingsScreen(), + ), + GoRoute( + path: '/login', + builder: (_, __) => const Scaffold(body: Text('Login')), + ), + ], + ); + + await tester.pumpWidget( + ProviderScope( + overrides: [ + tokenStorageProvider.overrideWithValue(fakeStorage), + settingsProvider.overrideWith(() => fakeSettings), + themeProvider.overrideWith(() => fakeTheme), + ], + child: MaterialApp.router(routerConfig: router), + ), + ); + await tester.pumpAndSettle(); + + // The segmented button should be present with the system segment selected. + final button = tester.widget<SegmentedButton<ThemeMode>>( + find.byKey(const Key('settings_theme_toggle')), + ); + expect(button.selected, equals({ThemeMode.system})); + }); + + testWidgets('tapping a segment calls setThemeMode with the right ThemeMode', + (tester) async { + final fakeStorage = _FakeTokenStorage().._token = 'alice'; + final fakeSettings = _FakeSettingsNotifier('http://10.0.2.2:8080'); + final fakeTheme = _FakeThemeNotifier(); + + final router = GoRouter( + initialLocation: '/', + routes: [ + GoRoute( + path: '/', + builder: (_, __) => const SettingsScreen(), + ), + GoRoute( + path: '/login', + builder: (_, __) => const Scaffold(body: Text('Login')), + ), + ], + ); + + await tester.pumpWidget( + ProviderScope( + overrides: [ + tokenStorageProvider.overrideWithValue(fakeStorage), + settingsProvider.overrideWith(() => fakeSettings), + themeProvider.overrideWith(() => fakeTheme), + ], + child: MaterialApp.router(routerConfig: router), + ), + ); + await tester.pumpAndSettle(); + + // Scroll the segmented button into view before tapping — the settings + // screen content may exceed the test viewport height. + await tester.ensureVisible(find.byKey(const Key('settings_theme_toggle'))); + await tester.pumpAndSettle(); + + // Tap the "Dark" segment label. + await tester.tap(find.text('Dark')); + await tester.pumpAndSettle(); + + // The fake notifier should have received ThemeMode.dark. + expect(fakeTheme.capturedMode, equals(ThemeMode.dark)); + }); + }); + + // -------------------------------------------------------------------------- // Logout flow // -------------------------------------------------------------------------- |