From 5451ea6e8f9ab4d39bd066d962a92a75f72c84f9 Mon Sep 17 00:00:00 2001 From: Devin Date: Thu, 25 May 2023 00:13:15 +0200 Subject: [PATCH] `Slider.onChangeStart` & `Slider.onChangeEnd` are not called on keyboard shortcuts (#126896) fixes https://github.com/flutter/flutter/issues/123315 -------- This PR makes changes to the _actionHandler function used on the Slider.Dart Widget for Key Events. It ensures onChangeStart is called at the beginning of a Key Event and onChangeEnd at the end of one. This PR includes a test for the changes made. I ran all existing tests after my changes were made and they passed. --- packages/flutter/lib/src/material/slider.dart | 23 ++- .../flutter/test/material/slider_test.dart | 136 ++++++++++++++---- 2 files changed, 129 insertions(+), 30 deletions(-) diff --git a/packages/flutter/lib/src/material/slider.dart b/packages/flutter/lib/src/material/slider.dart index 7f5bcaa1be..e4f8efb93b 100644 --- a/packages/flutter/lib/src/material/slider.dart +++ b/packages/flutter/lib/src/material/slider.dart @@ -684,6 +684,7 @@ class _SliderState extends State with TickerProviderStateMixin { void _actionHandler(_AdjustSliderIntent intent) { final _RenderSlider renderSlider = _renderObjectKey.currentContext!.findRenderObject()! as _RenderSlider; final TextDirection textDirection = Directionality.of(_renderObjectKey.currentContext!); + switch (intent.type) { case _SliderAdjustmentType.right: switch (textDirection) { @@ -1802,15 +1803,33 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin { void increaseAction() { if (isInteractive) { - onChanged!(clampDouble(value + _semanticActionUnit, 0.0, 1.0)); + onChangeStart!(currentValue); + final double increase = increaseValue(); + onChanged!(increase); + onChangeEnd!(increase); } } void decreaseAction() { if (isInteractive) { - onChanged!(clampDouble(value - _semanticActionUnit, 0.0, 1.0)); + onChangeStart!(currentValue); + final double decrease = decreaseValue(); + onChanged!(decrease); + onChangeEnd!(decrease); } } + + double get currentValue { + return clampDouble(value, 0.0, 1.0); + } + + double increaseValue() { + return clampDouble(value + _semanticActionUnit, 0.0, 1.0); + } + + double decreaseValue() { + return clampDouble(value - _semanticActionUnit, 0.0, 1.0); + } } class _AdjustSliderIntent extends Intent { diff --git a/packages/flutter/test/material/slider_test.dart b/packages/flutter/test/material/slider_test.dart index c4b01ff4ce..6430b0d314 100644 --- a/packages/flutter/test/material/slider_test.dart +++ b/packages/flutter/test/material/slider_test.dart @@ -2125,17 +2125,29 @@ void main() { testWidgets('Slider can be incremented and decremented by keyboard shortcuts - LTR', (WidgetTester tester) async { tester.binding.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; - double value = 0.5; + double startValue = 0.0; + double currentValue = 0.5; + double endValue = 0.0; await tester.pumpWidget( MaterialApp( home: Material( child: Center( child: StatefulBuilder(builder: (BuildContext context, StateSetter setState) { return Slider( - value: value, + value: currentValue, + onChangeStart: (double newValue) { + setState(() { + startValue = newValue; + }); + }, onChanged: (double newValue) { setState(() { - value = newValue; + currentValue = newValue; + }); + }, + onChangeEnd: (double newValue) { + setState(() { + endValue = newValue; }); }, autofocus: true, @@ -2149,34 +2161,54 @@ void main() { await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); await tester.pumpAndSettle(); - expect(value, 0.55); + expect(startValue, 0.5); + expect(currentValue, 0.55); + expect(endValue, 0.55); await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft); await tester.pumpAndSettle(); - expect(value, 0.5); + expect(startValue, 0.55); + expect(currentValue, 0.5); + expect(endValue, 0.5); await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); await tester.pumpAndSettle(); - expect(value, 0.55); + expect(startValue, 0.5); + expect(currentValue, 0.55); + expect(endValue, 0.55); await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); - expect(value, 0.5); + expect(startValue, 0.55); + expect(currentValue, 0.5); + expect(endValue, 0.5); }, variant: const TargetPlatformVariant({ TargetPlatform.android, TargetPlatform.fuchsia, TargetPlatform.linux, TargetPlatform.windows })); testWidgets('Slider can be incremented and decremented by keyboard shortcuts - LTR', (WidgetTester tester) async { tester.binding.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; - double value = 0.5; + double startValue = 0.0; + double currentValue = 0.5; + double endValue = 0.0; await tester.pumpWidget( MaterialApp( home: Material( child: Center( child: StatefulBuilder(builder: (BuildContext context, StateSetter setState) { return Slider( - value: value, + value: currentValue, + onChangeStart: (double newValue) { + setState(() { + startValue = newValue; + }); + }, onChanged: (double newValue) { setState(() { - value = newValue; + currentValue = newValue; + }); + }, + onChangeEnd: (double newValue) { + setState(() { + endValue = newValue; }); }, autofocus: true, @@ -2190,24 +2222,34 @@ void main() { await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); await tester.pumpAndSettle(); - expect(value, 0.6); + expect(startValue, 0.5); + expect(currentValue, 0.6); + expect(endValue, 0.6); await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft); await tester.pumpAndSettle(); - expect(value, 0.5); + expect(startValue, 0.6); + expect(currentValue, 0.5); + expect(endValue, 0.5); await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); await tester.pumpAndSettle(); - expect(value, 0.6); + expect(startValue, 0.5); + expect(currentValue, 0.6); + expect(endValue, 0.6); await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); - expect(value, 0.5); + expect(startValue, 0.6); + expect(currentValue, 0.5); + expect(endValue, 0.5); }, variant: const TargetPlatformVariant({ TargetPlatform.iOS, TargetPlatform.macOS })); testWidgets('Slider can be incremented and decremented by keyboard shortcuts - RTL', (WidgetTester tester) async { tester.binding.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; - double value = 0.5; + double startValue = 0.0; + double currentValue = 0.5; + double endValue = 0.0; await tester.pumpWidget( MaterialApp( home: Material( @@ -2216,10 +2258,20 @@ void main() { return Directionality( textDirection: TextDirection.rtl, child: Slider( - value: value, + value: currentValue, + onChangeStart: (double newValue) { + setState(() { + startValue = newValue; + }); + }, onChanged: (double newValue) { setState(() { - value = newValue; + currentValue = newValue; + }); + }, + onChangeEnd: (double newValue) { + setState(() { + endValue = newValue; }); }, autofocus: true, @@ -2234,24 +2286,34 @@ void main() { await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); await tester.pumpAndSettle(); - expect(value, 0.45); + expect(startValue, 0.5); + expect(currentValue, 0.45); + expect(endValue, 0.45); await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft); await tester.pumpAndSettle(); - expect(value, 0.5); + expect(startValue, 0.45); + expect(currentValue, 0.5); + expect(endValue, 0.5); await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); await tester.pumpAndSettle(); - expect(value, 0.55); + expect(startValue, 0.5); + expect(currentValue, 0.55); + expect(endValue, 0.55); await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); - expect(value, 0.5); + expect(startValue, 0.55); + expect(currentValue, 0.5); + expect(endValue, 0.5); }, variant: const TargetPlatformVariant({ TargetPlatform.android, TargetPlatform.fuchsia, TargetPlatform.linux, TargetPlatform.windows })); testWidgets('Slider can be incremented and decremented by keyboard shortcuts - RTL', (WidgetTester tester) async { tester.binding.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; - double value = 0.5; + double startValue = 0.0; + double currentValue = 0.5; + double endValue = 0.0; await tester.pumpWidget( MaterialApp( home: Material( @@ -2260,10 +2322,20 @@ void main() { return Directionality( textDirection: TextDirection.rtl, child: Slider( - value: value, + value: currentValue, + onChangeStart: (double newValue) { + setState(() { + startValue = newValue; + }); + }, onChanged: (double newValue) { setState(() { - value = newValue; + currentValue = newValue; + }); + }, + onChangeEnd: (double newValue) { + setState(() { + endValue = newValue; }); }, autofocus: true, @@ -2278,19 +2350,27 @@ void main() { await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); await tester.pumpAndSettle(); - expect(value, 0.4); + expect(startValue, 0.5); + expect(currentValue, 0.4); + expect(endValue, 0.4); await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft); await tester.pumpAndSettle(); - expect(value, 0.5); + expect(startValue, 0.4); + expect(currentValue, 0.5); + expect(endValue, 0.5); await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); await tester.pumpAndSettle(); - expect(value, 0.6); + expect(startValue, 0.5); + expect(currentValue, 0.6); + expect(endValue, 0.6); await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); - expect(value, 0.5); + expect(startValue, 0.6); + expect(currentValue, 0.5); + expect(endValue, 0.5); }, variant: const TargetPlatformVariant({ TargetPlatform.iOS, TargetPlatform.macOS })); testWidgets('In directional nav, Slider can be navigated out of by using up and down arrows', (WidgetTester tester) async {