Unverified Commit 8473da22 authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Fix `Slider` semantic node size (#115285)

parent 333397a0
...@@ -821,22 +821,11 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin { ...@@ -821,22 +821,11 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin {
// in range_slider.dart. // in range_slider.dart.
Size screenSize() => MediaQuery.of(context).size; Size screenSize() => MediaQuery.of(context).size;
VoidCallback? handleDidGainAccessibilityFocus; void handleDidGainAccessibilityFocus() {
switch (theme.platform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.iOS:
case TargetPlatform.linux:
case TargetPlatform.macOS:
break;
case TargetPlatform.windows:
handleDidGainAccessibilityFocus = () {
// Automatically activate the slider when it receives a11y focus. // Automatically activate the slider when it receives a11y focus.
if (!focusNode.hasFocus && focusNode.canRequestFocus) { if (!focusNode.hasFocus && focusNode.canRequestFocus) {
focusNode.requestFocus(); focusNode.requestFocus();
} }
};
break;
} }
final Map<ShortcutActivator, Intent> shortcutMap; final Map<ShortcutActivator, Intent> shortcutMap;
...@@ -857,11 +846,7 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin { ...@@ -857,11 +846,7 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin {
? math.min(MediaQuery.of(context).textScaleFactor, 1.3) ? math.min(MediaQuery.of(context).textScaleFactor, 1.3)
: MediaQuery.of(context).textScaleFactor; : MediaQuery.of(context).textScaleFactor;
return Semantics( return FocusableActionDetector(
container: true,
slider: true,
onDidGainAccessibilityFocus: handleDidGainAccessibilityFocus,
child: FocusableActionDetector(
actions: _actionMap, actions: _actionMap,
shortcuts: shortcutMap, shortcuts: shortcutMap,
focusNode: focusNode, focusNode: focusNode,
...@@ -870,6 +855,7 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin { ...@@ -870,6 +855,7 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin {
onShowFocusHighlight: _handleFocusHighlightChanged, onShowFocusHighlight: _handleFocusHighlightChanged,
onShowHoverHighlight: _handleHoverChanged, onShowHoverHighlight: _handleHoverChanged,
mouseCursor: effectiveMouseCursor, mouseCursor: effectiveMouseCursor,
includeFocusSemantics: false,
child: CompositedTransformTarget( child: CompositedTransformTarget(
link: _layerLink, link: _layerLink,
child: _SliderRenderObjectWidget( child: _SliderRenderObjectWidget(
...@@ -886,11 +872,11 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin { ...@@ -886,11 +872,11 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin {
onChangeEnd: _handleDragEnd, onChangeEnd: _handleDragEnd,
state: this, state: this,
semanticFormatterCallback: widget.semanticFormatterCallback, semanticFormatterCallback: widget.semanticFormatterCallback,
onDidGainAccessibilityFocus: handleDidGainAccessibilityFocus,
hasFocus: _focused, hasFocus: _focused,
hovering: _hovering, hovering: _hovering,
), ),
), ),
),
); );
} }
...@@ -949,6 +935,7 @@ class _SliderRenderObjectWidget extends LeafRenderObjectWidget { ...@@ -949,6 +935,7 @@ class _SliderRenderObjectWidget extends LeafRenderObjectWidget {
required this.onChangeEnd, required this.onChangeEnd,
required this.state, required this.state,
required this.semanticFormatterCallback, required this.semanticFormatterCallback,
required this.onDidGainAccessibilityFocus,
required this.hasFocus, required this.hasFocus,
required this.hovering, required this.hovering,
}); });
...@@ -964,6 +951,7 @@ class _SliderRenderObjectWidget extends LeafRenderObjectWidget { ...@@ -964,6 +951,7 @@ class _SliderRenderObjectWidget extends LeafRenderObjectWidget {
final ValueChanged<double>? onChangeStart; final ValueChanged<double>? onChangeStart;
final ValueChanged<double>? onChangeEnd; final ValueChanged<double>? onChangeEnd;
final SemanticFormatterCallback? semanticFormatterCallback; final SemanticFormatterCallback? semanticFormatterCallback;
final VoidCallback? onDidGainAccessibilityFocus;
final _SliderState state; final _SliderState state;
final bool hasFocus; final bool hasFocus;
final bool hovering; final bool hovering;
...@@ -984,6 +972,7 @@ class _SliderRenderObjectWidget extends LeafRenderObjectWidget { ...@@ -984,6 +972,7 @@ class _SliderRenderObjectWidget extends LeafRenderObjectWidget {
state: state, state: state,
textDirection: Directionality.of(context), textDirection: Directionality.of(context),
semanticFormatterCallback: semanticFormatterCallback, semanticFormatterCallback: semanticFormatterCallback,
onDidGainAccessibilityFocus: onDidGainAccessibilityFocus,
platform: Theme.of(context).platform, platform: Theme.of(context).platform,
hasFocus: hasFocus, hasFocus: hasFocus,
hovering: hovering, hovering: hovering,
...@@ -1008,6 +997,7 @@ class _SliderRenderObjectWidget extends LeafRenderObjectWidget { ...@@ -1008,6 +997,7 @@ class _SliderRenderObjectWidget extends LeafRenderObjectWidget {
..onChangeEnd = onChangeEnd ..onChangeEnd = onChangeEnd
..textDirection = Directionality.of(context) ..textDirection = Directionality.of(context)
..semanticFormatterCallback = semanticFormatterCallback ..semanticFormatterCallback = semanticFormatterCallback
..onDidGainAccessibilityFocus = onDidGainAccessibilityFocus
..platform = Theme.of(context).platform ..platform = Theme.of(context).platform
..hasFocus = hasFocus ..hasFocus = hasFocus
..hovering = hovering ..hovering = hovering
...@@ -1029,6 +1019,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1029,6 +1019,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
required TargetPlatform platform, required TargetPlatform platform,
required ValueChanged<double>? onChanged, required ValueChanged<double>? onChanged,
required SemanticFormatterCallback? semanticFormatterCallback, required SemanticFormatterCallback? semanticFormatterCallback,
required this.onDidGainAccessibilityFocus,
required this.onChangeStart, required this.onChangeStart,
required this.onChangeEnd, required this.onChangeEnd,
required _SliderState state, required _SliderState state,
...@@ -1114,6 +1105,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1114,6 +1105,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
bool _active = false; bool _active = false;
double _currentDragValue = 0.0; double _currentDragValue = 0.0;
Rect? overlayRect; Rect? overlayRect;
late Offset _thumbCenter;
// This rect is used in gesture calculations, where the gesture coordinates // This rect is used in gesture calculations, where the gesture coordinates
// are relative to the sliders origin. Therefore, the offset is passed as // are relative to the sliders origin. Therefore, the offset is passed as
...@@ -1259,6 +1251,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1259,6 +1251,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
} }
} }
VoidCallback? onDidGainAccessibilityFocus;
ValueChanged<double>? onChangeStart; ValueChanged<double>? onChangeStart;
ValueChanged<double>? onChangeEnd; ValueChanged<double>? onChangeEnd;
...@@ -1582,10 +1575,10 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1582,10 +1575,10 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
sliderTheme: _sliderTheme, sliderTheme: _sliderTheme,
isDiscrete: isDiscrete, isDiscrete: isDiscrete,
); );
final Offset thumbCenter = Offset(trackRect.left + visualPosition * trackRect.width, trackRect.center.dy); _thumbCenter = Offset(trackRect.left + visualPosition * trackRect.width, trackRect.center.dy);
if (isInteractive) { if (isInteractive) {
final Size overlaySize = sliderTheme.overlayShape!.getPreferredSize(isInteractive, false); final Size overlaySize = sliderTheme.overlayShape!.getPreferredSize(isInteractive, false);
overlayRect = Rect.fromCircle(center: thumbCenter, radius: overlaySize.width / 2.0); overlayRect = Rect.fromCircle(center: _thumbCenter, radius: overlaySize.width / 2.0);
} }
final Offset? secondaryOffset = (secondaryVisualPosition != null) ? Offset(trackRect.left + secondaryVisualPosition * trackRect.width, trackRect.center.dy) : null; final Offset? secondaryOffset = (secondaryVisualPosition != null) ? Offset(trackRect.left + secondaryVisualPosition * trackRect.width, trackRect.center.dy) : null;
...@@ -1596,7 +1589,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1596,7 +1589,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
sliderTheme: _sliderTheme, sliderTheme: _sliderTheme,
enableAnimation: _enableAnimation, enableAnimation: _enableAnimation,
textDirection: _textDirection, textDirection: _textDirection,
thumbCenter: thumbCenter, thumbCenter: _thumbCenter,
secondaryOffset: secondaryOffset, secondaryOffset: secondaryOffset,
isDiscrete: isDiscrete, isDiscrete: isDiscrete,
isEnabled: isInteractive, isEnabled: isInteractive,
...@@ -1605,7 +1598,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1605,7 +1598,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
if (!_overlayAnimation.isDismissed) { if (!_overlayAnimation.isDismissed) {
_sliderTheme.overlayShape!.paint( _sliderTheme.overlayShape!.paint(
context, context,
thumbCenter, _thumbCenter,
activationAnimation: _overlayAnimation, activationAnimation: _overlayAnimation,
enableAnimation: _enableAnimation, enableAnimation: _enableAnimation,
isDiscrete: isDiscrete, isDiscrete: isDiscrete,
...@@ -1642,7 +1635,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1642,7 +1635,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
sliderTheme: _sliderTheme, sliderTheme: _sliderTheme,
enableAnimation: _enableAnimation, enableAnimation: _enableAnimation,
textDirection: _textDirection, textDirection: _textDirection,
thumbCenter: thumbCenter, thumbCenter: _thumbCenter,
isEnabled: isInteractive, isEnabled: isInteractive,
); );
} }
...@@ -1655,7 +1648,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1655,7 +1648,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
if (attached) { if (attached) {
_sliderTheme.valueIndicatorShape!.paint( _sliderTheme.valueIndicatorShape!.paint(
context, context,
offset + thumbCenter, offset + _thumbCenter,
activationAnimation: _valueIndicatorAnimation, activationAnimation: _valueIndicatorAnimation,
enableAnimation: _enableAnimation, enableAnimation: _enableAnimation,
isDiscrete: isDiscrete, isDiscrete: isDiscrete,
...@@ -1674,7 +1667,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1674,7 +1667,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
_sliderTheme.thumbShape!.paint( _sliderTheme.thumbShape!.paint(
context, context,
thumbCenter, _thumbCenter,
activationAnimation: _overlayAnimation, activationAnimation: _overlayAnimation,
enableAnimation: _enableAnimation, enableAnimation: _enableAnimation,
isDiscrete: isDiscrete, isDiscrete: isDiscrete,
...@@ -1688,12 +1681,23 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1688,12 +1681,23 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
); );
} }
@override
void assembleSemanticsNode(SemanticsNode node, SemanticsConfiguration config, Iterable<SemanticsNode> children) {
node.rect = Rect.fromCenter(
center: _thumbCenter,
width: kMinInteractiveDimension,
height: kMinInteractiveDimension,
);
node.updateWith(config: config);
}
@override @override
void describeSemanticsConfiguration(SemanticsConfiguration config) { void describeSemanticsConfiguration(SemanticsConfiguration config) {
super.describeSemanticsConfiguration(config); super.describeSemanticsConfiguration(config);
// The Slider widget has its own Focus widget with semantics information, // The Slider widget has its own Focus widget with semantics information,
// and we want that semantics node to collect the semantics information here // and want that semantics node to collect the semantics information here
// so that it's all in the same node: otherwise Talkback sees that the node // so that it's all in the same node: otherwise Talkback sees that the node
// has focusable children, and it won't focus the Slider's Focus widget // has focusable children, and it won't focus the Slider's Focus widget
// because it thinks the Focus widget's node doesn't have anything to say // because it thinks the Focus widget's node doesn't have anything to say
...@@ -1701,9 +1705,23 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -1701,9 +1705,23 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
// information into one node means that Talkback will recognize that it has // information into one node means that Talkback will recognize that it has
// something to say and focus it when it receives keyboard focus. // something to say and focus it when it receives keyboard focus.
// (See https://github.com/flutter/flutter/issues/57038 for context). // (See https://github.com/flutter/flutter/issues/57038 for context).
config.isSemanticBoundary = false; config.isSemanticBoundary = true;
config.isEnabled = isInteractive; config.isEnabled = isInteractive;
config.isSlider = true;
config.isFocusable = isInteractive;
config.isFocused = hasFocus;
switch (_platform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.iOS:
case TargetPlatform.linux:
case TargetPlatform.macOS:
break;
case TargetPlatform.windows:
config.onDidGainAccessibilityFocus = onDidGainAccessibilityFocus;
break;
}
config.textDirection = textDirection; config.textDirection = textDirection;
if (isInteractive) { if (isInteractive) {
config.onIncrease = increaseAction; config.onIncrease = increaseAction;
......
...@@ -290,7 +290,7 @@ void main() { ...@@ -290,7 +290,7 @@ void main() {
}); });
testWidgets('SemanticsDebugger slider', (WidgetTester tester) async { testWidgets('SemanticsDebugger slider', (WidgetTester tester) async {
double value = 0.75; double value = 0.50;
await tester.pumpWidget( await tester.pumpWidget(
Directionality( Directionality(
...@@ -322,7 +322,7 @@ void main() { ...@@ -322,7 +322,7 @@ void main() {
// interpreted as a gesture by the semantics debugger and sent to the widget // interpreted as a gesture by the semantics debugger and sent to the widget
// as a semantic action that always moves by 10% of the complete track. // as a semantic action that always moves by 10% of the complete track.
await tester.fling(find.byType(Slider), const Offset(-100.0, 0.0), 2000.0, warnIfMissed: false); // hitting the debugger await tester.fling(find.byType(Slider), const Offset(-100.0, 0.0), 2000.0, warnIfMissed: false); // hitting the debugger
expect(value, equals(0.70)); expect(value, equals(0.45));
}); });
testWidgets('SemanticsDebugger checkbox', (WidgetTester tester) async { testWidgets('SemanticsDebugger checkbox', (WidgetTester tester) async {
......
...@@ -865,7 +865,7 @@ void main() { ...@@ -865,7 +865,7 @@ void main() {
await tester.pumpWidget(const MaterialApp(home: _SemanticsTestWidget())); await tester.pumpWidget(const MaterialApp(home: _SemanticsTestWidget()));
// We're expecting the traversal to start where the slider is. // We're expecting the traversal to start where the slider is.
final List<Matcher> expectedMatchers = <Matcher>[...fullTraversalMatchers]..removeRange(0, 8); final List<Matcher> expectedMatchers = <Matcher>[...fullTraversalMatchers]..removeRange(0, 7);
expect( expect(
tester.semantics.simulatedAccessibilityTraversal(start: find.byType(Slider)), tester.semantics.simulatedAccessibilityTraversal(start: find.byType(Slider)),
...@@ -887,7 +887,7 @@ void main() { ...@@ -887,7 +887,7 @@ void main() {
await tester.pumpWidget(const MaterialApp(home: _SemanticsTestWidget())); await tester.pumpWidget(const MaterialApp(home: _SemanticsTestWidget()));
// We're expecting the traversal to end where the slider is, inclusive. // We're expecting the traversal to end where the slider is, inclusive.
final Iterable<Matcher> expectedMatchers = <Matcher>[...fullTraversalMatchers].getRange(0, 9); final Iterable<Matcher> expectedMatchers = <Matcher>[...fullTraversalMatchers].getRange(0, 8);
expect( expect(
tester.semantics.simulatedAccessibilityTraversal(end: find.byType(Slider)), tester.semantics.simulatedAccessibilityTraversal(end: find.byType(Slider)),
...@@ -909,7 +909,7 @@ void main() { ...@@ -909,7 +909,7 @@ void main() {
await tester.pumpWidget(const MaterialApp(home: _SemanticsTestWidget())); await tester.pumpWidget(const MaterialApp(home: _SemanticsTestWidget()));
// We're expecting the traversal to start at the text field and end at the slider. // We're expecting the traversal to start at the text field and end at the slider.
final Iterable<Matcher> expectedMatchers = <Matcher>[...fullTraversalMatchers].getRange(1, 9); final Iterable<Matcher> expectedMatchers = <Matcher>[...fullTraversalMatchers].getRange(1, 8);
expect( expect(
tester.semantics.simulatedAccessibilityTraversal( tester.semantics.simulatedAccessibilityTraversal(
......
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