fix(settings): restore the shared compact row size
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.
This commit is contained in:
@@ -308,8 +308,6 @@ class _CatalogItemDetailScreenState extends State<CatalogItemDetailScreen> {
|
||||
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.
|
||||
|
||||
@@ -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),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -355,15 +355,10 @@ class _SettingsScreenState extends State<SettingsScreen> 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<SettingsScreen> 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<SettingsScreen> with FocusableTab, Moun
|
||||
}
|
||||
await showBackgroundDownloadWarningDialog(context, service: diagnostics);
|
||||
},
|
||||
dense: false,
|
||||
visualDensity: VisualDensity.standard,
|
||||
);
|
||||
},
|
||||
);
|
||||
@@ -588,10 +581,7 @@ class _SettingsScreenState extends State<SettingsScreen> 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<SettingsScreen> with FocusableTab, Moun
|
||||
_checkForUpdates();
|
||||
}
|
||||
},
|
||||
dense: false,
|
||||
visualDensity: VisualDensity.standard,
|
||||
),
|
||||
_buildAutoCheckUpdatesOnStartupTile(),
|
||||
],
|
||||
|
||||
@@ -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,
|
||||
),
|
||||
);
|
||||
|
||||
@@ -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<Widget> children;
|
||||
@@ -71,6 +69,8 @@ class SettingsGroup extends StatelessWidget {
|
||||
if (title != null) SettingsSectionHeader(title!),
|
||||
Padding(
|
||||
padding: margin,
|
||||
child: ListTileTheme.merge(
|
||||
visualDensity: const VisualDensity(vertical: -3),
|
||||
child: Column(
|
||||
children: [
|
||||
for (var i = 0; i < children.length; i++) ...[
|
||||
@@ -85,6 +85,7 @@ class SettingsGroup extends StatelessWidget {
|
||||
],
|
||||
),
|
||||
),
|
||||
),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<AppIcon>());
|
||||
expect(downloadTile.trailing, isA<AppIcon>());
|
||||
expect(subtitle.maxLines, 2);
|
||||
@@ -217,10 +217,10 @@ void main() {
|
||||
final updateTile = tester.widget<FocusableListTile>(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<AppIcon>());
|
||||
expect(updateTile.onTap, isNotNull);
|
||||
expect(find.descendant(of: updateTileFinder, matching: find.byType(LoadingIndicatorBox)), findsNothing);
|
||||
|
||||
@@ -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<Text>(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<String>(
|
||||
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')),
|
||||
],
|
||||
),
|
||||
),
|
||||
|
||||
Reference in New Issue
Block a user