From 9d7d36cfba5ff91e851fbd077fd8e56b3df21323 Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Thu, 30 Jan 2025 22:03:56 +0200 Subject: [PATCH] Fix `Checkbox` default visual density to meet Material 3 guidelines (#159081) Fixes [Compact Visual Density is wrongfully applied to Checkboxes with `MaterialTapTargetSize.padded` on desktop platforms according to Material 3 Guidelines](https://github.com/flutter/flutter/issues/156408) ### Description This PR updates default `Checkbox` visual density to not depend on `ThemeData.visualDenSity` as it will return `VisualDensity.compact` on desktop and break Material 3 guidelines for `Checkbox`. > [!NOTE] > This makes a similar fix as https://github.com/flutter/flutter/pull/110722. ### Code sample
expand to view the code sample ```dart import 'package:flutter/material.dart'; void main() => runApp(const MyApp()); class MyApp extends StatelessWidget { const MyApp({super.key}); @override Widget build(BuildContext context) { return MaterialApp( debugShowCheckedModeBanner: false, home: Scaffold( body: Center( child: Row( mainAxisAlignment: MainAxisAlignment.center, children: [ ColoredBox( color: Colors.amber, child: Checkbox( materialTapTargetSize: MaterialTapTargetSize.padded, value: true, onChanged: (bool? value) {}, ), ), Container( width: 48, height: 48, color: Colors.red, alignment: Alignment.center, child: const Text( '48x48px', style: TextStyle(fontSize: 10, color: Colors.white), ), ) ], ), ), ), ); } } ```
Screenshot 2024-11-18 at 18 02 01 Screenshot 2024-11-18 at 18 01 49 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../gen_defaults/lib/checkbox_template.dart | 2 +- .../flutter/lib/src/material/checkbox.dart | 6 +- .../flutter/lib/src/material/theme_data.dart | 12 ++-- .../flutter/test/material/checkbox_test.dart | 69 +++++++++++++++++-- 4 files changed, 76 insertions(+), 13 deletions(-) diff --git a/dev/tools/gen_defaults/lib/checkbox_template.dart b/dev/tools/gen_defaults/lib/checkbox_template.dart index 5ee85c476a..74159d6673 100644 --- a/dev/tools/gen_defaults/lib/checkbox_template.dart +++ b/dev/tools/gen_defaults/lib/checkbox_template.dart @@ -134,7 +134,7 @@ class _${blockName}DefaultsM3 extends CheckboxThemeData { MaterialTapTargetSize get materialTapTargetSize => _theme.materialTapTargetSize; @override - VisualDensity get visualDensity => _theme.visualDensity; + VisualDensity get visualDensity => VisualDensity.standard; @override OutlinedBorder get shape => const RoundedRectangleBorder( diff --git a/packages/flutter/lib/src/material/checkbox.dart b/packages/flutter/lib/src/material/checkbox.dart index c0ab2fb62f..6eb51af0cb 100644 --- a/packages/flutter/lib/src/material/checkbox.dart +++ b/packages/flutter/lib/src/material/checkbox.dart @@ -284,7 +284,9 @@ class Checkbox extends StatefulWidget { /// {@macro flutter.material.themedata.visualDensity} /// /// If null, then the value of [CheckboxThemeData.visualDensity] is used. If - /// that is also null, then the value of [ThemeData.visualDensity] is used. + /// that is also null and if [ThemeData.useMaterial3] is false, then the + /// value of [ThemeData.visualDensity] is used. Otherwise, the default value + /// is [VisualDensity.standard]. /// /// See also: /// @@ -1041,7 +1043,7 @@ class _CheckboxDefaultsM3 extends CheckboxThemeData { MaterialTapTargetSize get materialTapTargetSize => _theme.materialTapTargetSize; @override - VisualDensity get visualDensity => _theme.visualDensity; + VisualDensity get visualDensity => VisualDensity.standard; @override OutlinedBorder get shape => const RoundedRectangleBorder( diff --git a/packages/flutter/lib/src/material/theme_data.dart b/packages/flutter/lib/src/material/theme_data.dart index c321509bf8..3597e8ad5c 100644 --- a/packages/flutter/lib/src/material/theme_data.dart +++ b/packages/flutter/lib/src/material/theme_data.dart @@ -1161,10 +1161,14 @@ class ThemeData with Diagnosticable { /// A larger value translates to a spacing increase (less dense), and a /// smaller value translates to a spacing decrease (more dense). /// - /// In Material Design 3, the [visualDensity] does not override the value of - /// [IconButton.visualDensity] which defaults to [VisualDensity.standard] - /// for all platforms. To override the default value of [IconButton.visualDensity], - /// use [ThemeData.iconButtonTheme] instead. + /// In Material Design 3, the [visualDensity] does not override the default + /// visual for the following components which are set to [VisualDensity.standard] + /// for all platforms: + /// + /// * [IconButton] - To override the default value of [IconButton.visualDensity], + /// use [ThemeData.iconButtonTheme] instead. + /// * [Checkbox] - To override the default value of [Checkbox.visualDensity], + /// use [ThemeData.checkboxTheme] instead. /// {@endtemplate} final VisualDensity visualDensity; diff --git a/packages/flutter/test/material/checkbox_test.dart b/packages/flutter/test/material/checkbox_test.dart index 9ea0d2381b..e05c02d5e2 100644 --- a/packages/flutter/test/material/checkbox_test.dart +++ b/packages/flutter/test/material/checkbox_test.dart @@ -1057,9 +1057,57 @@ void main() { expect(value, isTrue); }); - testWidgets('Checkbox responds to density changes.', (WidgetTester tester) async { + testWidgets( + 'Material3 - Checkbox visual density cannot be overriden by ThemeData.visualDensity', + (WidgetTester tester) async { + const Key key = Key('test'); + Widget buldCheckbox() { + return MaterialApp( + theme: theme.copyWith(visualDensity: VisualDensity.compact), + home: Material( + child: Center(child: Checkbox(key: key, value: true, onChanged: (bool? value) {})), + ), + ); + } + + await tester.pumpWidget(buldCheckbox()); + await tester.pumpAndSettle(); + final RenderBox box = tester.renderObject(find.byKey(key)); + expect(box.size, equals(const Size(48, 48))); + }, + ); + + testWidgets( + 'Material3 - Checkbox with MaterialTapTargetSize.padded meets Material Guidelines on desktop', + (WidgetTester tester) async { + const Key key = Key('test'); + Widget buldCheckbox() { + return MaterialApp( + theme: theme, + home: Material( + child: Center( + child: Checkbox( + key: key, + materialTapTargetSize: MaterialTapTargetSize.padded, + value: true, + onChanged: (bool? value) {}, + ), + ), + ), + ); + } + + await tester.pumpWidget(buldCheckbox()); + await tester.pumpAndSettle(); + final RenderBox box = tester.renderObject(find.byKey(key)); + expect(box.size, equals(const Size(48, 48))); + }, + variant: TargetPlatformVariant.desktop(), + ); + + testWidgets('Checkbox responds to density changes', (WidgetTester tester) async { const Key key = Key('test'); - Future buildTest(VisualDensity visualDensity) async { + Future buildTest({VisualDensity? visualDensity}) async { return tester.pumpWidget( MaterialApp( theme: theme, @@ -1077,20 +1125,29 @@ void main() { ); } - await buildTest(VisualDensity.standard); + // Test the default visual density. + await buildTest(); + await tester.pumpAndSettle(); final RenderBox box = tester.renderObject(find.byKey(key)); + expect(box.size, equals(const Size(48, 48))); + + await buildTest(visualDensity: VisualDensity.standard); await tester.pumpAndSettle(); expect(box.size, equals(const Size(48, 48))); - await buildTest(const VisualDensity(horizontal: 3.0, vertical: 3.0)); + await buildTest(visualDensity: VisualDensity.compact); + await tester.pumpAndSettle(); + expect(box.size, equals(const Size(40, 40))); + + await buildTest(visualDensity: const VisualDensity(horizontal: 3.0, vertical: 3.0)); await tester.pumpAndSettle(); expect(box.size, equals(const Size(60, 60))); - await buildTest(const VisualDensity(horizontal: -3.0, vertical: -3.0)); + await buildTest(visualDensity: const VisualDensity(horizontal: -3.0, vertical: -3.0)); await tester.pumpAndSettle(); expect(box.size, equals(const Size(36, 36))); - await buildTest(const VisualDensity(horizontal: 3.0, vertical: -3.0)); + await buildTest(visualDensity: const VisualDensity(horizontal: 3.0, vertical: -3.0)); await tester.pumpAndSettle(); expect(box.size, equals(const Size(60, 36))); });