From 579cab8a868540b107e4e0c9f7afec3fbae11b60 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Mon, 17 Aug 2020 11:36:02 -0700 Subject: [PATCH] Prevent material switch from recreating its render object when it becomes disabled (#61398) --- packages/flutter/lib/src/widgets/actions.dart | 39 ++++++++------- .../flutter/test/material/checkbox_test.dart | 13 ++++- .../flutter/test/material/radio_test.dart | 21 ++++++++- .../flutter/test/material/slider_test.dart | 47 ++++++++++++++++++- .../flutter/test/material/switch_test.dart | 47 +++++++++++++++++++ 5 files changed, 145 insertions(+), 22 deletions(-) diff --git a/packages/flutter/lib/src/widgets/actions.dart b/packages/flutter/lib/src/widgets/actions.dart index f66d582e063..81e15da9928 100644 --- a/packages/flutter/lib/src/widgets/actions.dart +++ b/packages/flutter/lib/src/widgets/actions.dart @@ -1092,25 +1092,30 @@ class _FocusableActionDetectorState extends State { @override Widget build(BuildContext context) { - Widget child = MouseRegion( - onEnter: _handleMouseEnter, - onExit: _handleMouseExit, - cursor: widget.mouseCursor, - child: Focus( - focusNode: widget.focusNode, - autofocus: widget.autofocus, - canRequestFocus: _canRequestFocus, - onFocusChange: _handleFocusChange, - child: widget.child, + final Map> actions = widget.enabled && widget.actions != null + ? widget.actions + : const >{}; + final Map shortcuts = widget.enabled && widget.shortcuts != null + ? widget.shortcuts + : const {}; + + return Actions(actions: actions, + child: Shortcuts( + shortcuts: shortcuts, + child: MouseRegion( + onEnter: _handleMouseEnter, + onExit: _handleMouseExit, + cursor: widget.mouseCursor, + child: Focus( + focusNode: widget.focusNode, + autofocus: widget.autofocus, + canRequestFocus: _canRequestFocus, + onFocusChange: _handleFocusChange, + child: widget.child, + ), + ), ), ); - if (widget.enabled && widget.actions != null && widget.actions.isNotEmpty) { - child = Actions(actions: widget.actions, child: child); - } - if (widget.enabled && widget.shortcuts != null && widget.shortcuts.isNotEmpty) { - child = Shortcuts(shortcuts: widget.shortcuts, child: child); - } - return child; } } diff --git a/packages/flutter/test/material/checkbox_test.dart b/packages/flutter/test/material/checkbox_test.dart index d0044eb15a7..904faf7e96f 100644 --- a/packages/flutter/test/material/checkbox_test.dart +++ b/packages/flutter/test/material/checkbox_test.dart @@ -70,7 +70,7 @@ void main() { ), )); - expect(tester.getSemantics(find.byType(Focus)), matchesSemantics( + expect(tester.getSemantics(find.byType(Checkbox)), matchesSemantics( hasCheckedState: true, hasEnabledState: true, isEnabled: true, @@ -85,7 +85,7 @@ void main() { ), )); - expect(tester.getSemantics(find.byType(Focus)), matchesSemantics( + expect(tester.getSemantics(find.byType(Checkbox)), matchesSemantics( hasCheckedState: true, hasEnabledState: true, isChecked: true, @@ -101,6 +101,15 @@ void main() { ), )); + expect(tester.getSemantics(find.byWidgetPredicate((Widget widget) => widget.runtimeType.toString() == '_CheckboxRenderObjectWidget')), matchesSemantics( + hasCheckedState: true, + hasEnabledState: true, + // isFocusable is delayed by 1 frame. + isFocusable: true, + )); + + await tester.pump(); + // isFocusable should be false now after the 1 frame delay. expect(tester.getSemantics(find.byWidgetPredicate((Widget widget) => widget.runtimeType.toString() == '_CheckboxRenderObjectWidget')), matchesSemantics( hasCheckedState: true, hasEnabledState: true, diff --git a/packages/flutter/test/material/radio_test.dart b/packages/flutter/test/material/radio_test.dart index f3406fd3b8a..02d866edff6 100644 --- a/packages/flutter/test/material/radio_test.dart +++ b/packages/flutter/test/material/radio_test.dart @@ -237,7 +237,24 @@ void main() { expect(semantics, hasSemantics(TestSemantics.root( children: [ TestSemantics.rootChild( - id: 2, + id: 1, + flags: [ + SemanticsFlag.hasCheckedState, + SemanticsFlag.hasEnabledState, + SemanticsFlag.isInMutuallyExclusiveGroup, + SemanticsFlag.isFocusable, // This flag is delayed by 1 frame. + ], + ), + ], + ), ignoreRect: true, ignoreTransform: true)); + + await tester.pump(); + + // Now the isFocusable should be gone. + expect(semantics, hasSemantics(TestSemantics.root( + children: [ + TestSemantics.rootChild( + id: 1, flags: [ SemanticsFlag.hasCheckedState, SemanticsFlag.hasEnabledState, @@ -258,7 +275,7 @@ void main() { expect(semantics, hasSemantics(TestSemantics.root( children: [ TestSemantics.rootChild( - id: 2, + id: 1, flags: [ SemanticsFlag.hasCheckedState, SemanticsFlag.isChecked, diff --git a/packages/flutter/test/material/slider_test.dart b/packages/flutter/test/material/slider_test.dart index afba7eeff6f..4d3c1c3ddf1 100644 --- a/packages/flutter/test/material/slider_test.dart +++ b/packages/flutter/test/material/slider_test.dart @@ -1402,7 +1402,11 @@ void main() { children: [ TestSemantics( id: 4, - flags: [SemanticsFlag.hasEnabledState], + flags: [ + SemanticsFlag.hasEnabledState, + // isFocusable is delayed by 1 frame. + SemanticsFlag.isFocusable, + ], value: '50%', increasedValue: '55%', decreasedValue: '45%', @@ -1420,6 +1424,47 @@ void main() { ignoreTransform: true, ), ); + + await tester.pump(); + expect( + semantics, + hasSemantics( + TestSemantics.root( + children: [ + TestSemantics( + id: 1, + textDirection: TextDirection.ltr, + children: [ + TestSemantics( + id: 2, + children: [ + TestSemantics( + id: 3, + flags: [SemanticsFlag.scopesRoute], + children: [ + TestSemantics( + id: 4, + flags: [ + SemanticsFlag.hasEnabledState, + ], + value: '50%', + increasedValue: '55%', + decreasedValue: '45%', + textDirection: TextDirection.ltr, + ), + ], + ), + ], + ), + ], + ), + ], + ), + ignoreRect: true, + ignoreTransform: true, + ), + ); + semantics.dispose(); }, variant: const TargetPlatformVariant({ TargetPlatform.android, TargetPlatform.fuchsia, TargetPlatform.linux, TargetPlatform.windows })); diff --git a/packages/flutter/test/material/switch_test.dart b/packages/flutter/test/material/switch_test.dart index 22c4d13a05b..528c30cc2fa 100644 --- a/packages/flutter/test/material/switch_test.dart +++ b/packages/flutter/test/material/switch_test.dart @@ -1015,4 +1015,51 @@ void main() { await tester.pumpAndSettle(); }); + + testWidgets('Material switch should not recreate its render object when disabled', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/61247. + bool value = true; + bool enabled = true; + StateSetter stateSetter; + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + stateSetter = setState; + return Material( + child: Center( + child: Switch( + value: value, + onChanged: !enabled ? null : (bool newValue) { + setState(() { + value = newValue; + }); + }, + ), + ), + ); + }, + ), + ), + ); + + final RenderToggleable oldSwitchRenderObject = tester + .renderObject(find.byWidgetPredicate((Widget widget) => widget is LeafRenderObjectWidget)); + + stateSetter(() { value = false; }); + await tester.pump(); + // Disable the switch when the implicit animation begins. + stateSetter(() { enabled = false; }); + await tester.pump(); + + final RenderToggleable updatedSwitchRenderObject = tester + .renderObject(find.byWidgetPredicate((Widget widget) => widget is LeafRenderObjectWidget)); + + + expect(updatedSwitchRenderObject.isInteractive, false); + expect(updatedSwitchRenderObject, oldSwitchRenderObject); + expect(updatedSwitchRenderObject.position.isCompleted, false); + expect(updatedSwitchRenderObject.position.isDismissed, false); + }); }