Skip to content

Commit

Permalink
Fix Material 3 AppBar.leading action IconButtons (flutter#154512)
Browse files Browse the repository at this point in the history 
Fixes [`AppBar` back button focus/hover circle should not fill up whole height](flutter#141361)
Fixes [[Material 3] Date Range Picker close button has incorrect shape](flutter#154393)

This updates the leading condition added in flutter#110722

### Code sample

<details>
<summary>expand to view the code sample</summary> 

```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: <Widget>[
                  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: <Widget>[
                  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'),
                  ),
                ],
              ),
            ],
          ),
        ),
      ),
    );
  }
}
```

</details>

### Before

<img width="912" alt="Screenshot 2024-09-04 at 12 38 05" src="https://github.com/user-attachments/assets/25a6893c-89c9-4b45-a5bb-8da0eee71cd2">

### After

<img width="912" alt="Screenshot 2024-09-04 at 12 38 28" src="https://github.com/user-attachments/assets/49727183-568c-412e-9fa1-1eefd0cd87a7">
  • Loading branch information
@TahaTesser
TahaTesser authored Sep 6, 2024
1 parent 6ad6641 commit 755cf0b
Show file tree
Hide file tree
Showing 4 changed files with 237 additions and 32 deletions.
35 changes: 5 additions & 30 deletions packages/flutter/lib/src/material/action_buttons.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion packages/flutter/test/material/app_bar_sliver_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
199 changes: 199 additions & 0 deletions packages/flutter/test/material/app_bar_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -16,6 +17,7 @@ TextStyle? _iconStyle(WidgetTester tester, IconData icon) {
);
return iconRichText.text.style;
}

void main() {
setUp(() {
debugResetSemanticsIdCounter();
Expand Down Expand Up @@ -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);
Expand Down
29 changes: 28 additions & 1 deletion packages/flutter/test/material/date_range_picker_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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<DateTimeRange?> 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
Expand Down

0 comments on commit 755cf0b

Please sign in to comment.