firka/flutter
firka/flutter
4
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix memory leaks in navigation rail (#146988)

Browse Source
This commit is contained in:
Valentin Vignal 2024-04-18 23:03:19 +08:00 committed by GitHub
parent fb110b98da
commit cbf35b4e85
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 104 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

163
packages/flutter/lib/src/material/navigation_rail.dart
View File

@ -346,7 +346,7 @@ class _NavigationRailState extends State<NavigationRail> with TickerProviderStat
late List<AnimationController> _destinationControllers; late List<AnimationController> _destinationControllers;
late List<Animation<double>> _destinationAnimations; late List<Animation<double>> _destinationAnimations;
late AnimationController _extendedController; late AnimationController _extendedController;
late Animation<double> _extendedAnimation; late CurvedAnimation _extendedAnimation;
@override @override
void initState() { void initState() {
@ -488,6 +488,8 @@ class _NavigationRailState extends State<NavigationRail> with TickerProviderStat
controller.dispose(); controller.dispose();
} }
_extendedController.dispose(); _extendedController.dispose();
_extendedAnimation.dispose();
} }
void _initControllers() { void _initControllers() {
@ -528,8 +530,8 @@ class _NavigationRailState extends State<NavigationRail> with TickerProviderStat
} }
} }
class _RailDestination extends StatelessWidget { class _RailDestination extends StatefulWidget {
_RailDestination({ const _RailDestination({
required this.minWidth, required this.minWidth,
required this.minExtendedWidth, required this.minExtendedWidth,
required this.icon, required this.icon,
@ -547,11 +549,7 @@ class _RailDestination extends StatelessWidget {
this.indicatorColor, this.indicatorColor,
this.indicatorShape, this.indicatorShape,
this.disabled = false, this.disabled = false,
}) : _positionAnimation = CurvedAnimation( });
parent: ReverseAnimation(destinationAnimation),
curve: Curves.easeInOut,
reverseCurve: Curves.easeInOut.flipped,
);
final double minWidth; final double minWidth;
final double minExtendedWidth; final double minExtendedWidth;
@ -571,33 +569,70 @@ class _RailDestination extends StatelessWidget {
final ShapeBorder? indicatorShape; final ShapeBorder? indicatorShape;
final bool disabled; final bool disabled;
final Animation<double> _positionAnimation;
@override
State<_RailDestination> createState() => _RailDestinationState();
}
class _RailDestinationState extends State<_RailDestination> {
late CurvedAnimation _positionAnimation;
@override
void initState() {
super.initState();
_setPositionAnimation();
}
@override
void didUpdateWidget(_RailDestination oldWidget) {
super.didUpdateWidget(oldWidget);
if (widget.destinationAnimation != oldWidget.destinationAnimation) {
_positionAnimation.dispose();
_setPositionAnimation();
}
}
void _setPositionAnimation() {
_positionAnimation = CurvedAnimation(
parent: ReverseAnimation(widget.destinationAnimation),
curve: Curves.easeInOut,
reverseCurve: Curves.easeInOut.flipped,
);
}
@override
void dispose() {
_positionAnimation.dispose();
super.dispose();
}
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
assert( assert(
useIndicator || indicatorColor == null, widget.useIndicator || widget.indicatorColor == null,
'[NavigationRail.indicatorColor] does not have an effect when [NavigationRail.useIndicator] is false', '[NavigationRail.indicatorColor] does not have an effect when [NavigationRail.useIndicator] is false',
); );
final ThemeData theme = Theme.of(context); final ThemeData theme = Theme.of(context);
final TextDirection textDirection = Directionality.of(context); final TextDirection textDirection = Directionality.of(context);
final bool material3 = theme.useMaterial3; final bool material3 = theme.useMaterial3;
final EdgeInsets destinationPadding = (padding ?? EdgeInsets.zero).resolve(textDirection); final EdgeInsets destinationPadding = (widget.padding ?? EdgeInsets.zero).resolve(textDirection);
Offset indicatorOffset; Offset indicatorOffset;
bool applyXOffset = false; bool applyXOffset = false;
final Widget themedIcon = IconTheme( final Widget themedIcon = IconTheme(
data: disabled data: widget.disabled
? iconTheme.copyWith(color: theme.colorScheme.onSurface.withOpacity(0.38)) ? widget.iconTheme.copyWith(color: theme.colorScheme.onSurface.withOpacity(0.38))
: iconTheme, : widget.iconTheme,
child: icon, child: widget.icon,
); );
final Widget styledLabel = DefaultTextStyle( final Widget styledLabel = DefaultTextStyle(
style: disabled style: widget.disabled
? labelTextStyle.copyWith(color: theme.colorScheme.onSurface.withOpacity(0.38)) ? widget.labelTextStyle.copyWith(color: theme.colorScheme.onSurface.withOpacity(0.38))
: labelTextStyle, : widget.labelTextStyle,
child: label, child: widget.label,
); );
Widget content; Widget content;
@ -605,30 +640,30 @@ class _RailDestination extends StatelessWidget {
// The indicator height is fixed and equal to _kIndicatorHeight. // The indicator height is fixed and equal to _kIndicatorHeight.
// When the icon height is larger than the indicator height the indicator // When the icon height is larger than the indicator height the indicator
// vertical offset is used to vertically center the indicator. // vertical offset is used to vertically center the indicator.
final bool isLargeIconSize = iconTheme.size != null && iconTheme.size! > _kIndicatorHeight; final bool isLargeIconSize = widget.iconTheme.size != null && widget.iconTheme.size! > _kIndicatorHeight;
final double indicatorVerticalOffset = isLargeIconSize ? (iconTheme.size! - _kIndicatorHeight) / 2 : 0; final double indicatorVerticalOffset = isLargeIconSize ? (widget.iconTheme.size! - _kIndicatorHeight) / 2 : 0;
switch (labelType) { switch (widget.labelType) {
case NavigationRailLabelType.none: case NavigationRailLabelType.none:
// Split the destination spacing across the top and bottom to keep the icon centered. // Split the destination spacing across the top and bottom to keep the icon centered.
final Widget? spacing = material3 ? const SizedBox(height: _verticalDestinationSpacingM3 / 2) : null; final Widget? spacing = material3 ? const SizedBox(height: _verticalDestinationSpacingM3 / 2) : null;
indicatorOffset = Offset( indicatorOffset = Offset(
minWidth / 2 + destinationPadding.left, widget.minWidth / 2 + destinationPadding.left,
_verticalDestinationSpacingM3 / 2 + destinationPadding.top + indicatorVerticalOffset, _verticalDestinationSpacingM3 / 2 + destinationPadding.top + indicatorVerticalOffset,
); );
final Widget iconPart = Column( final Widget iconPart = Column(
children: <Widget>[ children: <Widget>[
if (spacing != null) spacing, if (spacing != null) spacing,
SizedBox( SizedBox(
width: minWidth, width: widget.minWidth,
height: material3 ? null : minWidth, height: material3 ? null : widget.minWidth,
child: Center( child: Center(
child: _AddIndicator( child: _AddIndicator(
addIndicator: useIndicator, addIndicator: widget.useIndicator,
indicatorColor: indicatorColor, indicatorColor: widget.indicatorColor,
indicatorShape: indicatorShape, indicatorShape: widget.indicatorShape,
isCircular: !material3, isCircular: !material3,
indicatorAnimation: destinationAnimation, indicatorAnimation: widget.destinationAnimation,
child: themedIcon, child: themedIcon,
), ),
), ),
@ -636,9 +671,9 @@ class _RailDestination extends StatelessWidget {
if (spacing != null) spacing, if (spacing != null) spacing,
], ],
); );
if (extendedTransitionAnimation.value == 0) { if (widget.extendedTransitionAnimation.value == 0) {
content = Padding( content = Padding(
padding: padding ?? EdgeInsets.zero, padding: widget.padding ?? EdgeInsets.zero,
child: Stack( child: Stack(
children: <Widget>[ children: <Widget>[
iconPart, iconPart,
@ -646,20 +681,20 @@ class _RailDestination extends StatelessWidget {
SizedBox.shrink( SizedBox.shrink(
child: Visibility.maintain( child: Visibility.maintain(
visible: false, visible: false,
child: label, child: widget.label,
), ),
), ),
], ],
), ),
); );
} else { } else {
final Animation<double> labelFadeAnimation = extendedTransitionAnimation.drive(CurveTween(curve: const Interval(0.0, 0.25))); final Animation<double> labelFadeAnimation = widget.extendedTransitionAnimation.drive(CurveTween(curve: const Interval(0.0, 0.25)));
applyXOffset = true; applyXOffset = true;
content = Padding( content = Padding(
padding: padding ?? EdgeInsets.zero, padding: widget.padding ?? EdgeInsets.zero,
child: ConstrainedBox( child: ConstrainedBox(
constraints: BoxConstraints( constraints: BoxConstraints(
minWidth: lerpDouble(minWidth, minExtendedWidth, extendedTransitionAnimation.value)!, minWidth: lerpDouble(widget.minWidth, widget.minExtendedWidth, widget.extendedTransitionAnimation.value)!,
), ),
child: ClipRect( child: ClipRect(
child: Row( child: Row(
@ -667,7 +702,7 @@ class _RailDestination extends StatelessWidget {
iconPart, iconPart,
Align( Align(
heightFactor: 1.0, heightFactor: 1.0,
widthFactor: extendedTransitionAnimation.value, widthFactor: widget.extendedTransitionAnimation.value,
alignment: AlignmentDirectional.centerStart, alignment: AlignmentDirectional.centerStart,
child: FadeTransition( child: FadeTransition(
alwaysIncludeSemantics: true, alwaysIncludeSemantics: true,
@ -675,7 +710,7 @@ class _RailDestination extends StatelessWidget {
child: styledLabel, child: styledLabel,
), ),
), ),
SizedBox(width: _horizontalDestinationPadding * extendedTransitionAnimation.value), SizedBox(width: _horizontalDestinationPadding * widget.extendedTransitionAnimation.value),
], ],
), ),
), ),
@ -685,30 +720,30 @@ class _RailDestination extends StatelessWidget {
case NavigationRailLabelType.selected: case NavigationRailLabelType.selected:
final double appearingAnimationValue = 1 - _positionAnimation.value; final double appearingAnimationValue = 1 - _positionAnimation.value;
final double verticalPadding = lerpDouble(_verticalDestinationPaddingNoLabel, _verticalDestinationPaddingWithLabel, appearingAnimationValue)!; final double verticalPadding = lerpDouble(_verticalDestinationPaddingNoLabel, _verticalDestinationPaddingWithLabel, appearingAnimationValue)!;
final Interval interval = selected ? const Interval(0.25, 0.75) : const Interval(0.75, 1.0); final Interval interval = widget.selected ? const Interval(0.25, 0.75) : const Interval(0.75, 1.0);
final Animation<double> labelFadeAnimation = destinationAnimation.drive(CurveTween(curve: interval)); final Animation<double> labelFadeAnimation = widget.destinationAnimation.drive(CurveTween(curve: interval));
final double minHeight = material3 ? 0 : minWidth; final double minHeight = material3 ? 0 : widget.minWidth;
final Widget topSpacing = SizedBox(height: material3 ? 0 : verticalPadding); final Widget topSpacing = SizedBox(height: material3 ? 0 : verticalPadding);
final Widget labelSpacing = SizedBox(height: material3 ? lerpDouble(0, _verticalIconLabelSpacingM3, appearingAnimationValue)! : 0); final Widget labelSpacing = SizedBox(height: material3 ? lerpDouble(0, _verticalIconLabelSpacingM3, appearingAnimationValue)! : 0);
final Widget bottomSpacing = SizedBox(height: material3 ? _verticalDestinationSpacingM3 : verticalPadding); final Widget bottomSpacing = SizedBox(height: material3 ? _verticalDestinationSpacingM3 : verticalPadding);
final double indicatorHorizontalPadding = (destinationPadding.left / 2) - (destinationPadding.right / 2); final double indicatorHorizontalPadding = (destinationPadding.left / 2) - (destinationPadding.right / 2);
final double indicatorVerticalPadding = destinationPadding.top; final double indicatorVerticalPadding = destinationPadding.top;
indicatorOffset = Offset( indicatorOffset = Offset(
minWidth / 2 + indicatorHorizontalPadding, widget.minWidth / 2 + indicatorHorizontalPadding,
indicatorVerticalPadding + indicatorVerticalOffset, indicatorVerticalPadding + indicatorVerticalOffset,
); );
if (minWidth < _NavigationRailDefaultsM2(context).minWidth!) { if (widget.minWidth < _NavigationRailDefaultsM2(context).minWidth!) {
indicatorOffset = Offset( indicatorOffset = Offset(
minWidth / 2 + _horizontalDestinationSpacingM3, widget.minWidth / 2 + _horizontalDestinationSpacingM3,
indicatorVerticalPadding + indicatorVerticalOffset, indicatorVerticalPadding + indicatorVerticalOffset,
); );
} }
content = Container( content = Container(
constraints: BoxConstraints( constraints: BoxConstraints(
minWidth: minWidth, minWidth: widget.minWidth,
minHeight: minHeight, minHeight: minHeight,
), ),
padding: padding ?? const EdgeInsets.symmetric(horizontal: _horizontalDestinationPadding), padding: widget.padding ?? const EdgeInsets.symmetric(horizontal: _horizontalDestinationPadding),
child: ClipRect( child: ClipRect(
child: Column( child: Column(
mainAxisSize: MainAxisSize.min, mainAxisSize: MainAxisSize.min,
@ -716,11 +751,11 @@ class _RailDestination extends StatelessWidget {
children: <Widget>[ children: <Widget>[
topSpacing, topSpacing,
_AddIndicator( _AddIndicator(
addIndicator: useIndicator, addIndicator: widget.useIndicator,
indicatorColor: indicatorColor, indicatorColor: widget.indicatorColor,
indicatorShape: indicatorShape, indicatorShape: widget.indicatorShape,
isCircular: false, isCircular: false,
indicatorAnimation: destinationAnimation, indicatorAnimation: widget.destinationAnimation,
child: themedIcon, child: themedIcon,
), ),
labelSpacing, labelSpacing,
@ -740,37 +775,37 @@ class _RailDestination extends StatelessWidget {
), ),
); );
case NavigationRailLabelType.all: case NavigationRailLabelType.all:
final double minHeight = material3 ? 0 : minWidth; final double minHeight = material3 ? 0 : widget.minWidth;
final Widget topSpacing = SizedBox(height: material3 ? 0 : _verticalDestinationPaddingWithLabel); final Widget topSpacing = SizedBox(height: material3 ? 0 : _verticalDestinationPaddingWithLabel);
final Widget labelSpacing = SizedBox(height: material3 ? _verticalIconLabelSpacingM3 : 0); final Widget labelSpacing = SizedBox(height: material3 ? _verticalIconLabelSpacingM3 : 0);
final Widget bottomSpacing = SizedBox(height: material3 ? _verticalDestinationSpacingM3 : _verticalDestinationPaddingWithLabel); final Widget bottomSpacing = SizedBox(height: material3 ? _verticalDestinationSpacingM3 : _verticalDestinationPaddingWithLabel);
final double indicatorHorizontalPadding = (destinationPadding.left / 2) - (destinationPadding.right / 2); final double indicatorHorizontalPadding = (destinationPadding.left / 2) - (destinationPadding.right / 2);
final double indicatorVerticalPadding = destinationPadding.top; final double indicatorVerticalPadding = destinationPadding.top;
indicatorOffset = Offset( indicatorOffset = Offset(
minWidth / 2 + indicatorHorizontalPadding, widget.minWidth / 2 + indicatorHorizontalPadding,
indicatorVerticalPadding + indicatorVerticalOffset, indicatorVerticalPadding + indicatorVerticalOffset,
); );
if (minWidth < _NavigationRailDefaultsM2(context).minWidth!) { if (widget.minWidth < _NavigationRailDefaultsM2(context).minWidth!) {
indicatorOffset = Offset( indicatorOffset = Offset(
minWidth / 2 + _horizontalDestinationSpacingM3, widget.minWidth / 2 + _horizontalDestinationSpacingM3,
indicatorVerticalPadding + indicatorVerticalOffset, indicatorVerticalPadding + indicatorVerticalOffset,
); );
} }
content = Container( content = Container(
constraints: BoxConstraints( constraints: BoxConstraints(
minWidth: minWidth, minWidth: widget.minWidth,
minHeight: minHeight, minHeight: minHeight,
), ),
padding: padding ?? const EdgeInsets.symmetric(horizontal: _horizontalDestinationPadding), padding: widget.padding ?? const EdgeInsets.symmetric(horizontal: _horizontalDestinationPadding),
child: Column( child: Column(
children: <Widget>[ children: <Widget>[
topSpacing, topSpacing,
_AddIndicator( _AddIndicator(
addIndicator: useIndicator, addIndicator: widget.useIndicator,
indicatorColor: indicatorColor, indicatorColor: widget.indicatorColor,
indicatorShape: indicatorShape, indicatorShape: widget.indicatorShape,
isCircular: false, isCircular: false,
indicatorAnimation: destinationAnimation, indicatorAnimation: widget.destinationAnimation,
child: themedIcon, child: themedIcon,
), ),
labelSpacing, labelSpacing,
@ -791,15 +826,15 @@ class _RailDestination extends StatelessWidget {
: colors.primary.withOpacity(0.04); : colors.primary.withOpacity(0.04);
return Semantics( return Semantics(
container: true, container: true,
selected: selected, selected: widget.selected,
child: Stack( child: Stack(
children: <Widget>[ children: <Widget>[
Material( Material(
type: MaterialType.transparency, type: MaterialType.transparency,
child: _IndicatorInkWell( child: _IndicatorInkWell(
onTap: disabled ? null : onTap, onTap: widget.disabled ? null : widget.onTap,
borderRadius: BorderRadius.all(Radius.circular(minWidth / 2.0)), borderRadius: BorderRadius.all(Radius.circular(widget.minWidth / 2.0)),
customBorder: indicatorShape, customBorder: widget.indicatorShape,
splashColor: effectiveSplashColor, splashColor: effectiveSplashColor,
hoverColor: effectiveHoverColor, hoverColor: effectiveHoverColor,
useMaterial3: material3, useMaterial3: material3,
@ -810,7 +845,7 @@ class _RailDestination extends StatelessWidget {
), ),
), ),
Semantics( Semantics(
label: indexLabel, label: widget.indexLabel,
), ),
], ],
), ),

6
packages/flutter/test/material/navigation_rail_theme_test.dart
View File

@ -5,6 +5,7 @@
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
void main() { void main() {
test('copyWith, ==, hashCode basics', () { test('copyWith, ==, hashCode basics', () {
@ -145,7 +146,10 @@ void main() {
expect(_indicatorDecoration(tester)?.shape, indicatorShape); expect(_indicatorDecoration(tester)?.shape, indicatorShape);
}); });
testWidgets('NavigationRail values take priority over NavigationRailThemeData values when both properties are specified', (WidgetTester tester) async { testWidgets('NavigationRail values take priority over NavigationRailThemeData values when both properties are specified',
// TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
const Color backgroundColor = Color(0x00000001); const Color backgroundColor = Color(0x00000001);
const double elevation = 7.0; const double elevation = 7.0;
const double selectedIconSize = 25.0; const double selectedIconSize = 25.0;