Skip to content

Commit

Permalink
Fix AppBar back button doesn't navigate back when using `TooltipTri…
Browse files Browse the repository at this point in the history 
…ggerMode.tap` in the `TooltipTheme` (flutter#155822)

Fixes [AppBar BackButton Tooltip interferes with clicking button when TooltipThemeData set to TooltipTriggerMode.tap](flutter#152315)

### Description
`TooltipTheme` with `triggerMode: TooltipTriggerMode.tap` prevents user from navigating back when tapping on the back button. `triggerMode: TooltipTriggerMode.tap` shouldn't interrupted the back button navigation back callback.

### 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(
      debugShowCheckedModeBanner: false,
      theme: ThemeData(
        tooltipTheme:
            const TooltipThemeData(triggerMode: TooltipTriggerMode.tap),
      ),
      home: Scaffold(
        appBar: AppBar(
          title: const Text('Sample'),
        ),
        body: Builder(builder: (context) {
          return Center(
            child: ElevatedButton(
              onPressed: () {
                Navigator.push(
                  context,
                  MaterialPageRoute(
                    builder: (context) => const SecondScreen(),
                  ),
                );
              },
              child: const Text('Go to Second Screen'),
            ),
          );
        }),
      ),
    );
  }
}

class SecondScreen extends StatelessWidget {
  const SecondScreen({super.key});

  @OverRide
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('Second Screen'),
      ),
      body: const Center(
        child: Tooltip(
          message: 'This is the second screen',
          child: Text('Tap to show tooltip'),
        ),
      ),
    );
  }
}
```

</details>

### Before

https://github.com/user-attachments/assets/ed2020f0-aef8-40f9-be05-55ff80e42e5d

### After

https://github.com/user-attachments/assets/8d65f9f8-d15d-453f-955f-e5d92f8a04e8
  • Loading branch information
@TahaTesser
TahaTesser authored Oct 15, 2024
1 parent 5d06db6 commit 8ea38a7
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 61 deletions.
86 changes: 43 additions & 43 deletions packages/flutter/lib/src/material/button_style_button.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ class _ButtonStyleState extends State<ButtonStyleButton> with TickerProviderStat
elevation = resolvedElevation;
backgroundColor = resolvedBackgroundColor;

Widget effectiveChild = Padding(
Widget result = Padding(
padding: padding,
child: Align(
alignment: resolvedAlignment!,
Expand All @@ -494,54 +494,40 @@ class _ButtonStyleState extends State<ButtonStyleButton> with TickerProviderStat
),
);
if (resolvedBackgroundBuilder != null) {
effectiveChild = resolvedBackgroundBuilder(context, statesController.value, effectiveChild);
result = resolvedBackgroundBuilder(context, statesController.value, result);
}

result = InkWell(
onTap: widget.onPressed,
onLongPress: widget.onLongPress,
onHover: widget.onHover,
mouseCursor: mouseCursor,
enableFeedback: resolvedEnableFeedback,
focusNode: widget.focusNode,
canRequestFocus: widget.enabled,
onFocusChange: widget.onFocusChange,
autofocus: widget.autofocus,
splashFactory: resolvedSplashFactory,
overlayColor: overlayColor,
highlightColor: Colors.transparent,
customBorder: resolvedShape!.copyWith(side: resolvedSide),
statesController: statesController,
child: IconTheme.merge(
data: IconThemeData(
color: resolvedIconColor ?? resolvedForegroundColor,
size: resolvedIconSize,
),
child: result,
),
);

if (widget.tooltip != null) {
effectiveChild = Tooltip(
result = Tooltip(
message: widget.tooltip,
child: effectiveChild,
child: result,
);
}

final Widget result = ConstrainedBox(
constraints: effectiveConstraints,
child: Material(
elevation: resolvedElevation!,
textStyle: resolvedTextStyle?.copyWith(color: resolvedForegroundColor),
shape: resolvedShape!.copyWith(side: resolvedSide),
color: resolvedBackgroundColor,
shadowColor: resolvedShadowColor,
surfaceTintColor: resolvedSurfaceTintColor,
type: resolvedBackgroundColor == null ? MaterialType.transparency : MaterialType.button,
animationDuration: resolvedAnimationDuration,
clipBehavior: effectiveClipBehavior,
child: InkWell(
onTap: widget.onPressed,
onLongPress: widget.onLongPress,
onHover: widget.onHover,
mouseCursor: mouseCursor,
enableFeedback: resolvedEnableFeedback,
focusNode: widget.focusNode,
canRequestFocus: widget.enabled,
onFocusChange: widget.onFocusChange,
autofocus: widget.autofocus,
splashFactory: resolvedSplashFactory,
overlayColor: overlayColor,
highlightColor: Colors.transparent,
customBorder: resolvedShape.copyWith(side: resolvedSide),
statesController: statesController,
child: IconTheme.merge(
data: IconThemeData(
color: resolvedIconColor ?? resolvedForegroundColor,
size: resolvedIconSize,
),
child: effectiveChild,
),
),
),
);

final Size minSize;
switch (resolvedTapTargetSize!) {
case MaterialTapTargetSize.padded:
Expand All @@ -561,7 +547,21 @@ class _ButtonStyleState extends State<ButtonStyleButton> with TickerProviderStat
enabled: widget.enabled,
child: _InputPadding(
minSize: minSize,
child: result,
child: ConstrainedBox(
constraints: effectiveConstraints,
child: Material(
elevation: resolvedElevation!,
textStyle: resolvedTextStyle?.copyWith(color: resolvedForegroundColor),
shape: resolvedShape.copyWith(side: resolvedSide),
color: resolvedBackgroundColor,
shadowColor: resolvedShadowColor,
surfaceTintColor: resolvedSurfaceTintColor,
type: resolvedBackgroundColor == null ? MaterialType.transparency : MaterialType.button,
animationDuration: resolvedAnimationDuration,
clipBehavior: effectiveClipBehavior,
child: result,
),
),
),
);
}
Expand Down
38 changes: 20 additions & 18 deletions packages/flutter/lib/src/material/icon_button.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,25 @@ class IconButton extends StatelessWidget {
),
);

result = InkResponse(
focusNode: focusNode,
autofocus: autofocus,
canRequestFocus: onPressed != null,
onTap: onPressed,
mouseCursor: mouseCursor ?? (onPressed == null ? SystemMouseCursors.basic : SystemMouseCursors.click),
enableFeedback: effectiveEnableFeedback,
focusColor: focusColor ?? theme.focusColor,
hoverColor: hoverColor ?? theme.hoverColor,
highlightColor: highlightColor ?? theme.highlightColor,
splashColor: splashColor ?? theme.splashColor,
radius: splashRadius ?? math.max(
Material.defaultSplashRadius,
(effectiveIconSize + math.min(effectivePadding.horizontal, effectivePadding.vertical)) * 0.7,
// x 0.5 for diameter -> radius and + 40% overflow derived from other Material apps.
),
child: result,
);

if (tooltip != null) {
result = Tooltip(
message: tooltip,
Expand All @@ -788,24 +807,7 @@ class IconButton extends StatelessWidget {
return Semantics(
button: true,
enabled: onPressed != null,
child: InkResponse(
focusNode: focusNode,
autofocus: autofocus,
canRequestFocus: onPressed != null,
onTap: onPressed,
mouseCursor: mouseCursor ?? (onPressed == null ? SystemMouseCursors.basic : SystemMouseCursors.click),
enableFeedback: effectiveEnableFeedback,
focusColor: focusColor ?? theme.focusColor,
hoverColor: hoverColor ?? theme.hoverColor,
highlightColor: highlightColor ?? theme.highlightColor,
splashColor: splashColor ?? theme.splashColor,
radius: splashRadius ?? math.max(
Material.defaultSplashRadius,
(effectiveIconSize + math.min(effectivePadding.horizontal, effectivePadding.vertical)) * 0.7,
// x 0.5 for diameter -> radius and + 40% overflow derived from other Material apps.
),
child: result,
),
child: result,
);
}

Expand Down
87 changes: 87 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 @@ -3167,6 +3167,93 @@ void main() {
expect(titleOffset.dx, backButtonOffset.dx + titleSpacing);
});

// Regression test for https://github.com/flutter/flutter/issues/152315
testWidgets('AppBar back button navigates to previous page on tap with TooltipTriggerMode.tap', (WidgetTester tester) async {
await tester.pumpWidget(MaterialApp(
theme: ThemeData(tooltipTheme: const TooltipThemeData(triggerMode: TooltipTriggerMode.tap)),
home: Scaffold(
body: Center(
child: Builder(
builder: (BuildContext context) {
return ElevatedButton(
onPressed: () {
Navigator.push(
context,
MaterialPageRoute<void>(
builder: (_) => Scaffold(
appBar: AppBar(
title: const Text('Second Screen'),
),
),
),
);
},
child: const Text('Go to second screen'),
);
}
),
),
),
));

expect(find.text('Second Screen'), findsNothing);

await tester.tap(find.text('Go to second screen'));
await tester.pumpAndSettle();

expect(find.text('Second Screen'), findsOneWidget);

await tester.tap(find.byType(BackButton));
await tester.pumpAndSettle();

expect(find.text('Second Screen'), findsNothing);
});

// Regression test for https://github.com/flutter/flutter/issues/152315
testWidgets('Material2 - AppBar back button navigates to previous page on tap with TooltipTriggerMode.tap', (WidgetTester tester) async {
await tester.pumpWidget(MaterialApp(
theme: ThemeData(
useMaterial3: false,
tooltipTheme: const TooltipThemeData(triggerMode: TooltipTriggerMode.tap),
),
home: Scaffold(
body: Center(
child: Builder(
builder: (BuildContext context) {
return ElevatedButton(
onPressed: () {
Navigator.push(
context,
MaterialPageRoute<void>(
builder: (_) => Scaffold(
appBar: AppBar(
title: const Text('Second Screen'),
),
),
),
);
},
child: const Text('Go to second screen'),
);
}
),
),
),
));

expect(find.text('Second Screen'), findsNothing);

await tester.tap(find.text('Go to second screen'));
await tester.pumpAndSettle();

expect(find.text('Second Screen'), findsOneWidget);

await tester.tap(find.byType(BackButton));
await tester.pumpAndSettle();

expect(find.text('Second Screen'), findsNothing);
});

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

0 comments on commit 8ea38a7

Please sign in to comment.