From e6d43ac6d466e04f4c7c6a819e8dc163a28365b3 Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Tue, 21 Nov 2023 19:00:06 +0200 Subject: [PATCH] Fix Chips with Tooltip throw an assertion when enabling or disabling (#138799) fixes [Enabling or disabling a `Chip`/`RawChip` with a tooltip throws an exception](https://github.com/flutter/flutter/issues/138287) ### Code sample
expand to view the code sample ```dart import 'package:flutter/material.dart'; void main() => runApp(const MyApp()); class MyApp extends StatelessWidget { const MyApp({super.key}); @override Widget build(BuildContext context) { bool isEnabled = true; return MaterialApp( home: Material( child: Center( child: StatefulBuilder( builder: (BuildContext context, StateSetter setState) { return Column( mainAxisSize: MainAxisSize.min, children: [ RawChip( tooltip: 'This is a tooltip', isEnabled: isEnabled, label: const Text('RawChip'), onPressed: isEnabled ? () {} : null, ), const SizedBox(height: 20), ElevatedButton( onPressed: () { setState(() { isEnabled = !isEnabled; }); }, child: Text('${isEnabled ? 'Disable' : 'Enable'} Chip'), ) ], ); }), ), ), ); } } ```
--- packages/flutter/lib/src/material/chip.dart | 25 +++++++--- packages/flutter/test/material/chip_test.dart | 49 +++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/packages/flutter/lib/src/material/chip.dart b/packages/flutter/lib/src/material/chip.dart index 5c785ae537..45fb145833 100644 --- a/packages/flutter/lib/src/material/chip.dart +++ b/packages/flutter/lib/src/material/chip.dart @@ -1554,12 +1554,7 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip required this.enableAnimation, this.avatarBorder, }) : _theme = theme, - _textDirection = textDirection { - checkmarkAnimation.addListener(markNeedsPaint); - avatarDrawerAnimation.addListener(markNeedsLayout); - deleteDrawerAnimation.addListener(markNeedsLayout); - enableAnimation.addListener(markNeedsPaint); - } + _textDirection = textDirection; bool? value; bool? isEnabled; @@ -2099,6 +2094,24 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip } } + @override + void attach(PipelineOwner owner) { + super.attach(owner); + checkmarkAnimation.addListener(markNeedsPaint); + avatarDrawerAnimation.addListener(markNeedsLayout); + deleteDrawerAnimation.addListener(markNeedsLayout); + enableAnimation.addListener(markNeedsPaint); + } + + @override + void detach() { + checkmarkAnimation.removeListener(markNeedsPaint); + avatarDrawerAnimation.removeListener(markNeedsLayout); + deleteDrawerAnimation.removeListener(markNeedsLayout); + enableAnimation.removeListener(markNeedsPaint); + super.detach(); + } + @override void dispose() { _childOpacityLayerHandler.layer = null; diff --git a/packages/flutter/test/material/chip_test.dart b/packages/flutter/test/material/chip_test.dart index 77098ff7bb..960ec6d3db 100644 --- a/packages/flutter/test/material/chip_test.dart +++ b/packages/flutter/test/material/chip_test.dart @@ -3604,6 +3604,55 @@ void main() { expect(getIconData(tester).color, const Color(0xff112233)); }); + // This is a regression test for https://github.com/flutter/flutter/issues/138287. + testWidgetsWithLeakTracking("Enabling and disabling Chip with Tooltip doesn't throw an exception", (WidgetTester tester) async { + bool isEnabled = true; + + await tester.pumpWidget(MaterialApp( + home: Material( + child: Center( + child: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return Column( + mainAxisSize: MainAxisSize.min, + children: [ + RawChip( + tooltip: 'tooltip', + isEnabled: isEnabled, + onPressed: isEnabled ? () {} : null, + label: const Text('RawChip'), + ), + ElevatedButton( + onPressed: () { + setState(() { + isEnabled = !isEnabled; + }); + }, + child: Text('${isEnabled ? 'Disable' : 'Enable'} Chip'), + ) + ], + ); + }, + ), + ), + ), + )); + + // Tap the elevated button to disable the chip with a tooltip. + await tester.tap(find.widgetWithText(ElevatedButton, 'Disable Chip')); + await tester.pumpAndSettle(); + + // No exception should be thrown. + expect(tester.takeException(), isNull); + + // Tap the elevated button to enable the chip with a tooltip. + await tester.tap(find.widgetWithText(ElevatedButton, 'Enable Chip')); + await tester.pumpAndSettle(); + + // No exception should be thrown. + expect(tester.takeException(), isNull); + }); + group('Material 2', () { // These tests are only relevant for Material 2. Once Material 2 // support is deprecated and the APIs are removed, these tests