From 4d3c122434ad6c1916758a596d600519ed060d0d Mon Sep 17 00:00:00 2001 From: yaakovschectman <109111084+yaakovschectman@users.noreply.github.com> Date: Thu, 8 Sep 2022 17:41:54 -0400 Subject: [PATCH] Use tristate checkbox engine changes (#111032) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Introduce tests for tristate checkboxes * Initial * Use tristate changes in engine * Flutter_test matchers test update * Comments, tests * Update packages/flutter/lib/src/semantics/semantics.dart Co-authored-by: Chris Bracken * Assert mutual exclusivity * Assert valid state before updating state * Update packages/flutter/lib/src/semantics/semantics.dart Typo fix in comment Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com> Co-authored-by: Chris Bracken Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com> --- .../flutter/lib/src/material/checkbox.dart | 1 + .../lib/src/rendering/custom_paint.dart | 3 ++ .../flutter/lib/src/rendering/proxy_box.dart | 3 ++ .../flutter/lib/src/semantics/semantics.dart | 38 +++++++++++++- packages/flutter/lib/src/widgets/basic.dart | 2 + .../flutter/test/material/checkbox_test.dart | 52 +++++++++++++++++++ .../test/widgets/custom_painter_test.dart | 13 +++-- .../flutter/test/widgets/semantics_test.dart | 47 ++++++++++++++++- packages/flutter_test/lib/src/matchers.dart | 6 +++ packages/flutter_test/test/matchers_test.dart | 1 + 10 files changed, 158 insertions(+), 8 deletions(-) diff --git a/packages/flutter/lib/src/material/checkbox.dart b/packages/flutter/lib/src/material/checkbox.dart index 0beb12708d..2c8d1ef2a0 100644 --- a/packages/flutter/lib/src/material/checkbox.dart +++ b/packages/flutter/lib/src/material/checkbox.dart @@ -474,6 +474,7 @@ class _CheckboxState extends State with TickerProviderStateMixin, Togg return Semantics( checked: widget.value ?? false, + mixed: widget.tristate ? widget.value == null : null, child: buildToggleable( mouseCursor: effectiveMouseCursor, focusNode: widget.focusNode, diff --git a/packages/flutter/lib/src/rendering/custom_paint.dart b/packages/flutter/lib/src/rendering/custom_paint.dart index 1bd3823088..c70733fe35 100644 --- a/packages/flutter/lib/src/rendering/custom_paint.dart +++ b/packages/flutter/lib/src/rendering/custom_paint.dart @@ -872,6 +872,9 @@ class RenderCustomPaint extends RenderProxyBox { if (properties.checked != null) { config.isChecked = properties.checked; } + if (properties.mixed != null) { + config.isCheckStateMixed = properties.mixed; + } if (properties.selected != null) { config.isSelected = properties.selected!; } diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index cea9b3e107..6465d88c93 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -4355,6 +4355,9 @@ class RenderSemanticsAnnotations extends RenderProxyBox { if (_properties.checked != null) { config.isChecked = _properties.checked; } + if (_properties.mixed != null) { + config.isCheckStateMixed = _properties.mixed; + } if (_properties.toggled != null) { config.isToggled = _properties.toggled; } diff --git a/packages/flutter/lib/src/semantics/semantics.dart b/packages/flutter/lib/src/semantics/semantics.dart index 0f9817356b..df377e4625 100644 --- a/packages/flutter/lib/src/semantics/semantics.dart +++ b/packages/flutter/lib/src/semantics/semantics.dart @@ -776,6 +776,7 @@ class SemanticsProperties extends DiagnosticableTree { const SemanticsProperties({ this.enabled, this.checked, + this.mixed, this.selected, this.toggled, this.button, @@ -851,14 +852,30 @@ class SemanticsProperties extends DiagnosticableTree { /// or similar widget with a "checked" state, and what its current /// state is. /// - /// This is mutually exclusive with [toggled]. + /// When the [Checkbox.value] of a tristate Checkbox is null, + /// indicating a mixed-state, this value shall be false, in which + /// case, [mixed] will be true. + /// + /// This is mutually exclusive with [toggled] and [mixed]. final bool? checked; + /// If non-null, indicates that this subtree represents a checkbox + /// or similar widget with a "half-checked" state or similar, and + /// whether it is currently in this half-checked state. + /// + /// This must be null when [Checkbox.tristate] is false, or + /// when the widget is not a checkbox. When a tristate + /// checkbox is fully unchecked/checked, this value shall + /// be false. + /// + /// This is mutually exclusive with [checked] and [toggled]. + final bool? mixed; + /// If non-null, indicates that this subtree represents a toggle switch /// or similar widget with an "on" state, and what its current /// state is. /// - /// This is mutually exclusive with [checked]. + /// This is mutually exclusive with [checked] and [mixed]. final bool? toggled; /// If non-null indicates that this subtree represents something that can be @@ -1490,6 +1507,7 @@ class SemanticsProperties extends DiagnosticableTree { void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); properties.add(DiagnosticsProperty('checked', checked, defaultValue: null)); + properties.add(DiagnosticsProperty('mixed', mixed, defaultValue: null)); properties.add(DiagnosticsProperty('selected', selected, defaultValue: null)); properties.add(StringProperty('label', label, defaultValue: null)); properties.add(AttributedStringProperty('attributedLabel', attributedLabel, defaultValue: null)); @@ -4189,10 +4207,26 @@ class SemanticsConfiguration { /// checked/unchecked state. bool? get isChecked => _hasFlag(SemanticsFlag.hasCheckedState) ? _hasFlag(SemanticsFlag.isChecked) : null; set isChecked(bool? value) { + assert(value != true || isCheckStateMixed != true); _setFlag(SemanticsFlag.hasCheckedState, true); _setFlag(SemanticsFlag.isChecked, value!); } + /// If this node has tristate that can be controlled by the user, whether + /// that state is in its mixed state. + /// + /// Do not call the setter for this field if the owning [RenderObject] doesn't + /// have checked/unchecked state that can be controlled by the user. + /// + /// The getter returns null if the owning [RenderObject] does not have + /// mixed checked state. + bool? get isCheckStateMixed => _hasFlag(SemanticsFlag.hasCheckedState) ? _hasFlag(SemanticsFlag.isCheckStateMixed) : null; + set isCheckStateMixed(bool? value) { + assert(value != true || isChecked != true); + _setFlag(SemanticsFlag.hasCheckedState, true); + _setFlag(SemanticsFlag.isCheckStateMixed, value!); + } + /// If this node has Boolean state that can be controlled by the user, whether /// that state is on or off, corresponding to true and false, respectively. /// diff --git a/packages/flutter/lib/src/widgets/basic.dart b/packages/flutter/lib/src/widgets/basic.dart index 037dc2347d..54770f38d5 100644 --- a/packages/flutter/lib/src/widgets/basic.dart +++ b/packages/flutter/lib/src/widgets/basic.dart @@ -6878,6 +6878,7 @@ class Semantics extends SingleChildRenderObjectWidget { bool excludeSemantics = false, bool? enabled, bool? checked, + bool? mixed, bool? selected, bool? toggled, bool? button, @@ -6943,6 +6944,7 @@ class Semantics extends SingleChildRenderObjectWidget { properties: SemanticsProperties( enabled: enabled, checked: checked, + mixed: mixed, toggled: toggled, selected: selected, button: button, diff --git a/packages/flutter/test/material/checkbox_test.dart b/packages/flutter/test/material/checkbox_test.dart index 4b49948c4c..146ebb8f0d 100644 --- a/packages/flutter/test/material/checkbox_test.dart +++ b/packages/flutter/test/material/checkbox_test.dart @@ -139,6 +139,57 @@ void main() { hasEnabledState: true, isChecked: true, )); + + await tester.pumpWidget(Theme( + data: theme, + child: const Material( + child: Checkbox( + value: null, + tristate: true, + onChanged: null, + ), + ), + )); + + expect(tester.getSemantics(find.byType(Checkbox)), matchesSemantics( + hasCheckedState: true, + hasEnabledState: true, + isCheckStateMixed: true, + )); + + await tester.pumpWidget(Theme( + data: theme, + child: const Material( + child: Checkbox( + value: true, + tristate: true, + onChanged: null, + ), + ), + )); + + expect(tester.getSemantics(find.byType(Checkbox)), matchesSemantics( + hasCheckedState: true, + hasEnabledState: true, + isChecked: true, + )); + + await tester.pumpWidget(Theme( + data: theme, + child: const Material( + child: Checkbox( + value: false, + tristate: true, + onChanged: null, + ), + ), + )); + + expect(tester.getSemantics(find.byType(Checkbox)), matchesSemantics( + hasCheckedState: true, + hasEnabledState: true, + )); + handle.dispose(); }); @@ -239,6 +290,7 @@ void main() { SemanticsFlag.hasEnabledState, SemanticsFlag.isEnabled, SemanticsFlag.isFocusable, + SemanticsFlag.isCheckStateMixed, ], actions: [SemanticsAction.tap], ), hasLength(1)); diff --git a/packages/flutter/test/widgets/custom_painter_test.dart b/packages/flutter/test/widgets/custom_painter_test.dart index 22fe11099b..46a05b5a75 100644 --- a/packages/flutter/test/widgets/custom_painter_test.dart +++ b/packages/flutter/test/widgets/custom_painter_test.dart @@ -451,7 +451,9 @@ void _defineTests() { List flags = SemanticsFlag.values.values.toList(); // [SemanticsFlag.hasImplicitScrolling] isn't part of [SemanticsProperties] // therefore it has to be removed. - flags.remove(SemanticsFlag.hasImplicitScrolling); + flags + ..remove(SemanticsFlag.hasImplicitScrolling) + ..remove(SemanticsFlag.isCheckStateMixed); TestSemantics expectedSemantics = TestSemantics.root( children: [ TestSemantics.rootChild( @@ -475,7 +477,8 @@ void _defineTests() { rect: Rect.fromLTRB(1.0, 2.0, 3.0, 4.0), properties: SemanticsProperties( enabled: true, - checked: true, + checked: false, + mixed: true, toggled: true, selected: true, hidden: true, @@ -502,7 +505,9 @@ void _defineTests() { flags = SemanticsFlag.values.values.toList(); // [SemanticsFlag.hasImplicitScrolling] isn't part of [SemanticsProperties] // therefore it has to be removed. - flags.remove(SemanticsFlag.hasImplicitScrolling); + flags + ..remove(SemanticsFlag.hasImplicitScrolling) + ..remove(SemanticsFlag.isChecked); expectedSemantics = TestSemantics.root( children: [ TestSemantics.rootChild( @@ -519,7 +524,7 @@ void _defineTests() { ); expect(semantics, hasSemantics(expectedSemantics, ignoreRect: true, ignoreTransform: true)); semantics.dispose(); - }, skip: true); // [intended] https://github.com/flutter/flutter/issues/110107 + }); group('diffing', () { testWidgets('complains about duplicate keys', (WidgetTester tester) async { diff --git a/packages/flutter/test/widgets/semantics_test.dart b/packages/flutter/test/widgets/semantics_test.dart index f9ca40bc2e..bdadfee6b9 100644 --- a/packages/flutter/test/widgets/semantics_test.dart +++ b/packages/flutter/test/widgets/semantics_test.dart @@ -584,7 +584,8 @@ void main() { flags ..remove(SemanticsFlag.hasToggledState) ..remove(SemanticsFlag.isToggled) - ..remove(SemanticsFlag.hasImplicitScrolling); + ..remove(SemanticsFlag.hasImplicitScrolling) + ..remove(SemanticsFlag.isCheckStateMixed); TestSemantics expectedSemantics = TestSemantics.root( children: [ @@ -631,8 +632,50 @@ void main() { ); expect(semantics, hasSemantics(expectedSemantics, ignoreId: true)); + + await tester.pumpWidget( + Semantics( + key: const Key('a'), + container: true, + explicitChildNodes: true, + // flags + enabled: true, + hidden: true, + checked: false, + mixed: true, + selected: true, + button: true, + slider: true, + keyboardKey: true, + link: true, + textField: true, + readOnly: true, + focused: true, + focusable: true, + inMutuallyExclusiveGroup: true, + header: true, + obscured: true, + multiline: true, + scopesRoute: true, + namesRoute: true, + image: true, + liveRegion: true, + ), + ); + flags + ..remove(SemanticsFlag.isChecked) + ..add(SemanticsFlag.isCheckStateMixed); semantics.dispose(); - }, skip: true); // [intended] https://github.com/flutter/flutter/issues/110107 + expectedSemantics = TestSemantics.root( + children: [ + TestSemantics.rootChild( + rect: TestSemantics.fullScreen, + flags: flags, + ), + ], + ); + expect(semantics, hasSemantics(expectedSemantics, ignoreId: true)); + }); testWidgets('Actions can be replaced without triggering semantics update', (WidgetTester tester) async { final SemanticsTester semantics = SemanticsTester(tester); diff --git a/packages/flutter_test/lib/src/matchers.dart b/packages/flutter_test/lib/src/matchers.dart index b10cfe091a..4fcd237672 100644 --- a/packages/flutter_test/lib/src/matchers.dart +++ b/packages/flutter_test/lib/src/matchers.dart @@ -542,6 +542,7 @@ Matcher matchesSemantics({ // Flags // bool hasCheckedState = false, bool isChecked = false, + bool isCheckStateMixed = false, bool isSelected = false, bool isButton = false, bool isSlider = false, @@ -617,6 +618,7 @@ Matcher matchesSemantics({ // Flags hasCheckedState: hasCheckedState, isChecked: isChecked, + isCheckStateMixed: isCheckStateMixed, isSelected: isSelected, isButton: isButton, isSlider: isSlider, @@ -713,6 +715,7 @@ Matcher containsSemantics({ // Flags bool? hasCheckedState, bool? isChecked, + bool? isCheckStateMixed, bool? isSelected, bool? isButton, bool? isSlider, @@ -788,6 +791,7 @@ Matcher containsSemantics({ // Flags hasCheckedState: hasCheckedState, isChecked: isChecked, + isCheckStateMixed: isCheckStateMixed, isSelected: isSelected, isButton: isButton, isSlider: isSlider, @@ -2085,6 +2089,7 @@ class _MatchesSemanticsData extends Matcher { // Flags required bool? hasCheckedState, required bool? isChecked, + required bool? isCheckStateMixed, required bool? isSelected, required bool? isButton, required bool? isSlider, @@ -2138,6 +2143,7 @@ class _MatchesSemanticsData extends Matcher { }) : flags = { if (hasCheckedState != null) SemanticsFlag.hasCheckedState: hasCheckedState, if (isChecked != null) SemanticsFlag.isChecked: isChecked, + if (isCheckStateMixed != null) SemanticsFlag.isCheckStateMixed: isCheckStateMixed, if (isSelected != null) SemanticsFlag.isSelected: isSelected, if (isButton != null) SemanticsFlag.isButton: isButton, if (isSlider != null) SemanticsFlag.isSlider: isSlider, diff --git a/packages/flutter_test/test/matchers_test.dart b/packages/flutter_test/test/matchers_test.dart index 6bbaf5650e..6d79967dd8 100644 --- a/packages/flutter_test/test/matchers_test.dart +++ b/packages/flutter_test/test/matchers_test.dart @@ -656,6 +656,7 @@ void main() { /* Flags */ hasCheckedState: true, isChecked: true, + isCheckStateMixed: true, isSelected: true, isButton: true, isSlider: true,