diff --git a/lib/screens/catalog_item_detail_screen.dart b/lib/screens/catalog_item_detail_screen.dart index b3f5dc3c..a7923138 100644 --- a/lib/screens/catalog_item_detail_screen.dart +++ b/lib/screens/catalog_item_detail_screen.dart @@ -308,8 +308,6 @@ class _CatalogItemDetailScreenState extends State { Widget _buildLibraryMatchTile(MediaItem match, int index) { return FocusableListTile( focusNode: _libraryMatchFocusNodes[index], - dense: false, - visualDensity: VisualDensity.standard, leading: BackendBadge(backend: match.backend, size: 24), // Plex matches carry their library title; Jellyfin's search-based // lookup doesn't, so fall back to the server name alone. diff --git a/lib/screens/settings/services_settings_screen.dart b/lib/screens/settings/services_settings_screen.dart index 835749ab..54b1440d 100644 --- a/lib/screens/settings/services_settings_screen.dart +++ b/lib/screens/settings/services_settings_screen.dart @@ -101,12 +101,10 @@ class _ServiceHubRow extends StatelessWidget { Widget build(BuildContext context) { return FocusableListTile( leading: leading, - title: Text(title, style: settingsOptionTitleStyle(context)), + title: Text(title), subtitle: Text(username != null ? t.services.connectedAs(username: username!) : t.services.notConnected), trailing: const AppIcon(Symbols.chevron_right_rounded, fill: 1), onTap: onTap, - dense: settingsRowDense(context), - visualDensity: settingsRowVisualDensity(context), ); } } diff --git a/lib/screens/settings/settings_screen.dart b/lib/screens/settings/settings_screen.dart index 7755e6db..c64618cf 100644 --- a/lib/screens/settings/settings_screen.dart +++ b/lib/screens/settings/settings_screen.dart @@ -355,15 +355,10 @@ class _SettingsScreenState extends State with FocusableTab, Moun return FocusableListTile( focusNode: _focusTracker.get(_kDownloadLocation), leading: const AppIcon(Symbols.folder_rounded, fill: 1), - title: Text( - isCustom ? t.settings.downloadLocationCustom : t.settings.downloadLocationDefault, - style: settingsOptionTitleStyle(context), - ), + title: Text(isCustom ? t.settings.downloadLocationCustom : t.settings.downloadLocationDefault), subtitle: Text(currentPath, maxLines: 2, overflow: .ellipsis), trailing: const AppIcon(Symbols.chevron_right_rounded, fill: 1), onTap: () => _showDownloadLocationDialog(), - dense: false, - visualDensity: VisualDensity.standard, ); }, ), @@ -412,7 +407,7 @@ class _SettingsScreenState extends State with FocusableTab, Moun return FocusableListTile( focusNode: _focusTracker.get(_kBackgroundDownloads), leading: AppIcon(icon, fill: 1, color: color), - title: Text(t.downloads.backgroundWarning.statusTile, style: settingsOptionTitleStyle(context)), + title: Text(t.downloads.backgroundWarning.statusTile), subtitle: Text(summary), trailing: const AppIcon(Symbols.chevron_right_rounded, fill: 1), onTap: () async { @@ -424,8 +419,6 @@ class _SettingsScreenState extends State with FocusableTab, Moun } await showBackgroundDownloadWarningDialog(context, service: diagnostics); }, - dense: false, - visualDensity: VisualDensity.standard, ); }, ); @@ -588,10 +581,7 @@ class _SettingsScreenState extends State with FocusableTab, Moun fill: 1, color: hasUpdate ? Colors.orange : null, ), - title: Text( - hasUpdate ? t.settings.updateAvailable : t.settings.checkForUpdates, - style: settingsOptionTitleStyle(context), - ), + title: Text(hasUpdate ? t.settings.updateAvailable : t.settings.checkForUpdates), subtitle: hasUpdate ? Text(t.update.versionAvailable(version: _updateInfo!['latestVersion'])) : null, trailing: _isCheckingForUpdate ? const LoadingIndicatorBox(size: 24) @@ -605,8 +595,6 @@ class _SettingsScreenState extends State with FocusableTab, Moun _checkForUpdates(); } }, - dense: false, - visualDensity: VisualDensity.standard, ), _buildAutoCheckUpdatesOnStartupTile(), ], diff --git a/lib/widgets/setting_tile.dart b/lib/widgets/setting_tile.dart index bdf62aab..26a504a6 100644 --- a/lib/widgets/setting_tile.dart +++ b/lib/widgets/setting_tile.dart @@ -45,12 +45,10 @@ class _SettingRow extends StatelessWidget { return FocusableListTile( focusNode: focusNode, leading: AppIcon(icon, fill: 1), - title: Text(title, style: settingsOptionTitleStyle(context)), + title: Text(title), subtitle: subtitle, trailing: trailing ?? const AppIcon(Symbols.chevron_right_rounded, fill: 1), onTap: onTap, - dense: settingsRowDense(context), - visualDensity: settingsRowVisualDensity(context), ); } } @@ -83,11 +81,9 @@ class SettingSwitchTile extends StatelessWidget { builder: (_, value, _) => FocusableSwitchListTile( focusNode: focusNode, secondary: AppIcon(icon, fill: 1), - title: Text(title, style: settingsOptionTitleStyle(context)), + title: Text(title), subtitle: subtitle != null ? Text(subtitle!) : null, value: value, - dense: settingsRowDense(context), - visualDensity: settingsRowVisualDensity(context), onChanged: enabled ? (v) => _writeAndNotify(pref, v, onAfterWrite) : null, ), ); diff --git a/lib/widgets/settings_section.dart b/lib/widgets/settings_section.dart index 84dcd37a..6cc7482b 100644 --- a/lib/widgets/settings_section.dart +++ b/lib/widgets/settings_section.dart @@ -1,24 +1,18 @@ import 'package:flutter/material.dart'; import '../theme/mono_tokens.dart'; -import '../utils/platform_detector.dart'; import 'app_icon.dart'; import 'expressive_button_group.dart'; -/// Standard settings-option title style. +/// Standard settings-option title style, for group children that are not a +/// [ListTile] (segmented controls, sliders) and so don't inherit its +/// typography. /// -/// Mobile uses Flutter's compact ListTile title size. Desktop and TV retain -/// the larger body style used for pointer and D-pad readability. -TextStyle? settingsOptionTitleStyle(BuildContext context) { - final style = Theme.of(context).textTheme.bodyLarge; - return PlatformDetector.isMobile(context) ? style?.copyWith(fontSize: 13) : style; -} - -/// Standard settings-row density, paired with [settingsOptionTitleStyle]: -/// compact on mobile and full-height on desktop and TV. -bool settingsRowDense(BuildContext context) => PlatformDetector.isMobile(context); - -VisualDensity settingsRowVisualDensity(BuildContext context) => - settingsRowDense(context) ? const VisualDensity(vertical: -3) : VisualDensity.standard; +/// The app renders every row compactly — [ThemeData.listTileTheme] sets +/// `dense: true` and the `Focusable*ListTile`s default to it — and Flutter +/// draws a dense [ListTile] title at 13. Match that so a settings page reads +/// as one family instead of one size per row type. +TextStyle? settingsOptionTitleStyle(BuildContext context) => + Theme.of(context).textTheme.bodyLarge?.copyWith(fontSize: 13); class SettingsSectionHeader extends StatelessWidget { final String title; @@ -50,6 +44,10 @@ class SettingsSectionHeader extends StatelessWidget { /// highlight paints clipped inside the card — that is the d-pad focus visual /// (background focus). The group adds no [Focus] nodes of its own; traversal /// order and externally-owned tile focus nodes are untouched. +/// +/// Children inherit the compact row geometry the `Focusable*ListTile`s default +/// to, so a plain [ListTile] used as a non-interactive info row lines up with +/// its interactive siblings instead of standing 11px taller. class SettingsGroup extends StatelessWidget { final String? title; final List children; @@ -71,18 +69,21 @@ class SettingsGroup extends StatelessWidget { if (title != null) SettingsSectionHeader(title!), Padding( padding: margin, - child: Column( - children: [ - for (var i = 0; i < children.length; i++) ...[ - if (i > 0) SizedBox(height: t.groupGap), - Material( - color: t.surface, - clipBehavior: Clip.antiAlias, - shape: RoundedRectangleBorder(borderRadius: groupItemRadii(context, i, children.length)), - child: children[i], - ), + child: ListTileTheme.merge( + visualDensity: const VisualDensity(vertical: -3), + child: Column( + children: [ + for (var i = 0; i < children.length; i++) ...[ + if (i > 0) SizedBox(height: t.groupGap), + Material( + color: t.surface, + clipBehavior: Clip.antiAlias, + shape: RoundedRectangleBorder(borderRadius: groupItemRadii(context, i, children.length)), + child: children[i], + ), + ], ], - ], + ), ), ), ], diff --git a/test/screens/settings/settings_screen_test.dart b/test/screens/settings/settings_screen_test.dart index e1da8045..ed1f58cb 100644 --- a/test/screens/settings/settings_screen_test.dart +++ b/test/screens/settings/settings_screen_test.dart @@ -78,7 +78,7 @@ void main() { } }); - testWidgets('migrated rows retain compact mobile navigation geometry and activation', (tester) async { + testWidgets('migrated rows retain the shared compact row geometry and activation', (tester) async { final harness = await _pumpSettingsScreen(tester); addTearDown(() => harness.dispose(tester)); @@ -119,7 +119,7 @@ void main() { find.descendant(of: focusableFinder, matching: find.byType(ListTile)), ); - expect(focusable.dense, isTrue, reason: '${row.title} must use the shared compact mobile density'); + expect(focusable.dense, isTrue, reason: '${row.title} must use the shared compact row density'); expect(focusable.visualDensity, const VisualDensity(vertical: -3)); expect(materialTile.dense, isTrue); expect(materialTile.visualDensity, const VisualDensity(vertical: -3)); @@ -167,7 +167,7 @@ void main() { expect(find.text(t.settings.clearImageCache), findsWidgets); }); - testWidgets('special download and generic update rows retain rich content at standard density', (tester) async { + testWidgets('special download and generic update rows keep rich content on the shared compact row', (tester) async { final harness = await _pumpSettingsScreen(tester); addTearDown(() => harness.dispose(tester)); @@ -182,10 +182,10 @@ void main() { final subtitle = downloadTile.subtitle! as Text; expect(_navigationTileFor(t.settings.downloadLocationDefault), findsNothing); - expect(materialDownloadTile.dense, isFalse); - expect(materialDownloadTile.visualDensity, VisualDensity.standard); - expect(downloadTile.dense, isFalse); - expect(downloadTile.visualDensity, VisualDensity.standard); + expect(materialDownloadTile.dense, isTrue); + expect(materialDownloadTile.visualDensity, const VisualDensity(vertical: -3)); + expect(downloadTile.dense, isTrue); + expect(downloadTile.visualDensity, const VisualDensity(vertical: -3)); expect(downloadTile.leading, isA()); expect(downloadTile.trailing, isA()); expect(subtitle.maxLines, 2); @@ -217,10 +217,10 @@ void main() { final updateTile = tester.widget(updateTileFinder); expect(_navigationTileFor(t.settings.checkForUpdates), findsNothing); - expect(updateTile.dense, isFalse); - expect(materialUpdateTile.dense, isFalse); - expect(materialUpdateTile.visualDensity, VisualDensity.standard); - expect(updateTile.visualDensity, VisualDensity.standard); + expect(updateTile.dense, isTrue); + expect(materialUpdateTile.dense, isTrue); + expect(materialUpdateTile.visualDensity, const VisualDensity(vertical: -3)); + expect(updateTile.visualDensity, const VisualDensity(vertical: -3)); expect(updateTile.trailing, isA()); expect(updateTile.onTap, isNotNull); expect(find.descendant(of: updateTileFinder, matching: find.byType(LoadingIndicatorBox)), findsNothing); diff --git a/test/widgets/setting_tile_test.dart b/test/widgets/setting_tile_test.dart index 5c778afa..2150da0f 100644 --- a/test/widgets/setting_tile_test.dart +++ b/test/widgets/setting_tile_test.dart @@ -5,48 +5,75 @@ import 'package:plezy/widgets/focusable_list_tile.dart'; import 'package:plezy/widgets/setting_tile.dart'; import 'package:plezy/widgets/settings_section.dart'; +/// Settings rows must render exactly like every other row in the app on every +/// platform. Each row type is measured against an untouched [FocusableListTile] +/// — the app-wide reference row — rather than against a hard-coded size, so the +/// assertions still hold if the app's row density is ever retuned. void main() { - testWidgets('mobile settings option titles match compact native rows', (tester) async { - await tester.pumpWidget(_harness(TargetPlatform.android, referenceDense: true)); + for (final platform in [TargetPlatform.android, TargetPlatform.macOS]) { + testWidgets('settings rows match the standard app row on $platform', (tester) async { + await tester.pumpWidget(_harness(platform)); - final referenceHeight = tester.getSize(find.text('Clear Cache')).height; - expect(tester.getSize(find.text('View Logs')).height, referenceHeight); - expect(tester.getSize(find.text('View Mode')).height, referenceHeight); + final referenceTitle = _titleFontSize(tester, 'Clear Cache'); + final referenceSubtitle = _subtitleFontSize(tester, 'Clear cached data'); + final referenceHeight = tester.getSize(find.byKey(const ValueKey('reference'))).height; - final referenceSubtitleHeight = tester.getSize(find.text('Clear cached data')).height; - expect(tester.getSize(find.text('View application logs')).height, referenceSubtitleHeight); - }); + // The dense ListTile title/subtitle sizes; spelled out so a silent + // theme-wide inflation can't make every side of the comparison agree. + expect(referenceTitle, 13); + expect(referenceSubtitle, 12); - testWidgets('desktop settings option titles retain standard row size', (tester) async { - await tester.pumpWidget(_harness(TargetPlatform.macOS, referenceDense: false)); - - final referenceHeight = tester.getSize(find.text('Clear Cache')).height; - expect(tester.getSize(find.text('View Logs')).height, referenceHeight); - expect(tester.getSize(find.text('View Mode')).height, referenceHeight); - }); + for (final title in ['View Logs', 'Enable Thing', 'View Mode', 'Account']) { + expect(_titleFontSize(tester, title), referenceTitle, reason: '$title title size'); + } + for (final subtitle in ['View application logs', 'Toggles the thing', 'signed in']) { + expect(_subtitleFontSize(tester, subtitle), referenceSubtitle, reason: '$subtitle subtitle size'); + } + for (final key in ['navigation', 'switch', 'plain']) { + expect(tester.getSize(find.byKey(ValueKey(key))).height, referenceHeight, reason: '$key row height'); + } + }); + } } -Widget _harness(TargetPlatform platform, {required bool referenceDense}) { +double? _titleFontSize(WidgetTester tester, String text) { + final finder = find.text(text); + final inherited = DefaultTextStyle.of(tester.element(finder)).style; + return inherited.merge(tester.widget(finder).style).fontSize; +} + +double? _subtitleFontSize(WidgetTester tester, String text) => + DefaultTextStyle.of(tester.element(find.text(text))).style.fontSize; + +Widget _harness(TargetPlatform platform) { return MaterialApp( theme: monoTheme(dark: false).copyWith(platform: platform), home: Scaffold( - body: Column( + body: SettingsGroup( children: [ SettingNavigationTile( + key: const ValueKey('navigation'), icon: Icons.article, title: 'View Logs', subtitle: 'View application logs', onTap: () {}, ), FocusableListTile( + key: const ValueKey('reference'), leading: const Icon(Icons.cleaning_services), title: const Text('Clear Cache'), subtitle: const Text('Clear cached data'), trailing: const Icon(Icons.chevron_right), - dense: referenceDense, - visualDensity: referenceDense ? const VisualDensity(vertical: -3) : VisualDensity.standard, onTap: () {}, ), + FocusableSwitchListTile( + key: const ValueKey('switch'), + secondary: const Icon(Icons.toggle_on), + title: const Text('Enable Thing'), + subtitle: const Text('Toggles the thing'), + value: true, + onChanged: (_) {}, + ), SegmentedSetting( icon: Icons.view_list, title: 'View Mode', @@ -57,6 +84,9 @@ Widget _harness(TargetPlatform platform, {required bool referenceDense}) { selected: 'grid', onChanged: (_) {}, ), + // Non-interactive info rows are plain ListTiles; the group hands them + // the same geometry as their interactive siblings. + const ListTile(key: ValueKey('plain'), title: Text('Account'), subtitle: Text('signed in')), ], ), ),