From eaf74bdd11038bc25486e52b34c811a7da562e77 Mon Sep 17 00:00:00 2001 From: Bruno Leroux Date: Fri, 13 Dec 2024 13:06:28 +0100 Subject: [PATCH] Fix NavigationDrawerDestination backgroundColor obscures interactions (#160239) ## Description This PR fixes `NavigationDrawerDestination.backgroundColor` obscuring ink well splashes and overlay. Before this PR the destination background color was renderer in a `ColoredBox` which hides the ink well effects. This PR replaces the `ColorsBox` with an `Ink`. ## Related Issue Fixes [NavigationDrawerDestination backgroundColor obscures interaction states](https://github.com/flutter/flutter/issues/160109) ## Tests Updates 1 test. --- .../lib/src/material/navigation_drawer.dart | 16 ++-- .../test/material/navigation_drawer_test.dart | 91 ++++++++++++------- 2 files changed, 64 insertions(+), 43 deletions(-) diff --git a/packages/flutter/lib/src/material/navigation_drawer.dart b/packages/flutter/lib/src/material/navigation_drawer.dart index 3cb24aecf0..ae07566010 100644 --- a/packages/flutter/lib/src/material/navigation_drawer.dart +++ b/packages/flutter/lib/src/material/navigation_drawer.dart @@ -10,6 +10,7 @@ import 'package:flutter/widgets.dart'; import 'color_scheme.dart'; import 'colors.dart'; import 'drawer.dart'; +import 'ink_decoration.dart'; import 'ink_well.dart'; import 'material.dart'; import 'material_localizations.dart'; @@ -196,9 +197,12 @@ class NavigationDrawerDestination extends StatelessWidget { this.enabled = true, }); - /// Sets the color of the destination. + /// The background color of the destination. /// - /// If this is null, then [NavigationDrawerThemeData.backgroundColor]. + /// If this is null, no background is set and [NavigationDrawer.backgroundColor] will be visible. + /// + /// This is the background color of the whole rectangular area behind the drawer destination. + /// To customize only the indicator color consider using [NavigationDrawer.indicatorColor]. final Color? backgroundColor; /// The [Widget] (usually an [Icon]) that's displayed for this @@ -339,9 +343,6 @@ class _NavigationDestinationBuilder extends StatelessWidget { /// Defaults to true. final bool enabled; - /// Sets the color of navigation destination. - /// - /// If this is null, then [NavigationDrawerThemeData.backgroundColor] is used. final Color? backgroundColor; @override @@ -386,9 +387,8 @@ class _NavigationDestinationBuilder extends StatelessWidget { ), ); - final Color? color = backgroundColor ?? navigationDrawerTheme.backgroundColor; - if (color != null) { - return ColoredBox(color: color, child: destination); + if (backgroundColor != null) { + return Ink(color: backgroundColor, child: destination); } return destination; } diff --git a/packages/flutter/test/material/navigation_drawer_test.dart b/packages/flutter/test/material/navigation_drawer_test.dart index 494d74dd20..723986cab5 100644 --- a/packages/flutter/test/material/navigation_drawer_test.dart +++ b/packages/flutter/test/material/navigation_drawer_test.dart @@ -82,45 +82,66 @@ void main() { expect(_getMaterial(tester).color, equals(color)); }); - testWidgets('NavigationDrawer can update destination background color', - (WidgetTester tester) async { - const Color color = Colors.yellow; - final GlobalKey scaffoldKey = GlobalKey(); - final ThemeData theme = ThemeData(); + testWidgets( + 'NavigationDestinationDrawer background color is customizable', + (WidgetTester tester) async { + const Color color = Colors.yellow; + final GlobalKey scaffoldKey = GlobalKey(); + final ThemeData theme = ThemeData(); - await tester.pumpWidget( - _buildWidget( - scaffoldKey, - NavigationDrawer( - children: [ - Text('Headline', style: theme.textTheme.bodyLarge), - NavigationDrawerDestination( - icon: Icon(Icons.ac_unit, color: theme.iconTheme.color), - label: Text('AC', style: theme.textTheme.bodySmall), - backgroundColor: color, - ), - NavigationDrawerDestination( - icon: Icon(Icons.access_alarm, color: theme.iconTheme.color), - label: Text('Alarm', style: theme.textTheme.bodySmall), - backgroundColor: color, - ), - ], - onDestinationSelected: (int i) {}, + await tester.pumpWidget( + _buildWidget( + scaffoldKey, + NavigationDrawer( + children: [ + Text('Headline', style: theme.textTheme.bodyLarge), + NavigationDrawerDestination( + icon: Icon(Icons.ac_unit, color: theme.iconTheme.color), + label: Text('AC', style: theme.textTheme.bodySmall), + ), + NavigationDrawerDestination( + icon: Icon(Icons.access_alarm, color: theme.iconTheme.color), + label: Text('Alarm', style: theme.textTheme.bodySmall), + backgroundColor: color, + ), + ], + onDestinationSelected: (int i) {}, + ), ), - ), - ); + ); - scaffoldKey.currentState!.openDrawer(); - await tester.pump(const Duration(seconds: 1)); // animation done - final ColoredBox destinationColor = tester.firstWidget( - find.descendant( - of: find.byType(NavigationDrawerDestination), - matching: find.byType(ColoredBox), - ), - ); + Finder findDestinationInk(String label) { + return find.descendant( + of: find.ancestor( + of: find.text(label), + matching: find.byType(NavigationDrawerDestination), + ), + matching: find.byType(Ink), + ); + } - expect(destinationColor.color, equals(color)); - }); + scaffoldKey.currentState!.openDrawer(); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); // Animation done. + + // Destination with no custom background color. + await tester.tap(find.text('AC')); + await tester.pump(); + + expect(findDestinationInk('AC'), findsNothing); + + // Destination with a custom background color. + await tester.tap(find.byIcon(Icons.access_alarm)); + await tester.pump(); + + // A Material is added with the custom color. + expect(findDestinationInk('Alarm'), findsOne); + final BoxDecoration destinationDecoration = tester.firstWidget( + findDestinationInk('Alarm'), + ).decoration! as BoxDecoration; + expect(destinationDecoration.color, color); + }, + ); testWidgets('NavigationDrawer can update elevation', (WidgetTester tester) async {