Unverified Commit 07cc3f7a authored by Bruno Leroux's avatar Bruno Leroux Committed by GitHub

Form fields onChange callback should be called on reset (#134295)

## Description

This PR fixes form fields in order to call the `onChange` callback when the form is reset.

This change is based on the work done in https://github.com/flutter/flutter/pull/123108.

I considered adding the `onChange` callback to the `FormField` superclass but it would break existing code because two of the three subclasses defines the `onChange` callback with `ValueChanged<String>?` type and the third one defines it with `ValueChanged<String?>?`. 

## Related Issue

Fixes https://github.com/flutter/flutter/issues/123009.

## Tests

Adds 3 tests.
parent 518b7751
...@@ -133,7 +133,7 @@ class CupertinoTextFormFieldRow extends FormField<String> { ...@@ -133,7 +133,7 @@ class CupertinoTextFormFieldRow extends FormField<String> {
int? minLines, int? minLines,
bool expands = false, bool expands = false,
int? maxLength, int? maxLength,
ValueChanged<String>? onChanged, this.onChanged,
GestureTapCallback? onTap, GestureTapCallback? onTap,
VoidCallback? onEditingComplete, VoidCallback? onEditingComplete,
ValueChanged<String>? onFieldSubmitted, ValueChanged<String>? onFieldSubmitted,
...@@ -179,9 +179,7 @@ class CupertinoTextFormFieldRow extends FormField<String> { ...@@ -179,9 +179,7 @@ class CupertinoTextFormFieldRow extends FormField<String> {
void onChangedHandler(String value) { void onChangedHandler(String value) {
field.didChange(value); field.didChange(value);
if (onChanged != null) { onChanged?.call(value);
onChanged(value);
}
} }
return CupertinoFormRow( return CupertinoFormRow(
...@@ -260,6 +258,9 @@ class CupertinoTextFormFieldRow extends FormField<String> { ...@@ -260,6 +258,9 @@ class CupertinoTextFormFieldRow extends FormField<String> {
/// initialize its [TextEditingController.text] with [initialValue]. /// initialize its [TextEditingController.text] with [initialValue].
final TextEditingController? controller; final TextEditingController? controller;
/// {@macro flutter.material.TextFormField.onChanged}
final ValueChanged<String>? onChanged;
static Widget _defaultContextMenuBuilder(BuildContext context, EditableTextState editableTextState) { static Widget _defaultContextMenuBuilder(BuildContext context, EditableTextState editableTextState) {
return CupertinoAdaptiveTextSelectionToolbar.editableText( return CupertinoAdaptiveTextSelectionToolbar.editableText(
editableTextState: editableTextState, editableTextState: editableTextState,
...@@ -328,13 +329,11 @@ class _CupertinoTextFormFieldRowState extends FormFieldState<String> { ...@@ -328,13 +329,11 @@ class _CupertinoTextFormFieldRowState extends FormFieldState<String> {
@override @override
void reset() { void reset() {
// Set the controller value before calling super.reset() to let
// _handleControllerChanged suppress the change.
_effectiveController!.text = widget.initialValue!;
super.reset(); super.reset();
_cupertinoTextFormFieldRow.onChanged?.call(_effectiveController!.text);
if (widget.initialValue != null) {
setState(() {
_effectiveController!.text = widget.initialValue!;
});
}
} }
void _handleControllerChanged() { void _handleControllerChanged() {
......
...@@ -1736,13 +1736,12 @@ class DropdownButtonFormField<T> extends FormField<T> { ...@@ -1736,13 +1736,12 @@ class DropdownButtonFormField<T> extends FormField<T> {
} }
class _DropdownButtonFormFieldState<T> extends FormFieldState<T> { class _DropdownButtonFormFieldState<T> extends FormFieldState<T> {
DropdownButtonFormField<T> get _dropdownButtonFormField => widget as DropdownButtonFormField<T>;
@override @override
void didChange(T? value) { void didChange(T? value) {
super.didChange(value); super.didChange(value);
final DropdownButtonFormField<T> dropdownButtonFormField = widget as DropdownButtonFormField<T>; _dropdownButtonFormField.onChanged!(value);
assert(dropdownButtonFormField.onChanged != null);
dropdownButtonFormField.onChanged!(value);
} }
@override @override
...@@ -1752,4 +1751,10 @@ class _DropdownButtonFormFieldState<T> extends FormFieldState<T> { ...@@ -1752,4 +1751,10 @@ class _DropdownButtonFormFieldState<T> extends FormFieldState<T> {
setValue(widget.initialValue); setValue(widget.initialValue);
} }
} }
@override
void reset() {
super.reset();
_dropdownButtonFormField.onChanged!(value);
}
} }
...@@ -131,7 +131,7 @@ class TextFormField extends FormField<String> { ...@@ -131,7 +131,7 @@ class TextFormField extends FormField<String> {
int? minLines, int? minLines,
bool expands = false, bool expands = false,
int? maxLength, int? maxLength,
ValueChanged<String>? onChanged, this.onChanged,
GestureTapCallback? onTap, GestureTapCallback? onTap,
TapRegionCallback? onTapOutside, TapRegionCallback? onTapOutside,
VoidCallback? onEditingComplete, VoidCallback? onEditingComplete,
...@@ -193,9 +193,7 @@ class TextFormField extends FormField<String> { ...@@ -193,9 +193,7 @@ class TextFormField extends FormField<String> {
.applyDefaults(Theme.of(field.context).inputDecorationTheme); .applyDefaults(Theme.of(field.context).inputDecorationTheme);
void onChangedHandler(String value) { void onChangedHandler(String value) {
field.didChange(value); field.didChange(value);
if (onChanged != null) { onChanged?.call(value);
onChanged(value);
}
} }
return UnmanagedRestorationScope( return UnmanagedRestorationScope(
bucket: field.bucket, bucket: field.bucket,
...@@ -272,6 +270,12 @@ class TextFormField extends FormField<String> { ...@@ -272,6 +270,12 @@ class TextFormField extends FormField<String> {
/// initialize its [TextEditingController.text] with [initialValue]. /// initialize its [TextEditingController.text] with [initialValue].
final TextEditingController? controller; final TextEditingController? controller;
/// {@template flutter.material.TextFormField.onChanged}
/// Called when the user initiates a change to the TextField's
/// value: when they have inserted or deleted text or reset the form.
/// {@endtemplate}
final ValueChanged<String>? onChanged;
static Widget _defaultContextMenuBuilder(BuildContext context, EditableTextState editableTextState) { static Widget _defaultContextMenuBuilder(BuildContext context, EditableTextState editableTextState) {
return AdaptiveTextSelectionToolbar.editableText( return AdaptiveTextSelectionToolbar.editableText(
editableTextState: editableTextState, editableTextState: editableTextState,
...@@ -365,10 +369,11 @@ class _TextFormFieldState extends FormFieldState<String> { ...@@ -365,10 +369,11 @@ class _TextFormFieldState extends FormFieldState<String> {
@override @override
void reset() { void reset() {
// setState will be called in the superclass, so even though state is being // Set the controller value before calling super.reset() to let
// manipulated, no setState call is needed here. // _handleControllerChanged suppress the change.
_effectiveController.text = widget.initialValue ?? ''; _effectiveController.text = widget.initialValue ?? '';
super.reset(); super.reset();
_textFormField.onChanged?.call(_effectiveController.text);
} }
void _handleControllerChanged() { void _handleControllerChanged() {
......
...@@ -490,4 +490,43 @@ void main() { ...@@ -490,4 +490,43 @@ void main() {
final CupertinoTextField rtlTextFieldWidget = tester.widget(rtlTextFieldFinder); final CupertinoTextField rtlTextFieldWidget = tester.widget(rtlTextFieldFinder);
expect(rtlTextFieldWidget.textDirection, TextDirection.rtl); expect(rtlTextFieldWidget.textDirection, TextDirection.rtl);
}); });
testWidgets('CupertinoTextFormFieldRow onChanged is called when the form is reset', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/123009.
final GlobalKey<FormFieldState<String>> stateKey = GlobalKey<FormFieldState<String>>();
final GlobalKey<FormState> formKey = GlobalKey<FormState>();
String value = 'initialValue';
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: Form(
key: formKey,
child: CupertinoTextFormFieldRow(
key: stateKey,
initialValue: value,
onChanged: (String newValue) {
value = newValue;
},
),
),
),
),
);
// Initial value is 'initialValue'.
expect(stateKey.currentState!.value, 'initialValue');
expect(value, 'initialValue');
// Change value to 'changedValue'.
await tester.enterText(find.byType(CupertinoTextField), 'changedValue');
expect(stateKey.currentState!.value,'changedValue');
expect(value, 'changedValue');
// Should be back to 'initialValue' when the form is reset.
formKey.currentState!.reset();
await tester.pump();
expect(stateKey.currentState!.value,'initialValue');
expect(value, 'initialValue');
});
} }
...@@ -1231,4 +1231,52 @@ void main() { ...@@ -1231,4 +1231,52 @@ void main() {
inkWell = tester.widget<InkWell>(find.byType(InkWell)); inkWell = tester.widget<InkWell>(find.byType(InkWell));
expect(inkWell.borderRadius, errorBorderRadius); expect(inkWell.borderRadius, errorBorderRadius);
}); });
testWidgets('DropdownButtonFormField onChanged is called when the form is reset', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/123009.
final GlobalKey<FormFieldState<String>> stateKey = GlobalKey<FormFieldState<String>>();
final GlobalKey<FormState> formKey = GlobalKey<FormState>();
String? value;
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Form(
key: formKey,
child: DropdownButtonFormField<String>(
key: stateKey,
value: 'One',
items: <String>['One', 'Two', 'Free', 'Four']
.map<DropdownMenuItem<String>>((String value) {
return DropdownMenuItem<String>(
value: value,
child: Text(value),
);
}).toList(),
onChanged: (String? newValue) {
value = newValue;
},
),
),
),
),
);
// Initial value is 'One'.
expect(value, isNull);
expect(stateKey.currentState!.value, equals('One'));
// Select 'Two'.
await tester.tap(find.text('One'));
await tester.pumpAndSettle();
await tester.tap(find.text('Two').last);
await tester.pumpAndSettle();
expect(value, equals('Two'));
expect(stateKey.currentState!.value, equals('Two'));
// Should be back to 'One' when the form is reset.
formKey.currentState!.reset();
expect(value, equals('One'));
expect(stateKey.currentState!.value, equals('One'));
});
} }
...@@ -816,6 +816,31 @@ void main() { ...@@ -816,6 +816,31 @@ void main() {
expect(find.text('initialValue'), findsOneWidget); expect(find.text('initialValue'), findsOneWidget);
}); });
testWidgetsWithLeakTracking('reset resets the text fields value to the controller initial value', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController(text: 'initialValue');
addTearDown(controller.dispose);
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: TextFormField(
controller: controller,
),
),
),
),
);
await tester.enterText(find.byType(TextFormField), 'changedValue');
final FormFieldState<String> state = tester.state<FormFieldState<String>>(find.byType(TextFormField));
state.reset();
expect(find.text('changedValue'), findsNothing);
expect(find.text('initialValue'), findsOneWidget);
});
// Regression test for https://github.com/flutter/flutter/issues/34847. // Regression test for https://github.com/flutter/flutter/issues/34847.
testWidgetsWithLeakTracking("didChange resets the text field's value to empty when passed null", (WidgetTester tester) async { testWidgetsWithLeakTracking("didChange resets the text field's value to empty when passed null", (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
...@@ -1478,4 +1503,41 @@ void main() { ...@@ -1478,4 +1503,41 @@ void main() {
await tester.pump(); await tester.pump();
expect(textField.cursorColor, errorColor); expect(textField.cursorColor, errorColor);
}); });
testWidgetsWithLeakTracking('TextFormField onChanged is called when the form is reset', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/123009.
final GlobalKey<FormFieldState<String>> stateKey = GlobalKey<FormFieldState<String>>();
final GlobalKey<FormState> formKey = GlobalKey<FormState>();
String value = 'initialValue';
await tester.pumpWidget(MaterialApp(
home: Scaffold(
body: Form(
key: formKey,
child: TextFormField(
key: stateKey,
initialValue: value,
onChanged: (String newValue) {
value = newValue;
},
),
),
),
));
// Initial value is 'initialValue'.
expect(stateKey.currentState!.value, 'initialValue');
expect(value, 'initialValue');
// Change value to 'changedValue'.
await tester.enterText(find.byType(TextField), 'changedValue');
expect(stateKey.currentState!.value,'changedValue');
expect(value, 'changedValue');
// Should be back to 'initialValue' when the form is reset.
formKey.currentState!.reset();
await tester.pump();
expect(stateKey.currentState!.value,'initialValue');
expect(value, 'initialValue');
});
} }
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