Unverified Commit 6e2bbf94 authored by Bruno Leroux's avatar Bruno Leroux Committed by GitHub

Fix Animations in NavigationDestination icons don't work (#123400)

parent 94f8772c
...@@ -414,7 +414,7 @@ class NavigationDestination extends StatelessWidget { ...@@ -414,7 +414,7 @@ class NavigationDestination extends StatelessWidget {
/// animation value of 0 is unselected and 1 is selected. /// animation value of 0 is unselected and 1 is selected.
/// ///
/// See [NavigationDestination] for an example. /// See [NavigationDestination] for an example.
class _NavigationDestinationBuilder extends StatelessWidget { class _NavigationDestinationBuilder extends StatefulWidget {
/// Builds a destination (icon + label) to use in a Material 3 [NavigationBar]. /// Builds a destination (icon + label) to use in a Material 3 [NavigationBar].
const _NavigationDestinationBuilder({ const _NavigationDestinationBuilder({
required this.buildIcon, required this.buildIcon,
...@@ -459,31 +459,34 @@ class _NavigationDestinationBuilder extends StatelessWidget { ...@@ -459,31 +459,34 @@ class _NavigationDestinationBuilder extends StatelessWidget {
/// Defaults to null, in which case the [label] text will be used. /// Defaults to null, in which case the [label] text will be used.
final String? tooltip; final String? tooltip;
@override
State<_NavigationDestinationBuilder> createState() => _NavigationDestinationBuilderState();
}
class _NavigationDestinationBuilderState extends State<_NavigationDestinationBuilder> {
final GlobalKey iconKey = GlobalKey();
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
final _NavigationDestinationInfo info = _NavigationDestinationInfo.of(context); final _NavigationDestinationInfo info = _NavigationDestinationInfo.of(context);
final NavigationBarThemeData navigationBarTheme = NavigationBarTheme.of(context); final NavigationBarThemeData navigationBarTheme = NavigationBarTheme.of(context);
final NavigationBarThemeData defaults = _defaultsFor(context); final NavigationBarThemeData defaults = _defaultsFor(context);
final GlobalKey labelKey = GlobalKey();
final bool selected = info.selectedIndex == info.index;
return _NavigationBarDestinationSemantics( return _NavigationBarDestinationSemantics(
child: _NavigationBarDestinationTooltip( child: _NavigationBarDestinationTooltip(
message: tooltip ?? label, message: widget.tooltip ?? widget.label,
child: _IndicatorInkWell( child: _IndicatorInkWell(
key: UniqueKey(), iconKey: iconKey,
labelKey: labelKey,
labelBehavior: info.labelBehavior, labelBehavior: info.labelBehavior,
selected: selected,
customBorder: navigationBarTheme.indicatorShape ?? defaults.indicatorShape, customBorder: navigationBarTheme.indicatorShape ?? defaults.indicatorShape,
onTap: info.onTap, onTap: info.onTap,
child: Row( child: Row(
children: <Widget>[ children: <Widget>[
Expanded( Expanded(
child: _NavigationBarDestinationLayout( child: _NavigationBarDestinationLayout(
icon: buildIcon(context), icon: widget.buildIcon(context),
labelKey: labelKey, iconKey: iconKey,
label: buildLabel(context), label: widget.buildLabel(context),
), ),
), ),
], ],
...@@ -496,10 +499,8 @@ class _NavigationDestinationBuilder extends StatelessWidget { ...@@ -496,10 +499,8 @@ class _NavigationDestinationBuilder extends StatelessWidget {
class _IndicatorInkWell extends InkResponse { class _IndicatorInkWell extends InkResponse {
const _IndicatorInkWell({ const _IndicatorInkWell({
super.key, required this.iconKey,
required this.labelKey,
required this.labelBehavior, required this.labelBehavior,
required this.selected,
super.customBorder, super.customBorder,
super.onTap, super.onTap,
super.child, super.child,
...@@ -508,32 +509,15 @@ class _IndicatorInkWell extends InkResponse { ...@@ -508,32 +509,15 @@ class _IndicatorInkWell extends InkResponse {
highlightColor: Colors.transparent, highlightColor: Colors.transparent,
); );
final GlobalKey labelKey; final GlobalKey iconKey;
final NavigationDestinationLabelBehavior labelBehavior; final NavigationDestinationLabelBehavior labelBehavior;
final bool selected;
@override @override
RectCallback? getRectCallback(RenderBox referenceBox) { RectCallback? getRectCallback(RenderBox referenceBox) {
final RenderBox labelBox = labelKey.currentContext!.findRenderObject()! as RenderBox;
final Rect labelRect = labelBox.localToGlobal(Offset.zero) & labelBox.size;
final double labelPadding;
switch (labelBehavior) {
case NavigationDestinationLabelBehavior.alwaysShow:
labelPadding = labelRect.height / 2;
case NavigationDestinationLabelBehavior.onlyShowSelected:
labelPadding = selected ? labelRect.height / 2 : 0;
case NavigationDestinationLabelBehavior.alwaysHide:
labelPadding = 0;
}
final double indicatorOffsetX = referenceBox.size.width / 2;
final double indicatorOffsetY = referenceBox.size.height / 2 - labelPadding;
return () { return () {
return Rect.fromCenter( final RenderBox iconBox = iconKey.currentContext!.findRenderObject()! as RenderBox;
center: Offset(indicatorOffsetX, indicatorOffsetY), final Rect iconRect = iconBox.localToGlobal(Offset.zero) & iconBox.size;
width: _kIndicatorWidth, return referenceBox.globalToLocal(iconRect.topLeft) & iconBox.size;
height: _kIndicatorHeight,
);
}; };
} }
} }
...@@ -774,7 +758,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget { ...@@ -774,7 +758,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget {
/// 3 [NavigationBar]. /// 3 [NavigationBar].
const _NavigationBarDestinationLayout({ const _NavigationBarDestinationLayout({
required this.icon, required this.icon,
required this.labelKey, required this.iconKey,
required this.label, required this.label,
}); });
...@@ -783,10 +767,10 @@ class _NavigationBarDestinationLayout extends StatelessWidget { ...@@ -783,10 +767,10 @@ class _NavigationBarDestinationLayout extends StatelessWidget {
/// See [NavigationDestination.icon]. /// See [NavigationDestination.icon].
final Widget icon; final Widget icon;
/// The global key for the label of this destination. /// The global key for the icon of this destination.
/// ///
/// This is used to determine the position of the label relative to the icon. /// This is used to determine the position of the icon.
final GlobalKey labelKey; final GlobalKey iconKey;
/// The label widget that sits below the icon. /// The label widget that sits below the icon.
/// ///
...@@ -796,7 +780,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget { ...@@ -796,7 +780,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget {
/// See [NavigationDestination.label]. /// See [NavigationDestination.label].
final Widget label; final Widget label;
static final Key _iconKey = UniqueKey(); static final Key _labelKey = UniqueKey();
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
...@@ -810,7 +794,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget { ...@@ -810,7 +794,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget {
LayoutId( LayoutId(
id: _NavigationDestinationLayoutDelegate.iconId, id: _NavigationDestinationLayoutDelegate.iconId,
child: RepaintBoundary( child: RepaintBoundary(
key: _iconKey, key: iconKey,
child: icon, child: icon,
), ),
), ),
...@@ -820,7 +804,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget { ...@@ -820,7 +804,7 @@ class _NavigationBarDestinationLayout extends StatelessWidget {
alwaysIncludeSemantics: true, alwaysIncludeSemantics: true,
opacity: animation, opacity: animation,
child: RepaintBoundary( child: RepaintBoundary(
key: labelKey, key: _labelKey,
child: label, child: label,
), ),
), ),
......
...@@ -7,6 +7,8 @@ ...@@ -7,6 +7,8 @@
@Tags(<String>['reduced-test-set']) @Tags(<String>['reduced-test-set'])
library; library;
import 'dart:math';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
...@@ -1217,6 +1219,52 @@ void main() { ...@@ -1217,6 +1219,52 @@ void main() {
await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_onlyShowSelected_unselected_m2.png')); await expectLater(find.byType(NavigationBar), matchesGoldenFile('indicator_onlyShowSelected_unselected_m2.png'));
}); });
testWidgets('Destination icon does not rebuild when tapped', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/122811.
Widget buildNavigationBar() {
return MaterialApp(
home: Scaffold(
bottomNavigationBar: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
int selectedIndex = 0;
return NavigationBar(
selectedIndex: selectedIndex,
destinations: const <Widget>[
NavigationDestination(
icon: IconWithRandomColor(icon: Icons.ac_unit),
label: 'AC',
),
NavigationDestination(
icon: IconWithRandomColor(icon: Icons.access_alarm),
label: 'Alarm',
),
],
onDestinationSelected: (int i) {
setState(() {
selectedIndex = i;
});
},
);
}
),
),
);
}
await tester.pumpWidget(buildNavigationBar());
Icon icon = tester.widget<Icon>(find.byType(Icon).last);
final Color initialColor = icon.color!;
// Trigger a rebuild.
await tester.tap(find.text('Alarm'));
await tester.pumpAndSettle();
// Icon color should be the same as before the rebuild.
icon = tester.widget<Icon>(find.byType(Icon).last);
expect(icon.color, initialColor);
});
}); });
} }
...@@ -1245,3 +1293,15 @@ ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) { ...@@ -1245,3 +1293,15 @@ ShapeDecoration? _getIndicatorDecoration(WidgetTester tester) {
), ),
).decoration as ShapeDecoration?; ).decoration as ShapeDecoration?;
} }
class IconWithRandomColor extends StatelessWidget {
const IconWithRandomColor({super.key, required this.icon});
final IconData icon;
@override
Widget build(BuildContext context) {
final Color randomColor = Color((Random().nextDouble() * 0xFFFFFF).toInt()).withOpacity(1.0);
return Icon(icon, color: randomColor);
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment