From 197cd4d6651602874e17d5ed34a83f72ed5eec98 Mon Sep 17 00:00:00 2001 From: David Martos Date: Fri, 19 Jan 2024 00:04:26 +0100 Subject: [PATCH] Update margin between label and icon in Tab to better reflect Material specs (#140698) This PR improves the distance between the label and the icon in the Tab widget. I updated the margin to 2 pixels, taken from the Figma design page for Material 3. On Material 2 I left the default value of 10 pixels. Related to #128696 (In particular, the distance between label and icon) Here are some screenshots for comparison. I looked a bit into the other mentioned issue of the tab height not following the M3 spec. Flutter uses 72 and the spec uses 64. But because Tab is a PreferredSizeWidget, I don't think there is an easy way to provide a different size depending on `ThemeData.useMaterial3`, because there is no `BuildContext` available. I provide a sample image for the 64 height as well for context on the linked issue, even though it's not part of the PR changes. The screenshots are taken side by side with the image at: https://m3.material.io/components/tabs/guidelines ## Original ![original](https://github.com/flutter/flutter/assets/22084723/f52d46bb-eaf9-4519-976e-9ea07c021e14) ## New (tab height = 72, Flutter default for 8 years) ![new_72](https://github.com/flutter/flutter/assets/22084723/8c9d3510-eaca-4b7d-92d8-0d06a7e75136) ## New (tab height = 64, M3 spec) ![new_64](https://github.com/flutter/flutter/assets/22084723/f8811b70-766f-4a4f-b069-33673b1e3744) --- dev/tools/gen_defaults/lib/tabs_template.dart | 5 +++ packages/flutter/lib/src/material/tabs.dart | 20 ++++++++-- packages/flutter/test/material/tabs_test.dart | 39 +++++++++++++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/dev/tools/gen_defaults/lib/tabs_template.dart b/dev/tools/gen_defaults/lib/tabs_template.dart index f520f617b2..25c895744a 100644 --- a/dev/tools/gen_defaults/lib/tabs_template.dart +++ b/dev/tools/gen_defaults/lib/tabs_template.dart @@ -77,6 +77,11 @@ class _${blockName}PrimaryDefaultsM3 extends TabBarTheme { TabAlignment? get tabAlignment => isScrollable ? TabAlignment.startOffset : TabAlignment.fill; static double indicatorWeight = ${getToken('md.comp.primary-navigation-tab.active-indicator.height')}; + + // TODO(davidmartos96): This value doesn't currently exist in + // https://m3.material.io/components/tabs/specs + // Update this when the token is available. + static const EdgeInsetsGeometry iconMargin = EdgeInsets.only(bottom: 2); } class _${blockName}SecondaryDefaultsM3 extends TabBarTheme { diff --git a/packages/flutter/lib/src/material/tabs.dart b/packages/flutter/lib/src/material/tabs.dart index f630920386..ee2d40f3c1 100644 --- a/packages/flutter/lib/src/material/tabs.dart +++ b/packages/flutter/lib/src/material/tabs.dart @@ -107,7 +107,7 @@ class Tab extends StatelessWidget implements PreferredSizeWidget { super.key, this.text, this.icon, - this.iconMargin = const EdgeInsets.only(bottom: 10.0), + this.iconMargin, this.height, this.child, }) : assert(text != null || child != null || icon != null), @@ -132,7 +132,10 @@ class Tab extends StatelessWidget implements PreferredSizeWidget { /// /// Only useful when used in combination with [icon], and either one of /// [text] or [child] is non-null. - final EdgeInsetsGeometry iconMargin; + /// + /// Defaults to 2 pixels of bottom margin. If [ThemeData.useMaterial3] is false, + /// then defaults to 10 pixels of bottom margin. + final EdgeInsetsGeometry? iconMargin; /// The height of the [Tab]. /// @@ -159,11 +162,15 @@ class Tab extends StatelessWidget implements PreferredSizeWidget { label = icon!; } else { calculatedHeight = _kTextAndIconTabHeight; + final EdgeInsetsGeometry effectiveIconMargin = iconMargin ?? + (Theme.of(context).useMaterial3 + ? _TabsPrimaryDefaultsM3.iconMargin + : _TabsDefaultsM2.iconMargin); label = Column( mainAxisAlignment: MainAxisAlignment.center, children: [ Container( - margin: iconMargin, + margin: effectiveIconMargin, child: icon, ), _buildLabelText(), @@ -2266,6 +2273,8 @@ class _TabsDefaultsM2 extends TabBarTheme { @override TabAlignment? get tabAlignment => isScrollable ? TabAlignment.start : TabAlignment.fill; + + static const EdgeInsetsGeometry iconMargin = EdgeInsets.only(bottom: 10); } // BEGIN GENERATED TOKEN PROPERTIES - Tabs @@ -2340,6 +2349,11 @@ class _TabsPrimaryDefaultsM3 extends TabBarTheme { TabAlignment? get tabAlignment => isScrollable ? TabAlignment.startOffset : TabAlignment.fill; static double indicatorWeight = 3.0; + + // TODO(davidmartos96): This value doesn't currently exist in + // https://m3.material.io/components/tabs/specs + // Update this when the token is available. + static const EdgeInsetsGeometry iconMargin = EdgeInsets.only(bottom: 2); } class _TabsSecondaryDefaultsM3 extends TabBarTheme { diff --git a/packages/flutter/test/material/tabs_test.dart b/packages/flutter/test/material/tabs_test.dart index 3cc05a412d..74d8ec8e56 100644 --- a/packages/flutter/test/material/tabs_test.dart +++ b/packages/flutter/test/material/tabs_test.dart @@ -386,6 +386,45 @@ void main() { material3 ? const Size(14.25, 72.0) : const Size(14.0, 72.0)); }); + testWidgets('Material2 - Default Tab iconMargin', (WidgetTester tester) async { + await tester.pumpWidget(MaterialApp( + theme: ThemeData(useMaterial3: false), + home: const Material( + child: Tab( + icon: Icon(Icons.house), + text: 'x', + ), + ), + )); + + double getIconMargin() { + final Rect iconRect = tester.getRect(find.byIcon(Icons.house)); + final Rect labelRect = tester.getRect(find.text('x')); + return labelRect.top - iconRect.bottom; + } + + expect(getIconMargin(), equals(10)); + }); + + testWidgets('Material3 - Default Tab iconMargin', (WidgetTester tester) async { + await tester.pumpWidget(const MaterialApp( + home: Material( + child: Tab( + icon: Icon(Icons.house), + text: 'x', + ), + ), + )); + + double getIconMargin() { + final Rect iconRect = tester.getRect(find.byIcon(Icons.house)); + final Rect labelRect = tester.getRect(find.text('x')); + return labelRect.top - iconRect.bottom; + } + + expect(getIconMargin(), equals(2)); + }); + testWidgets('Tab color - normal', (WidgetTester tester) async { final ThemeData theme = ThemeData(fontFamily: 'FlutterTest'); final bool material3 = theme.useMaterial3;