Unverified Commit 1f891a0f authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Fix `RangeSlider` semantics node size (#114999)

* Fix `RangeSlider` semantics node size and add RTL semantics test

* Add TODO comments
parent 09a4f234
...@@ -840,6 +840,8 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -840,6 +840,8 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
late TapGestureRecognizer _tap; late TapGestureRecognizer _tap;
bool _active = false; bool _active = false;
late RangeValues _newValues; late RangeValues _newValues;
late Offset _startThumbCenter;
late Offset _endThumbCenter;
bool get isEnabled => onChanged != null; bool get isEnabled => onChanged != null;
...@@ -1303,8 +1305,8 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1303,8 +1305,8 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
sliderTheme: _sliderTheme, sliderTheme: _sliderTheme,
isDiscrete: isDiscrete, isDiscrete: isDiscrete,
); );
final Offset startThumbCenter = Offset(trackRect.left + startVisualPosition * trackRect.width, trackRect.center.dy); _startThumbCenter = Offset(trackRect.left + startVisualPosition * trackRect.width, trackRect.center.dy);
final Offset endThumbCenter = Offset(trackRect.left + endVisualPosition * trackRect.width, trackRect.center.dy); _endThumbCenter = Offset(trackRect.left + endVisualPosition * trackRect.width, trackRect.center.dy);
_sliderTheme.rangeTrackShape!.paint( _sliderTheme.rangeTrackShape!.paint(
context, context,
...@@ -1313,8 +1315,8 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1313,8 +1315,8 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
sliderTheme: _sliderTheme, sliderTheme: _sliderTheme,
enableAnimation: _enableAnimation, enableAnimation: _enableAnimation,
textDirection: _textDirection, textDirection: _textDirection,
startThumbCenter: startThumbCenter, startThumbCenter: _startThumbCenter,
endThumbCenter: endThumbCenter, endThumbCenter: _endThumbCenter,
isDiscrete: isDiscrete, isDiscrete: isDiscrete,
isEnabled: isEnabled, isEnabled: isEnabled,
); );
...@@ -1327,7 +1329,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1327,7 +1329,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
if (startThumbSelected) { if (startThumbSelected) {
_sliderTheme.overlayShape!.paint( _sliderTheme.overlayShape!.paint(
context, context,
startThumbCenter, _startThumbCenter,
activationAnimation: _overlayAnimation, activationAnimation: _overlayAnimation,
enableAnimation: _enableAnimation, enableAnimation: _enableAnimation,
isDiscrete: isDiscrete, isDiscrete: isDiscrete,
...@@ -1343,7 +1345,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1343,7 +1345,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
if (endThumbSelected) { if (endThumbSelected) {
_sliderTheme.overlayShape!.paint( _sliderTheme.overlayShape!.paint(
context, context,
endThumbCenter, _endThumbCenter,
activationAnimation: _overlayAnimation, activationAnimation: _overlayAnimation,
enableAnimation: _enableAnimation, enableAnimation: _enableAnimation,
isDiscrete: isDiscrete, isDiscrete: isDiscrete,
...@@ -1381,21 +1383,21 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1381,21 +1383,21 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
sliderTheme: _sliderTheme, sliderTheme: _sliderTheme,
enableAnimation: _enableAnimation, enableAnimation: _enableAnimation,
textDirection: _textDirection, textDirection: _textDirection,
startThumbCenter: startThumbCenter, startThumbCenter: _startThumbCenter,
endThumbCenter: endThumbCenter, endThumbCenter: _endThumbCenter,
isEnabled: isEnabled, isEnabled: isEnabled,
); );
} }
} }
} }
final double thumbDelta = (endThumbCenter.dx - startThumbCenter.dx).abs(); final double thumbDelta = (_endThumbCenter.dx - _startThumbCenter.dx).abs();
final bool isLastThumbStart = _lastThumbSelection == Thumb.start; final bool isLastThumbStart = _lastThumbSelection == Thumb.start;
final Thumb bottomThumb = isLastThumbStart ? Thumb.end : Thumb.start; final Thumb bottomThumb = isLastThumbStart ? Thumb.end : Thumb.start;
final Thumb topThumb = isLastThumbStart ? Thumb.start : Thumb.end; final Thumb topThumb = isLastThumbStart ? Thumb.start : Thumb.end;
final Offset bottomThumbCenter = isLastThumbStart ? endThumbCenter : startThumbCenter; final Offset bottomThumbCenter = isLastThumbStart ? _endThumbCenter : _startThumbCenter;
final Offset topThumbCenter = isLastThumbStart ? startThumbCenter : endThumbCenter; final Offset topThumbCenter = isLastThumbStart ? _startThumbCenter : _endThumbCenter;
final TextPainter bottomLabelPainter = isLastThumbStart ? _endLabelPainter : _startLabelPainter; final TextPainter bottomLabelPainter = isLastThumbStart ? _endLabelPainter : _startLabelPainter;
final TextPainter topLabelPainter = isLastThumbStart ? _startLabelPainter : _endLabelPainter; final TextPainter topLabelPainter = isLastThumbStart ? _startLabelPainter : _endLabelPainter;
final double bottomValue = isLastThumbStart ? endValue : startValue; final double bottomValue = isLastThumbStart ? endValue : startValue;
...@@ -1441,7 +1443,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1441,7 +1443,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
if (shouldPaintValueIndicators) { if (shouldPaintValueIndicators) {
final double startOffset = sliderTheme.rangeValueIndicatorShape!.getHorizontalShift( final double startOffset = sliderTheme.rangeValueIndicatorShape!.getHorizontalShift(
parentBox: this, parentBox: this,
center: startThumbCenter, center: _startThumbCenter,
labelPainter: _startLabelPainter, labelPainter: _startLabelPainter,
activationAnimation: _valueIndicatorAnimation, activationAnimation: _valueIndicatorAnimation,
textScaleFactor: textScaleFactor, textScaleFactor: textScaleFactor,
...@@ -1449,7 +1451,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1449,7 +1451,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
); );
final double endOffset = sliderTheme.rangeValueIndicatorShape!.getHorizontalShift( final double endOffset = sliderTheme.rangeValueIndicatorShape!.getHorizontalShift(
parentBox: this, parentBox: this,
center: endThumbCenter, center: _endThumbCenter,
labelPainter: _endLabelPainter, labelPainter: _endLabelPainter,
activationAnimation: _valueIndicatorAnimation, activationAnimation: _valueIndicatorAnimation,
textScaleFactor: textScaleFactor, textScaleFactor: textScaleFactor,
...@@ -1575,8 +1577,16 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1575,8 +1577,16 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
); );
// Split the semantics node area between the start and end nodes. // Split the semantics node area between the start and end nodes.
final Rect leftRect = Rect.fromPoints(node.rect.topLeft, node.rect.bottomCenter); final Rect leftRect = Rect.fromCenter(
final Rect rightRect = Rect.fromPoints(node.rect.topCenter, node.rect.bottomRight); center: _startThumbCenter,
width: kMinInteractiveDimension,
height: kMinInteractiveDimension,
);
final Rect rightRect = Rect.fromCenter(
center: _endThumbCenter,
width: kMinInteractiveDimension,
height: kMinInteractiveDimension,
);
switch (textDirection) { switch (textDirection) {
case TextDirection.ltr: case TextDirection.ltr:
_startSemanticsNode!.rect = leftRect; _startSemanticsNode!.rect = leftRect;
......
...@@ -1853,7 +1853,7 @@ void main() { ...@@ -1853,7 +1853,7 @@ void main() {
); );
}); });
testWidgets('Range Slider Semantics', (WidgetTester tester) async { testWidgets('Range Slider Semantics - ltr', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
home: Theme( home: Theme(
...@@ -1862,7 +1862,7 @@ void main() { ...@@ -1862,7 +1862,7 @@ void main() {
textDirection: TextDirection.ltr, textDirection: TextDirection.ltr,
child: Material( child: Material(
child: RangeSlider( child: RangeSlider(
values: const RangeValues(10.0, 12.0), values: const RangeValues(10.0, 30.0),
max: 100.0, max: 100.0,
onChanged: (RangeValues v) { }, onChanged: (RangeValues v) { },
), ),
...@@ -1874,8 +1874,9 @@ void main() { ...@@ -1874,8 +1874,9 @@ void main() {
await tester.pumpAndSettle(); await tester.pumpAndSettle();
final SemanticsNode semanticsNode = tester.getSemantics(find.byType(RangeSlider));
expect( expect(
tester.getSemantics(find.byType(RangeSlider)), semanticsNode,
matchesSemantics( matchesSemantics(
scopesRoute: true, scopesRoute: true,
children:<Matcher>[ children:<Matcher>[
...@@ -1888,8 +1889,9 @@ void main() { ...@@ -1888,8 +1889,9 @@ void main() {
hasIncreaseAction: true, hasIncreaseAction: true,
hasDecreaseAction: true, hasDecreaseAction: true,
value: '10%', value: '10%',
increasedValue: '10%', increasedValue: '15%',
decreasedValue: '5%', decreasedValue: '5%',
rect: const Rect.fromLTRB(75.2, 276.0, 123.2, 324.0),
), ),
matchesSemantics( matchesSemantics(
isEnabled: true, isEnabled: true,
...@@ -1897,15 +1899,124 @@ void main() { ...@@ -1897,15 +1899,124 @@ void main() {
hasEnabledState: true, hasEnabledState: true,
hasIncreaseAction: true, hasIncreaseAction: true,
hasDecreaseAction: true, hasDecreaseAction: true,
value: '12%', value: '30%',
increasedValue: '17%', increasedValue: '35%',
decreasedValue: '12%', decreasedValue: '25%',
rect: const Rect.fromLTRB(225.6, 276.0, 273.6, 324.0),
), ),
], ],
), ),
], ],
), ),
); );
// TODO(tahatesser): This is a workaround for matching
// the semantics node rects by avoiding floating point errors.
// https://github.com/flutter/flutter/issues/115079
// Get semantics node rects.
final List<Rect> rects = <Rect>[];
semanticsNode.visitChildren((SemanticsNode node) {
node.visitChildren((SemanticsNode node) {
// Round rect values to avoid floating point errors.
rects.add(
Rect.fromLTRB(
node.rect.left.roundToDouble(),
node.rect.top.roundToDouble(),
node.rect.right.roundToDouble(),
node.rect.bottom.roundToDouble(),
),
);
return true;
});
return true;
});
// Test that the semantics node rect sizes are correct.
expect(rects, <Rect>[
const Rect.fromLTRB(75.0, 276.0, 123.0, 324.0),
const Rect.fromLTRB(226.0, 276.0, 274.0, 324.0)
]);
});
testWidgets('Range Slider Semantics - rtl', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: Theme(
data: ThemeData.light(),
child: Directionality(
textDirection: TextDirection.rtl,
child: Material(
child: RangeSlider(
values: const RangeValues(10.0, 30.0),
max: 100.0,
onChanged: (RangeValues v) { },
),
),
),
),
),
);
await tester.pumpAndSettle();
final SemanticsNode semanticsNode = tester.getSemantics(find.byType(RangeSlider));
expect(
semanticsNode,
matchesSemantics(
scopesRoute: true,
children:<Matcher>[
matchesSemantics(
children: <Matcher>[
matchesSemantics(
isEnabled: true,
isSlider: true,
hasEnabledState: true,
hasIncreaseAction: true,
hasDecreaseAction: true,
value: '10%',
increasedValue: '15%',
decreasedValue: '5%',
),
matchesSemantics(
isEnabled: true,
isSlider: true,
hasEnabledState: true,
hasIncreaseAction: true,
hasDecreaseAction: true,
value: '30%',
increasedValue: '35%',
decreasedValue: '25%',
),
],
),
],
),
);
// TODO(tahatesser): This is a workaround for matching
// the semantics node rects by avoiding floating point errors.
// https://github.com/flutter/flutter/issues/115079
// Get semantics node rects.
final List<Rect> rects = <Rect>[];
semanticsNode.visitChildren((SemanticsNode node) {
node.visitChildren((SemanticsNode node) {
// Round rect values to avoid floating point errors.
rects.add(
Rect.fromLTRB(
node.rect.left.roundToDouble(),
node.rect.top.roundToDouble(),
node.rect.right.roundToDouble(),
node.rect.bottom.roundToDouble(),
),
);
return true;
});
return true;
});
// Test that the semantics node rect sizes are correct.
expect(rects, <Rect>[
const Rect.fromLTRB(526.0, 276.0, 574.0, 324.0),
const Rect.fromLTRB(677.0, 276.0, 725.0, 324.0)
]);
}); });
testWidgets('Range Slider implements debugFillProperties', (WidgetTester tester) async { testWidgets('Range Slider implements debugFillProperties', (WidgetTester tester) async {
......
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