Unverified Commit 400136b0 authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Fix `Slider` overlay and value indicator interactive behavior on desktop. (#113543)

parent 2a59bd52
......@@ -609,6 +609,7 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin {
final double lerpValue = _lerp(value);
if (lerpValue != widget.value) {
widget.onChanged!(lerpValue);
_focusNode?.requestFocus();
}
}
......@@ -1090,6 +1091,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
late TapGestureRecognizer _tap;
bool _active = false;
double _currentDragValue = 0.0;
Rect? overlayRect;
// This rect is used in gesture calculations, where the gesture coordinates
// are relative to the sliders origin. Therefore, the offset is passed as
......@@ -1258,7 +1260,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
return;
}
_hasFocus = value;
_updateForFocusOrHover(_hasFocus);
_updateForFocus(_hasFocus);
markNeedsSemanticsUpdate();
}
......@@ -1271,11 +1273,24 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
return;
}
_hovering = value;
_updateForFocusOrHover(_hovering);
_updateForHover(_hovering);
}
void _updateForFocusOrHover(bool hasFocusOrIsHovering) {
if (hasFocusOrIsHovering) {
/// True if the slider is interactive and the slider thumb is being
/// hovered over by a pointer.
bool _hoveringThumb = false;
bool get hoveringThumb => _hoveringThumb;
set hoveringThumb(bool value) {
assert(value != null);
if (value == _hoveringThumb) {
return;
}
_hoveringThumb = value;
_updateForHover(_hovering);
}
void _updateForFocus(bool focused) {
if (focused) {
_state.overlayController.forward();
if (showValueIndicator) {
_state.valueIndicatorController.forward();
......@@ -1288,6 +1303,18 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
}
}
void _updateForHover(bool hovered) {
// Only show overlay when pointer is hovering the thumb.
if (hovered && hoveringThumb) {
_state.overlayController.forward();
} else {
// Only remove overlay when Slider is unfocused.
if (!hasFocus) {
_state.overlayController.reverse();
}
}
}
bool get showValueIndicator {
switch (_sliderTheme.showValueIndicator!) {
case ShowValueIndicator.onlyForDiscrete:
......@@ -1404,7 +1431,7 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
_state.interactionTimer?.cancel();
_state.interactionTimer = Timer(_minimumInteractionTime * timeDilation, () {
_state.interactionTimer = null;
if (!_active &&
if (!_active && !hasFocus &&
_state.valueIndicatorController.status == AnimationStatus.completed) {
_state.valueIndicatorController.reverse();
}
......@@ -1422,7 +1449,9 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
onChangeEnd?.call(_discretize(_currentDragValue));
_active = false;
_currentDragValue = 0.0;
_state.overlayController.reverse();
if (!hasFocus) {
_state.overlayController.reverse();
}
if (showValueIndicator && _state.interactionTimer == null) {
_state.valueIndicatorController.reverse();
......@@ -1476,6 +1505,9 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
_drag.addPointer(event);
_tap.addPointer(event);
}
if (isInteractive && overlayRect != null) {
hoveringThumb = overlayRect!.contains(event.localPosition);
}
}
@override
......@@ -1529,6 +1561,10 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
isDiscrete: isDiscrete,
);
final Offset thumbCenter = Offset(trackRect.left + visualPosition * trackRect.width, trackRect.center.dy);
if (isInteractive) {
final Size overlaySize = sliderTheme.overlayShape!.getPreferredSize(isInteractive, false);
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;
_sliderTheme.trackShape!.paint(
......
......@@ -1748,7 +1748,7 @@ void main() {
paints..circle(color: Colors.orange[500]),
);
// Check that the overlay does not show when focused and disabled.
// Check that the overlay does not show when unfocused and disabled.
await tester.pumpWidget(buildApp(enabled: false));
await tester.pumpAndSettle();
expect(focusNode.hasPrimaryFocus, isFalse);
......@@ -1990,10 +1990,10 @@ void main() {
await drag.up();
await tester.pumpAndSettle();
// Slider does not have an overlay when stopped dragging.
// Slider still has overlay when stopped dragging.
expect(
Material.of(tester.element(find.byType(Slider))),
isNot(paints..circle(color: Colors.lime[500])),
paints..circle(color: Colors.lime[500]),
);
});
......@@ -3177,4 +3177,208 @@ void main() {
expect(sliderEnd, true);
expect(dragStarted, false);
});
testWidgets('Overlay appear only when hovered on the thumb on desktop', (WidgetTester tester) async {
double value = 0.5;
const Color overlayColor = Color(0xffff0000);
Widget buildApp({bool enabled = true}) {
return MaterialApp(
home: Material(
child: Center(
child: StatefulBuilder(builder: (BuildContext context, StateSetter setState) {
return Slider(
value: value,
overlayColor: const MaterialStatePropertyAll<Color?>(overlayColor),
onChanged: enabled
? (double newValue) {
setState(() {
value = newValue;
});
}
: null,
);
}),
),
),
);
}
await tester.pumpWidget(buildApp());
// Slider does not have overlay when enabled and not hovered.
await tester.pumpAndSettle();
expect(
Material.of(tester.element(find.byType(Slider))),
isNot(paints..circle(color: overlayColor)),
);
// Hover on the slider but outside the thumb.
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.moveTo(tester.getTopLeft(find.byType(Slider)));
await tester.pumpWidget(buildApp());
await tester.pumpAndSettle();
expect(
Material.of(tester.element(find.byType(Slider))),
isNot(paints..circle(color: overlayColor)),
);
// Hover on the thumb.
await gesture.moveTo(tester.getCenter(find.byType(Slider)));
await tester.pumpAndSettle();
expect(
Material.of(tester.element(find.byType(Slider))),
paints..circle(color: overlayColor),
);
// Hover on the slider but outside the thumb.
await gesture.moveTo(tester.getBottomRight(find.byType(Slider)));
await tester.pumpAndSettle();
expect(
Material.of(tester.element(find.byType(Slider))),
isNot(paints..circle(color: overlayColor)),
);
}, variant: TargetPlatformVariant.desktop());
testWidgets('Overlay remains when Slider is in focus on desktop', (WidgetTester tester) async {
double value = 0.5;
const Color overlayColor = Color(0xffff0000);
final FocusNode focusNode = FocusNode();
Widget buildApp({bool enabled = true}) {
return MaterialApp(
home: Material(
child: Center(
child: StatefulBuilder(builder: (BuildContext context, StateSetter setState) {
return Slider(
value: value,
focusNode: focusNode,
overlayColor: const MaterialStatePropertyAll<Color?>(overlayColor),
onChanged: enabled
? (double newValue) {
setState(() {
value = newValue;
});
}
: null,
);
}),
),
),
);
}
await tester.pumpWidget(buildApp());
// Slider does not have overlay when enabled and not tapped.
await tester.pumpAndSettle();
expect(focusNode.hasFocus, false);
expect(
Material.of(tester.element(find.byType(Slider))),
isNot(paints..circle(color: overlayColor)),
);
final Offset sliderCenter = tester.getCenter(find.byType(Slider));
Offset tapLocation = Offset(sliderCenter.dx + 50, sliderCenter.dy);
// Tap somewhere to bring overlay.
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.down(tapLocation);
await gesture.up();
focusNode.requestFocus();
await tester.pumpAndSettle();
expect(focusNode.hasFocus, true);
expect(
Material.of(tester.element(find.byType(Slider))),
paints..circle(color: overlayColor),
);
tapLocation = Offset(sliderCenter.dx - 50, sliderCenter.dy);
await gesture.down(tapLocation);
await gesture.up();
await tester.pumpAndSettle();
expect(focusNode.hasFocus, true);
expect(
Material.of(tester.element(find.byType(Slider))),
paints..circle(color: overlayColor),
);
focusNode.unfocus();
await tester.pumpAndSettle();
expect(focusNode.hasFocus, false);
expect(
Material.of(tester.element(find.byType(Slider))),
isNot(paints..circle(color: overlayColor)),
);
}, variant: TargetPlatformVariant.desktop());
testWidgets('Value indicator remains when Slider is in focus on desktop', (WidgetTester tester) async {
double value = 0.5;
final FocusNode focusNode = FocusNode();
Widget buildApp({bool enabled = true}) {
return MaterialApp(
theme: ThemeData(
sliderTheme: const SliderThemeData(
showValueIndicator: ShowValueIndicator.always,
),
),
home: Material(
child: Center(
child: StatefulBuilder(builder: (BuildContext context, StateSetter setState) {
return Slider(
value: value,
focusNode: focusNode,
divisions: 5,
label: value.toStringAsFixed(1),
onChanged: enabled
? (double newValue) {
setState(() {
value = newValue;
});
}
: null,
);
}),
),
),
);
}
await tester.pumpWidget(buildApp());
// Slider does not show value indicator without focus.
await tester.pumpAndSettle();
expect(focusNode.hasFocus, false);
RenderBox valueIndicatorBox = tester.renderObject(find.byType(Overlay));
expect(
valueIndicatorBox,
isNot(paints..path(color: const Color(0xff000000))..paragraph()),
);
final Offset sliderCenter = tester.getCenter(find.byType(Slider));
final Offset tapLocation = Offset(sliderCenter.dx + 50, sliderCenter.dy);
// Tap somewhere to bring value indicator.
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.down(tapLocation);
await gesture.up();
focusNode.requestFocus();
await tester.pumpAndSettle();
expect(focusNode.hasFocus, true);
valueIndicatorBox = tester.renderObject(find.byType(Overlay));
expect(
valueIndicatorBox,
paints..path(color: const Color(0xff000000))..paragraph(),
);
focusNode.unfocus();
await tester.pumpAndSettle();
expect(focusNode.hasFocus, false);
expect(
valueIndicatorBox,
isNot(paints..path(color: const Color(0xff000000))..paragraph()),
);
}, variant: TargetPlatformVariant.desktop());
}
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