Skip to content

Commit 4f959b9

Browse files
authored
Fix Slider onChanged callback order & never calls onChangeStart on SliderInteraction.slideOnly allowed interaction (#136720)
fixes [Slider will call onChanged before onChangeStart when sliding.](flutter/flutter#136707) This fixes a regression from flutter/flutter#121483 ### Code sample <details> <summary>expand to view the code sample</summary> ```dart import 'package:flutter/material.dart'; void main() => runApp(const MyApp()); class MyApp extends StatelessWidget { const MyApp({super.key}); @OverRide Widget build(BuildContext context) { return const MaterialApp( debugShowCheckedModeBanner: false, home: Example(), ); } } class Example extends StatefulWidget { const Example({super.key}); @OverRide State<Example> createState() => _ExampleState(); } class _ExampleState extends State<Example> { double _sliderValue = 0.5; @OverRide Widget build(BuildContext context) { return Scaffold( body: Center( child: Slider( // allowedInteraction: SliderInteraction.tapAndSlide, // allowedInteraction: SliderInteraction.tapOnly, // allowedInteraction: SliderInteraction.slideOnly // allowedInteraction: SliderInteraction.slideThumb, value: _sliderValue, onChangeStart: (newValue) { print("onChangeStart ......"); }, onChanged: (newValue) { print("onChanged ......"); setState(() { _sliderValue = newValue; }); }, onChangeEnd: (newValue) { print("onChangeEnd ......"); }, ), ), ); } } ``` </details>
1 parent d195d90 commit 4f959b9

File tree

2 files changed

+80
-5
lines changed

2 files changed

+80
-5
lines changed

packages/flutter/lib/src/material/slider.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,21 +1494,21 @@ class _RenderSlider extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
14941494
case SliderInteraction.tapOnly:
14951495
_active = true;
14961496
_currentDragValue = _getValueFromGlobalPosition(globalPosition);
1497-
onChanged!(_discretize(_currentDragValue));
14981497
case SliderInteraction.slideThumb:
14991498
if (_isPointerOnOverlay(globalPosition)) {
15001499
_active = true;
15011500
_currentDragValue = value;
15021501
}
15031502
case SliderInteraction.slideOnly:
1504-
break;
1503+
onChangeStart?.call(_discretize(value));
15051504
}
15061505

15071506
if (_active) {
15081507
// We supply the *current* value as the start location, so that if we have
15091508
// a tap, it consists of a call to onChangeStart with the previous value and
15101509
// a call to onChangeEnd with the new value.
15111510
onChangeStart?.call(_discretize(value));
1511+
onChanged!(_discretize(_currentDragValue));
15121512
_state.overlayController.forward();
15131513
if (showValueIndicator) {
15141514
_state.valueIndicatorController.forward();

packages/flutter/test/material/slider_test.dart

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3886,6 +3886,7 @@ void main() {
38863886
// (slider's left padding (overlayRadius), windowHeight / 2)
38873887
const Offset startOfTheSliderTrack = Offset(24, 300);
38883888
const Offset centerOfTheSlideTrack = Offset(400, 300);
3889+
final List<String> logs = <String>[];
38893890

38903891
Widget buildWidget() => MaterialApp(
38913892
home: Material(
@@ -3895,11 +3896,18 @@ void main() {
38953896
value: value,
38963897
key: sliderKey,
38973898
allowedInteraction: SliderInteraction.tapOnly,
3899+
onChangeStart: (double newValue) {
3900+
logs.add('onChangeStart');
3901+
},
38983902
onChanged: (double newValue) {
3903+
logs.add('onChanged');
38993904
setState(() {
39003905
value = newValue;
39013906
});
39023907
},
3908+
onChangeEnd: (double newValue) {
3909+
logs.add('onChangeEnd');
3910+
},
39033911
);
39043912
}),
39053913
),
@@ -3909,26 +3917,35 @@ void main() {
39093917
// allow tap only
39103918
await tester.pumpWidget(buildWidget());
39113919

3920+
expect(logs, isEmpty);
3921+
39123922
// test tap
39133923
final TestGesture gesture = await tester.startGesture(centerOfTheSlideTrack);
39143924
await tester.pump();
39153925
// changes from 1.0 -> 0.5
39163926
expect(value, 0.5);
3927+
expect(logs, <String>['onChangeStart', 'onChanged']);
39173928

39183929
// test slide
39193930
await gesture.moveTo(startOfTheSliderTrack);
39203931
await tester.pump();
39213932
// has no effect, remains 0.5
39223933
expect(value, 0.5);
3934+
expect(logs, <String>['onChangeStart', 'onChanged']);
3935+
3936+
await gesture.up();
3937+
await tester.pump();
3938+
expect(logs, <String>['onChangeStart', 'onChanged', 'onChangeEnd']);
39233939
});
39243940

3925-
testWidgetsWithLeakTracking('SliderInteraction.tapAndSlide', (WidgetTester tester) async {
3941+
testWidgetsWithLeakTracking('SliderInteraction.tapAndSlide (default)', (WidgetTester tester) async {
39263942
double value = 1.0;
39273943
final Key sliderKey = UniqueKey();
39283944
// (slider's left padding (overlayRadius), windowHeight / 2)
39293945
const Offset startOfTheSliderTrack = Offset(24, 300);
39303946
const Offset centerOfTheSlideTrack = Offset(400, 300);
39313947
const Offset endOfTheSliderTrack = Offset(800 - 24, 300);
3948+
final List<String> logs = <String>[];
39323949

39333950
Widget buildWidget() => MaterialApp(
39343951
home: Material(
@@ -3937,12 +3954,18 @@ void main() {
39373954
return Slider(
39383955
value: value,
39393956
key: sliderKey,
3940-
// allowedInteraction: SliderInteraction.tapAndSlide, // default
3957+
onChangeStart: (double newValue) {
3958+
logs.add('onChangeStart');
3959+
},
39413960
onChanged: (double newValue) {
3961+
logs.add('onChanged');
39423962
setState(() {
39433963
value = newValue;
39443964
});
39453965
},
3966+
onChangeEnd: (double newValue) {
3967+
logs.add('onChangeEnd');
3968+
},
39463969
);
39473970
}),
39483971
),
@@ -3951,11 +3974,14 @@ void main() {
39513974

39523975
await tester.pumpWidget(buildWidget());
39533976

3977+
expect(logs, isEmpty);
3978+
39543979
// Test tap.
39553980
final TestGesture gesture = await tester.startGesture(centerOfTheSlideTrack);
39563981
await tester.pump();
39573982
// changes from 1.0 -> 0.5
39583983
expect(value, 0.5);
3984+
expect(logs, <String>['onChangeStart', 'onChanged']);
39593985

39603986
// test slide
39613987
await gesture.moveTo(startOfTheSliderTrack);
@@ -3966,6 +3992,12 @@ void main() {
39663992
await tester.pump();
39673993
// changes from 0.0 -> 1.0
39683994
expect(value, 1.0);
3995+
expect(logs, <String>['onChangeStart', 'onChanged', 'onChanged', 'onChanged']);
3996+
3997+
await gesture.up();
3998+
await tester.pump();
3999+
4000+
expect(logs, <String>['onChangeStart', 'onChanged', 'onChanged', 'onChanged', 'onChangeEnd']);
39694001
});
39704002

39714003
testWidgetsWithLeakTracking('SliderInteraction.slideOnly', (WidgetTester tester) async {
@@ -3975,6 +4007,7 @@ void main() {
39754007
const Offset startOfTheSliderTrack = Offset(24, 300);
39764008
const Offset centerOfTheSlideTrack = Offset(400, 300);
39774009
const Offset endOfTheSliderTrack = Offset(800 - 24, 300);
4010+
final List<String> logs = <String>[];
39784011

39794012
Widget buildApp() {
39804013
return MaterialApp(
@@ -3985,11 +4018,18 @@ void main() {
39854018
value: value,
39864019
key: sliderKey,
39874020
allowedInteraction: SliderInteraction.slideOnly,
4021+
onChangeStart: (double newValue) {
4022+
logs.add('onChangeStart');
4023+
},
39884024
onChanged: (double newValue) {
4025+
logs.add('onChanged');
39894026
setState(() {
39904027
value = newValue;
39914028
});
39924029
},
4030+
onChangeEnd: (double newValue) {
4031+
logs.add('onChangeEnd');
4032+
},
39934033
);
39944034
}),
39954035
),
@@ -3999,11 +4039,14 @@ void main() {
39994039

40004040
await tester.pumpWidget(buildApp());
40014041

4042+
expect(logs, isEmpty);
4043+
40024044
// test tap
40034045
final TestGesture gesture = await tester.startGesture(centerOfTheSlideTrack);
40044046
await tester.pump();
40054047
// has no effect as tap is disabled, remains 1.0
40064048
expect(value, 1.0);
4049+
expect(logs, <String>['onChangeStart']);
40074050

40084051
// test slide
40094052
await gesture.moveTo(startOfTheSliderTrack);
@@ -4014,6 +4057,12 @@ void main() {
40144057
await tester.pump();
40154058
// changes from 0.0 -> 1.0
40164059
expect(value, 1.0);
4060+
expect(logs, <String>['onChangeStart', 'onChanged', 'onChanged']);
4061+
4062+
await gesture.up();
4063+
await tester.pump();
4064+
4065+
expect(logs, <String>['onChangeStart', 'onChanged', 'onChanged', 'onChangeEnd']);
40174066
});
40184067

40194068
testWidgetsWithLeakTracking('SliderInteraction.slideThumb', (WidgetTester tester) async {
@@ -4023,6 +4072,7 @@ void main() {
40234072
const Offset startOfTheSliderTrack = Offset(24, 300);
40244073
const Offset centerOfTheSliderTrack = Offset(400, 300);
40254074
const Offset endOfTheSliderTrack = Offset(800 - 24, 300);
4075+
final List<String> logs = <String>[];
40264076

40274077
Widget buildApp() {
40284078
return MaterialApp(
@@ -4033,11 +4083,18 @@ void main() {
40334083
value: value,
40344084
key: sliderKey,
40354085
allowedInteraction: SliderInteraction.slideThumb,
4086+
onChangeStart: (double newValue) {
4087+
logs.add('onChangeStart');
4088+
},
40364089
onChanged: (double newValue) {
4090+
logs.add('onChanged');
40374091
setState(() {
40384092
value = newValue;
40394093
});
40404094
},
4095+
onChangeEnd: (double newValue) {
4096+
logs.add('onChangeEnd');
4097+
},
40414098
);
40424099
}),
40434100
),
@@ -4047,40 +4104,58 @@ void main() {
40474104

40484105
await tester.pumpWidget(buildApp());
40494106

4107+
expect(logs, isEmpty);
4108+
40504109
// test tap
40514110
final TestGesture gesture = await tester.startGesture(centerOfTheSliderTrack);
40524111
await tester.pump();
40534112
// has no effect, remains 1.0
40544113
expect(value, 1.0);
4114+
expect(logs, isEmpty);
40554115

40564116
// test slide
40574117
await gesture.moveTo(startOfTheSliderTrack);
40584118
await tester.pump();
40594119
// has no effect, remains 1.0
40604120
expect(value, 1.0);
4121+
expect(logs, isEmpty);
40614122

40624123
// test slide thumb
40634124
await gesture.up();
40644125
await gesture.down(endOfTheSliderTrack); // where the thumb is
40654126
await tester.pump();
40664127
// has no effect, remains 1.0
40674128
expect(value, 1.0);
4129+
expect(logs, <String>['onChangeStart']);
4130+
40684131
await gesture.moveTo(centerOfTheSliderTrack);
40694132
await tester.pump();
40704133
// changes from 1.0 -> 0.5
40714134
expect(value, 0.5);
4135+
expect(logs, <String>['onChangeStart', 'onChanged']);
40724136

40734137
// test tap inside overlay but not on thumb, then slide
40744138
await gesture.up();
40754139
// default overlay radius is 12, so 10 is inside the overlay
40764140
await gesture.down(centerOfTheSliderTrack.translate(-10, 0));
40774141
await tester.pump();
4078-
// has no effect, remains 1.0
4142+
// changes from 1.0 -> 0.5
40794143
expect(value, 0.5);
4144+
expect(logs, <String>['onChangeStart', 'onChanged', 'onChangeEnd', 'onChangeStart']);
4145+
40804146
await gesture.moveTo(endOfTheSliderTrack.translate(-10, 0));
40814147
await tester.pump();
40824148
// changes from 0.5 -> 1.0
40834149
expect(value, 1.0);
4150+
expect(logs, <String>['onChangeStart', 'onChanged', 'onChangeEnd', 'onChangeStart', 'onChanged']);
4151+
4152+
await gesture.up();
4153+
await tester.pump();
4154+
4155+
expect(
4156+
logs,
4157+
<String>['onChangeStart', 'onChanged', 'onChangeEnd', 'onChangeStart', 'onChanged', 'onChangeEnd'],
4158+
);
40844159
});
40854160
});
40864161
}

0 commit comments

Comments
 (0)