From 755cf0bd3f73ac2fe38d973a196e5486f5fe70b9 Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Sat, 7 Sep 2024 00:10:35 +0300 Subject: [PATCH] Fix Material 3 `AppBar.leading` action IconButtons (#154512) Fixes [`AppBar` back button focus/hover circle should not fill up whole height](https://github.com/flutter/flutter/issues/141361) Fixes [[Material 3] Date Range Picker close button has incorrect shape](https://github.com/flutter/flutter/issues/154393) This updates the leading condition added in 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( home: Scaffold( body: SingleChildScrollView( child: Column( children: [ Column( spacing: 10.0, mainAxisSize: MainAxisSize.min, children: [ AppBar( leading: BackButton( style: IconButton.styleFrom(backgroundColor: Colors.red), ), backgroundColor: Theme.of(context).colorScheme.secondaryContainer, title: const Text('AppBar with BackButton'), ), AppBar( leading: CloseButton( style: IconButton.styleFrom(backgroundColor: Colors.red), ), backgroundColor: Theme.of(context).colorScheme.secondaryContainer, title: const Text('AppBar with CloseButton'), ), AppBar( leading: DrawerButton( style: IconButton.styleFrom(backgroundColor: Colors.red), ), backgroundColor: Theme.of(context).colorScheme.secondaryContainer, title: const Text('AppBar with DrawerButton'), ), ], ), const Divider(), Column( spacing: 10.0, mainAxisSize: MainAxisSize.min, children: [ AppBar( leading: BackButton( style: IconButton.styleFrom(backgroundColor: Colors.red), ), backgroundColor: Theme.of(context).colorScheme.secondaryContainer, toolbarHeight: 100.0, title: const Text('AppBar with custom height'), ), AppBar( leading: CloseButton( style: IconButton.styleFrom(backgroundColor: Colors.red), ), backgroundColor: Theme.of(context).colorScheme.secondaryContainer, toolbarHeight: 100.0, title: const Text('AppBar with custom height'), ), AppBar( leading: DrawerButton( style: IconButton.styleFrom(backgroundColor: Colors.red), ), backgroundColor: Theme.of(context).colorScheme.secondaryContainer, toolbarHeight: 100.0, title: const Text('AppBar with custom height'), ), ], ), ], ), ), ), ); } } ```
### Before Screenshot 2024-09-04 at 12 38 05 ### After Screenshot 2024-09-04 at 12 38 28 --- .../lib/src/material/action_buttons.dart | 35 +-- .../test/material/app_bar_sliver_test.dart | 6 +- .../flutter/test/material/app_bar_test.dart | 199 ++++++++++++++++++ .../test/material/date_range_picker_test.dart | 29 ++- 4 files changed, 237 insertions(+), 32 deletions(-) diff --git a/packages/flutter/lib/src/material/action_buttons.dart b/packages/flutter/lib/src/material/action_buttons.dart index 4d6e125a54..68cc2b2787 100644 --- a/packages/flutter/lib/src/material/action_buttons.dart +++ b/packages/flutter/lib/src/material/action_buttons.dart @@ -21,42 +21,17 @@ import 'material_localizations.dart'; import 'scaffold.dart'; import 'theme.dart'; -abstract class _ActionButton extends StatelessWidget { +abstract class _ActionButton extends IconButton { /// Creates a Material Design icon button. const _ActionButton({ super.key, - this.color, - required this.icon, - required this.onPressed, + super.color, + super.style, + super.onPressed, + required super.icon, this.standardComponent, - this.style, }); - /// The icon to display inside the button. - final Widget icon; - - /// The callback that is called when the button is tapped - /// or otherwise activated. - /// - /// If this is set to null, the button will do a default action - /// when it is tapped or activated. - final VoidCallback? onPressed; - - /// The color to use for the icon. - /// - /// Defaults to the [IconThemeData.color] specified in the ambient [IconTheme], - /// which usually matches the ambient [Theme]'s [ThemeData.iconTheme]. - final Color? color; - - /// Customizes this icon button's appearance. - /// - /// The [style] is only used for Material 3 [IconButton]s. If [ThemeData.useMaterial3] - /// is set to true, [style] is preferred for icon button customization, and any - /// parameters defined in [style] will override the same parameters in [IconButton]. - /// - /// Null by default. - final ButtonStyle? style; - /// An enum value to use to identify this button as a type of /// [StandardComponentType]. final StandardComponentType? standardComponent; diff --git a/packages/flutter/test/material/app_bar_sliver_test.dart b/packages/flutter/test/material/app_bar_sliver_test.dart index cef0f0f5bf..f1de2c04d9 100644 --- a/packages/flutter/test/material/app_bar_sliver_test.dart +++ b/packages/flutter/test/material/app_bar_sliver_test.dart @@ -108,7 +108,11 @@ void main() { await tester.pumpAndSettle(); final Finder collapsedTitle = find.text(title).last; - final Offset backButtonOffset = tester.getTopRight(find.byType(BackButton)); + // Get the offset of the Center widget that wraps the IconButton. + final Offset backButtonOffset = tester.getTopRight(find.ancestor( + of: find.byType(IconButton), + matching: find.byType(Center), + )); final Offset titleOffset = tester.getTopLeft(collapsedTitle); expect(titleOffset.dx, backButtonOffset.dx + titleSpacing); }); diff --git a/packages/flutter/test/material/app_bar_test.dart b/packages/flutter/test/material/app_bar_test.dart index 976ceb0b77..1c2ad30b74 100644 --- a/packages/flutter/test/material/app_bar_test.dart +++ b/packages/flutter/test/material/app_bar_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; @@ -16,6 +17,7 @@ TextStyle? _iconStyle(WidgetTester tester, IconData icon) { ); return iconRichText.text.style; } + void main() { setUp(() { debugResetSemanticsIdCounter(); @@ -2968,6 +2970,203 @@ void main() { }); }); + testWidgets('AppBar.leading size with custom IconButton', (WidgetTester tester) async { + final Key leadingKey = UniqueKey(); + final Key titleKey = UniqueKey(); + const double titleSpacing = 16.0; + final ThemeData theme = ThemeData(); + + await tester.pumpWidget(MaterialApp( + home: Scaffold( + appBar: AppBar( + leading: IconButton( + key: leadingKey, + onPressed: () {}, + icon: const Icon(Icons.menu), + ), + centerTitle: false, + title: Text( + 'Title', + key: titleKey, + ), + ), + ), + )); + + final Finder buttonFinder = find.byType(IconButton); + expect(tester.getSize(buttonFinder), const Size(48.0, 48.0)); + + final TestGesture gesture = await tester.createGesture( + kind: PointerDeviceKind.mouse, + ); + await gesture.addPointer(); + await gesture.moveTo(tester.getCenter(buttonFinder)); + await tester.pumpAndSettle(); + expect( + buttonFinder, + paints + ..rect( + rect: const Rect.fromLTRB(0.0, 0.0, 40.0, 40.0), + color: theme.colorScheme.onSurface.withOpacity(0.08), + ), + ); + + // Get the offset of the Center widget that wraps the IconButton. + final Offset backButtonOffset = tester.getTopRight(find.ancestor( + of: buttonFinder, + matching: find.byType(Center), + )); + final Offset titleOffset = tester.getTopLeft(find.byKey(titleKey)); + expect(titleOffset.dx, backButtonOffset.dx + titleSpacing); + }); + + testWidgets('AppBar.leading size with custom BackButton', (WidgetTester tester) async { + final Key leadingKey = UniqueKey(); + final Key titleKey = UniqueKey(); + const double titleSpacing = 16.0; + final ThemeData theme = ThemeData(); + + await tester.pumpWidget(MaterialApp( + home: Scaffold( + appBar: AppBar( + leading: BackButton( + key: leadingKey, + onPressed: () {}, + ), + centerTitle: false, + title: Text( + 'Title', + key: titleKey, + ), + ), + ), + )); + + final Finder buttonFinder = find.byType(BackButton); + expect(tester.getSize(buttonFinder), const Size(48.0, 48.0)); + + final TestGesture gesture = await tester.createGesture( + kind: PointerDeviceKind.mouse, + ); + await gesture.addPointer(); + await gesture.moveTo(tester.getCenter(buttonFinder)); + await tester.pumpAndSettle(); + expect( + buttonFinder, + paints + ..rect( + rect: const Rect.fromLTRB(0.0, 0.0, 40.0, 40.0), + color: theme.colorScheme.onSurface.withOpacity(0.08), + ), + ); + + // Get the offset of the Center widget that wraps the IconButton. + final Offset backButtonOffset = tester.getTopRight(find.ancestor( + of: buttonFinder, + matching: find.byType(Center), + )); + final Offset titleOffset = tester.getTopLeft(find.byKey(titleKey)); + expect(titleOffset.dx, backButtonOffset.dx + titleSpacing); + }); + + testWidgets('AppBar.leading size with custom CloseButton', (WidgetTester tester) async { + final Key leadingKey = UniqueKey(); + final Key titleKey = UniqueKey(); + const double titleSpacing = 16.0; + final ThemeData theme = ThemeData(); + + await tester.pumpWidget(MaterialApp( + home: Scaffold( + appBar: AppBar( + leading: CloseButton( + key: leadingKey, + onPressed: () {}, + ), + centerTitle: false, + title: Text( + 'Title', + key: titleKey, + ), + ), + ), + )); + + final Finder buttonFinder = find.byType(CloseButton); + expect(tester.getSize(buttonFinder), const Size(48.0, 48.0)); + + final TestGesture gesture = await tester.createGesture( + kind: PointerDeviceKind.mouse, + ); + await gesture.addPointer(); + await gesture.moveTo(tester.getCenter(buttonFinder)); + await tester.pumpAndSettle(); + expect( + buttonFinder, + paints + ..rect( + rect: const Rect.fromLTRB(0.0, 0.0, 40.0, 40.0), + color: theme.colorScheme.onSurface.withOpacity(0.08), + ), + ); + + // Get the offset of the Center widget that wraps the IconButton. + final Offset backButtonOffset = tester.getTopRight(find.ancestor( + of: buttonFinder, + matching: find.byType(Center), + )); + final Offset titleOffset = tester.getTopLeft(find.byKey(titleKey)); + expect(titleOffset.dx, backButtonOffset.dx + titleSpacing); + }); + + testWidgets('AppBar.leading size with custom DrawerButton', (WidgetTester tester) async { + final Key leadingKey = UniqueKey(); + final Key titleKey = UniqueKey(); + const double titleSpacing = 16.0; + final ThemeData theme = ThemeData(); + + await tester.pumpWidget(MaterialApp( + home: Scaffold( + appBar: AppBar( + leading: DrawerButton( + key: leadingKey, + onPressed: () {}, + ), + centerTitle: false, + title: Text( + 'Title', + key: titleKey, + ), + ), + ), + )); + + final Finder buttonFinder = find.byType(DrawerButton); + expect(tester.getSize(buttonFinder), const Size(48.0, 48.0)); + + final TestGesture gesture = await tester.createGesture( + kind: PointerDeviceKind.mouse, + ); + await gesture.addPointer(); + await gesture.moveTo(tester.getCenter(buttonFinder)); + await tester.pumpAndSettle(); + expect( + buttonFinder, + paints + ..rect( + rect: const Rect.fromLTRB(0.0, 0.0, 40.0, 40.0), + color: theme.colorScheme.onSurface.withOpacity(0.08), + ), + ); + + // Get the offset of the Center widget that wraps the IconButton. + final Offset backButtonOffset = tester.getTopRight(find.ancestor( + of: buttonFinder, + matching: find.byType(Center), + )); + final Offset titleOffset = tester.getTopLeft(find.byKey(titleKey)); + expect(titleOffset.dx, backButtonOffset.dx + titleSpacing); + }); + group('Material 2', () { testWidgets('Material2 - AppBar draws a light system bar for a dark background', (WidgetTester tester) async { final ThemeData darkTheme = ThemeData.dark(useMaterial3: false); diff --git a/packages/flutter/test/material/date_range_picker_test.dart b/packages/flutter/test/material/date_range_picker_test.dart index 04c21da155..cbe73bdcc7 100644 --- a/packages/flutter/test/material/date_range_picker_test.dart +++ b/packages/flutter/test/material/date_range_picker_test.dart @@ -127,7 +127,10 @@ void main() { expect(saveText, findsOneWidget); // Test the close button position. - final Offset closeButtonBottomRight = tester.getBottomRight(find.byType(CloseButton)); + final Offset closeButtonBottomRight = tester.getBottomRight(find.ancestor( + of: find.byType(IconButton), + matching: find.byType(Center), + )); final Offset helpTextTopLeft = tester.getTopLeft(helpText); expect(closeButtonBottomRight.dx, 56.0); expect(closeButtonBottomRight.dy, helpTextTopLeft.dy); @@ -1592,6 +1595,30 @@ void main() { await tester.pumpAndSettle(); }); + // This is a regression test for https://github.com/flutter/flutter/issues/154393. + testWidgets('DateRangePicker close button shape should be square', (WidgetTester tester) async { + await preparePicker(tester, (Future range) async { + final ThemeData theme = ThemeData(); + final Finder buttonFinder = find.widgetWithIcon(IconButton, Icons.close); + expect(tester.getSize(buttonFinder), const Size(48.0, 48.0)); + + // Test the close button overlay size is square. + final TestGesture gesture = await tester.createGesture( + kind: PointerDeviceKind.mouse, + ); + await gesture.addPointer(); + await gesture.moveTo(tester.getCenter(buttonFinder)); + await tester.pumpAndSettle(); + expect( + buttonFinder, + paints + ..rect( + rect: const Rect.fromLTRB(0.0, 0.0, 40.0, 40.0), + color: theme.colorScheme.onSurfaceVariant.withOpacity(0.08), + ), + ); + }, useMaterial3: true); + }); group('Material 2', () { // These tests are only relevant for Material 2. Once Material 2