Skip to content

Commit

Permalink
Fix RangeSlider semantics node size (#114999)
Browse files Browse the repository at this point in the history 
* Fix `RangeSlider` semantics node size and add RTL semantics test

* Add TODO comments
  • Loading branch information
@TahaTesser
TahaTesser committed Nov 10, 2022
1 parent 09a4f23 commit 1f891a0
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 22 deletions.
40 changes: 25 additions & 15 deletions packages/flutter/lib/src/material/range_slider.dart
View file
Expand Up @@ -840,6 +840,8 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
late TapGestureRecognizer _tap;
bool _active = false;
late RangeValues _newValues;
late Offset _startThumbCenter;
late Offset _endThumbCenter;

bool get isEnabled => onChanged != null;

Expand Down Expand Up @@ -1303,8 +1305,8 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
sliderTheme: _sliderTheme,
isDiscrete: isDiscrete,
);
final Offset startThumbCenter = Offset(trackRect.left + startVisualPosition * trackRect.width, trackRect.center.dy);
final Offset endThumbCenter = Offset(trackRect.left + endVisualPosition * trackRect.width, trackRect.center.dy);
_startThumbCenter = Offset(trackRect.left + startVisualPosition * trackRect.width, trackRect.center.dy);
_endThumbCenter = Offset(trackRect.left + endVisualPosition * trackRect.width, trackRect.center.dy);

_sliderTheme.rangeTrackShape!.paint(
context,
Expand All @@ -1313,8 +1315,8 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
sliderTheme: _sliderTheme,
enableAnimation: _enableAnimation,
textDirection: _textDirection,
startThumbCenter: startThumbCenter,
endThumbCenter: endThumbCenter,
startThumbCenter: _startThumbCenter,
endThumbCenter: _endThumbCenter,
isDiscrete: isDiscrete,
isEnabled: isEnabled,
);
Expand All @@ -1327,7 +1329,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
if (startThumbSelected) {
_sliderTheme.overlayShape!.paint(
context,
startThumbCenter,
_startThumbCenter,
activationAnimation: _overlayAnimation,
enableAnimation: _enableAnimation,
isDiscrete: isDiscrete,
Expand All @@ -1343,7 +1345,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
if (endThumbSelected) {
_sliderTheme.overlayShape!.paint(
context,
endThumbCenter,
_endThumbCenter,
activationAnimation: _overlayAnimation,
enableAnimation: _enableAnimation,
isDiscrete: isDiscrete,
Expand Down Expand Up @@ -1381,21 +1383,21 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
sliderTheme: _sliderTheme,
enableAnimation: _enableAnimation,
textDirection: _textDirection,
startThumbCenter: startThumbCenter,
endThumbCenter: endThumbCenter,
startThumbCenter: _startThumbCenter,
endThumbCenter: _endThumbCenter,
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 Thumb bottomThumb = isLastThumbStart ? Thumb.end : Thumb.start;
final Thumb topThumb = isLastThumbStart ? Thumb.start : Thumb.end;
final Offset bottomThumbCenter = isLastThumbStart ? endThumbCenter : startThumbCenter;
final Offset topThumbCenter = isLastThumbStart ? startThumbCenter : endThumbCenter;
final Offset bottomThumbCenter = isLastThumbStart ? _endThumbCenter : _startThumbCenter;
final Offset topThumbCenter = isLastThumbStart ? _startThumbCenter : _endThumbCenter;
final TextPainter bottomLabelPainter = isLastThumbStart ? _endLabelPainter : _startLabelPainter;
final TextPainter topLabelPainter = isLastThumbStart ? _startLabelPainter : _endLabelPainter;
final double bottomValue = isLastThumbStart ? endValue : startValue;
Expand Down Expand Up @@ -1441,15 +1443,15 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
if (shouldPaintValueIndicators) {
final double startOffset = sliderTheme.rangeValueIndicatorShape!.getHorizontalShift(
parentBox: this,
center: startThumbCenter,
center: _startThumbCenter,
labelPainter: _startLabelPainter,
activationAnimation: _valueIndicatorAnimation,
textScaleFactor: textScaleFactor,
sizeWithOverflow: resolvedscreenSize,
);
final double endOffset = sliderTheme.rangeValueIndicatorShape!.getHorizontalShift(
parentBox: this,
center: endThumbCenter,
center: _endThumbCenter,
labelPainter: _endLabelPainter,
activationAnimation: _valueIndicatorAnimation,
textScaleFactor: textScaleFactor,
Expand Down Expand Up @@ -1575,8 +1577,16 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
);

// Split the semantics node area between the start and end nodes.
final Rect leftRect = Rect.fromPoints(node.rect.topLeft, node.rect.bottomCenter);
final Rect rightRect = Rect.fromPoints(node.rect.topCenter, node.rect.bottomRight);
final Rect leftRect = Rect.fromCenter(
center: _startThumbCenter,
width: kMinInteractiveDimension,
height: kMinInteractiveDimension,
);
final Rect rightRect = Rect.fromCenter(
center: _endThumbCenter,
width: kMinInteractiveDimension,
height: kMinInteractiveDimension,
);
switch (textDirection) {
case TextDirection.ltr:
_startSemanticsNode!.rect = leftRect;
Expand Down
125 changes: 118 additions & 7 deletions packages/flutter/test/material/range_slider_test.dart
View file
Expand Up @@ -1853,7 +1853,7 @@ void main() {
);
});

testWidgets('Range Slider Semantics', (WidgetTester tester) async {
testWidgets('Range Slider Semantics - ltr', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: Theme(
Expand All @@ -1862,7 +1862,7 @@ void main() {
textDirection: TextDirection.ltr,
child: Material(
child: RangeSlider(
values: const RangeValues(10.0, 12.0),
values: const RangeValues(10.0, 30.0),
max: 100.0,
onChanged: (RangeValues v) { },
),
Expand All @@ -1874,8 +1874,9 @@ void main() {

await tester.pumpAndSettle();

final SemanticsNode semanticsNode = tester.getSemantics(find.byType(RangeSlider));
expect(
tester.getSemantics(find.byType(RangeSlider)),
semanticsNode,
matchesSemantics(
scopesRoute: true,
children:<Matcher>[
Expand All @@ -1888,24 +1889,134 @@ void main() {
hasIncreaseAction: true,
hasDecreaseAction: true,
value: '10%',
increasedValue: '10%',
increasedValue: '15%',
decreasedValue: '5%',
rect: const Rect.fromLTRB(75.2, 276.0, 123.2, 324.0),
),
matchesSemantics(
isEnabled: true,
isSlider: true,
hasEnabledState: true,
hasIncreaseAction: true,
hasDecreaseAction: true,
value: '12%',
increasedValue: '17%',
decreasedValue: '12%',
value: '30%',
increasedValue: '35%',
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 {
Expand Down

0 comments on commit 1f891a0

Please sign in to comment.