From 15b54e2ec6bc4b8396a3b753cf970eb7b12ff763 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:04:53 +0200 Subject: [PATCH] fix(settings): restore the shared compact row size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings rows carried their own platform-conditional typography and density, so on desktop and TV they rendered a 16px title, 14px subtitle and 80px row while every other row in the app — the Focusable*ListTile defaults plus ThemeData.listTileTheme's `dense: true` — renders 13/12 in 61px. Drop the overrides instead of re-tuning them: the tile defaults already encode the app's row style, and the explicit title styles were redundant under a dense ListTile (they also masked the disabled/selected title color). settingsOptionTitleStyle now only serves group children that are not ListTiles, and matches the dense title unconditionally. SettingsGroup hands its children that same compact density, so the plain ListTiles used as non-interactive info rows stop standing 11px taller than their interactive siblings. --- lib/screens/catalog_item_detail_screen.dart | 2 - .../settings/services_settings_screen.dart | 4 +- lib/screens/settings/settings_screen.dart | 18 +---- lib/widgets/setting_tile.dart | 8 +-- lib/widgets/settings_section.dart | 53 ++++++++------- .../settings/settings_screen_test.dart | 22 +++--- test/widgets/setting_tile_test.dart | 68 +++++++++++++------ 7 files changed, 93 insertions(+), 82 deletions(-) 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')), ], ), ),