From a96c0eb29a42676912fb28ac9b46901702947d11 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 24 Feb 2025 15:14:15 -0800 Subject: [PATCH 1/5] Fix test input state issues --- .../property_editor_inputs.dart | 25 +++++++++++++------ .../property_editor_types.dart | 12 +++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart index 0483b0ef3a5..b7635d93c78 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart @@ -204,14 +204,17 @@ class _TextInput extends StatefulWidget { class _TextInputState extends State<_TextInput> with _PropertyInputMixin<_TextInput, T>, AutoDisposeMixin { - String currentValue = ''; + static const _paddingDiffComparedToDropdown = 1.0; - double paddingDiffComparedToDropdown = 1.0; - late FocusNode _focusNode; + String _currentValue = ''; + + late final TextEditingController _controller; + late final FocusNode _focusNode; @override void initState() { super.initState(); + _controller = TextEditingController(text: widget.property.valueDisplay); _focusNode = FocusNode(debugLabel: 'text-input-${widget.property.name}'); addAutoDisposeListener(_focusNode, () async { @@ -221,12 +224,20 @@ class _TextInputState extends State<_TextInput> }); } + @override + void didUpdateWidget(_TextInput oldWidget) { + super.didUpdateWidget(oldWidget); + if (oldWidget.property != widget.property) { + _controller.text = widget.property.valueDisplay; + } + } + @override Widget build(BuildContext context) { final theme = Theme.of(context); return TextFormField( focusNode: _focusNode, - initialValue: widget.property.valueDisplay, + controller: _controller, enabled: widget.property.isEditable, autovalidateMode: AutovalidateMode.onUserInteraction, validator: (text) => inputValidator(text, property: widget.property), @@ -237,13 +248,13 @@ class _TextInputState extends State<_TextInput> // Note: The text input has an extra pixel compared to the dropdown // input. Therefore, to have their sizes match, subtract a half pixel // from the padding. - padding: defaultSpacing - (paddingDiffComparedToDropdown / 2), + padding: defaultSpacing - (_paddingDiffComparedToDropdown / 2), ), style: theme.fixedFontStyle, onChanged: (newValue) { clearServerError(); setState(() { - currentValue = newValue; + _currentValue = newValue; }); }, onEditingComplete: _editProperty, @@ -253,7 +264,7 @@ class _TextInputState extends State<_TextInput> Future _editProperty() async { await editProperty( widget.property, - valueAsString: currentValue, + valueAsString: _currentValue, editPropertyCallback: widget.editProperty, ); } diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart index f212ec63ccc..bc1d7001a84 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart @@ -167,6 +167,18 @@ class EditableProperty extends EditableArgument { Object? convertFromInputString(String? _) { throw UnimplementedError(); } + + @override + bool operator ==(Object other) { + return other is EditableProperty && + other.name == name && + other.type == type && + other.value == value && + other.hasArgument == hasArgument; + } + + @override + int get hashCode => Object.hash(name, type, value, hasArgument); } mixin NumericProperty on EditableProperty { From fe4972daec2ecdd95e6f024fb190bb694a0ba0fc Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 24 Feb 2025 16:31:50 -0800 Subject: [PATCH 2/5] Add a test case --- .../property_editor/property_editor_test.dart | 95 ++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-) diff --git a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart index d9c839bf359..6072c612902 100644 --- a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart +++ b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart @@ -27,6 +27,8 @@ void main() { final LocationToArgsResult locationToArgsResult = { (document: textDocument1, position: activeCursorPosition1): result1, (document: textDocument2, position: activeCursorPosition2): result2, + (document: textDocument1, position: activeCursorPosition3): resultWithText, + (document: textDocument1, position: activeCursorPosition4): resultWithTitle, }; late MockEditorClient mockEditorClient; @@ -58,7 +60,9 @@ void main() { Future> waitForEditableArgs() { final argsCompleter = Completer>(); listener = () { - argsCompleter.complete(controller.editableWidgetData.value!.args); + if (!argsCompleter.isCompleted) { + argsCompleter.complete(controller.editableWidgetData.value!.args); + } }; controller.editableWidgetData.addListener(listener!); return argsCompleter.future; @@ -131,6 +135,43 @@ void main() { verifyEditableArgs(actual: editableArgs, expected: result2.args); }); }); + + testWidgets('verify editable arguments update when widget changes', ( + tester, + ) async { + await tester.runAsync(() async { + // Load the property editor. + await tester.pumpWidget(wrap(propertyEditor)); + + // Send an active location changed event. + final editableArgsFuture1 = waitForEditableArgs(); + eventController.add(activeLocationChangedEvent3); + + // Wait for the expected editable args. + await editableArgsFuture1; + await tester.pumpAndSettle(); + + // Verify the inputs. + final textInput = _findTextFormField('text'); + expect(textInput, findsOneWidget); + final textValue = _textFormFieldValue(textInput, tester: tester); + expect(textValue, equals('This is some text.')); + + // Send an active location changed event. + final editableArgsFuture2 = waitForEditableArgs(); + eventController.add(activeLocationChangedEvent4); + + // Wait for the expected editable args. + await editableArgsFuture2; + await tester.pumpAndSettle(); + + // Verify the inputs. + final titleInput = _findTextFormField('title*'); + expect(titleInput, findsOneWidget); + final titleValue = _textFormFieldValue(titleInput, tester: tester); + expect(titleValue, equals('Hello world!')); + }); + }); }); group('inputs for editable arguments', () { @@ -629,6 +670,15 @@ Finder _findTextFormField(String inputName) => find.ancestor( matching: find.byType(TextFormField), ); +String? _textFormFieldValue( + Finder textFormFieldFinder, { + required WidgetTester tester, +}) { + final textFormFieldWidget = tester.widget(textFormFieldFinder); + final textEditingController = textFormFieldWidget.controller; + return textEditingController?.text; +} + Finder _labelForInput(Finder inputFinder, {required String matching}) { final rowFinder = find.ancestor(of: inputFinder, matching: find.byType(Row)); final labelFinder = find.descendant( @@ -802,6 +852,30 @@ final activeLocationChangedEvent2 = ActiveLocationChangedEvent( textDocument: textDocument2, ); +// Location position 3 +final activeCursorPosition3 = CursorPosition(character: 55, line: 2); +final anchorCursorPosition3 = CursorPosition(character: 60, line: 4); +final editorSelection3 = EditorSelection( + active: activeCursorPosition3, + anchor: anchorCursorPosition3, +); +final activeLocationChangedEvent3 = ActiveLocationChangedEvent( + selections: [editorSelection3], + textDocument: textDocument1, +); + +// Location position 4 +final activeCursorPosition4 = CursorPosition(character: 10, line: 11); +final anchorCursorPosition4 = CursorPosition(character: 12, line: 2); +final editorSelection4 = EditorSelection( + active: activeCursorPosition4, + anchor: anchorCursorPosition4, +); +final activeLocationChangedEvent4 = ActiveLocationChangedEvent( + selections: [editorSelection4], + textDocument: textDocument1, +); + // Widget name and documentation const widgetName = 'MyFlutterWidget'; @@ -918,3 +992,22 @@ final resultWithWidgetNameAndDocsNoArgs = EditableArgumentsResult( documentation: dartDocText, args: [], ); + +// Example results for text input state change test cases. +final textProperty = EditableArgument.fromJson({ + 'name': 'text', + 'value': 'This is some text.', + 'type': 'string', + 'isEditable': true, + 'isNullable': true, + 'isRequired': false, + 'hasArgument': true, +}); +final resultWithText = EditableArgumentsResult( + name: 'WidgetWithText', + args: [textProperty], +); +final resultWithTitle = EditableArgumentsResult( + name: 'WidgetWithTitle', + args: [titleProperty], +); From 55f7df9a4cddb57dfe140f0b8b57a123d77658df Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 25 Feb 2025 09:20:55 -0800 Subject: [PATCH 3/5] Remove controller and use key instead --- .../property_editor/property_editor_inputs.dart | 16 ++++------------ .../property_editor/property_editor_test.dart | 3 +-- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart index b7635d93c78..0128c316f33 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart @@ -120,7 +120,8 @@ class _DropdownInputState extends State<_DropdownInput> @override Widget build(BuildContext context) { final theme = Theme.of(context); - return DropdownButtonFormField( + return DropdownButtonFormField(g + key: Key(widget.property.hashCode.toString()), value: widget.property.valueDisplay, autovalidateMode: AutovalidateMode.onUserInteraction, validator: (text) => inputValidator(text, property: widget.property), @@ -208,13 +209,11 @@ class _TextInputState extends State<_TextInput> String _currentValue = ''; - late final TextEditingController _controller; late final FocusNode _focusNode; @override void initState() { super.initState(); - _controller = TextEditingController(text: widget.property.valueDisplay); _focusNode = FocusNode(debugLabel: 'text-input-${widget.property.name}'); addAutoDisposeListener(_focusNode, () async { @@ -224,20 +223,13 @@ class _TextInputState extends State<_TextInput> }); } - @override - void didUpdateWidget(_TextInput oldWidget) { - super.didUpdateWidget(oldWidget); - if (oldWidget.property != widget.property) { - _controller.text = widget.property.valueDisplay; - } - } - @override Widget build(BuildContext context) { final theme = Theme.of(context); return TextFormField( + key: Key(widget.property.hashCode.toString()), focusNode: _focusNode, - controller: _controller, + initialValue: widget.property.valueDisplay, enabled: widget.property.isEditable, autovalidateMode: AutovalidateMode.onUserInteraction, validator: (text) => inputValidator(text, property: widget.property), diff --git a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart index 6072c612902..7b8fd8e6c90 100644 --- a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart +++ b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart @@ -675,8 +675,7 @@ String? _textFormFieldValue( required WidgetTester tester, }) { final textFormFieldWidget = tester.widget(textFormFieldFinder); - final textEditingController = textFormFieldWidget.controller; - return textEditingController?.text; + return textFormFieldWidget.initialValue; } Finder _labelForInput(Finder inputFinder, {required String matching}) { From 461946acfa7e953660d6d29d65bf1b6fe3596967 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 25 Feb 2025 09:21:47 -0800 Subject: [PATCH 4/5] Accidental typo --- .../ide_shared/property_editor/property_editor_inputs.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart index 0128c316f33..ad0836272cc 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart @@ -120,7 +120,7 @@ class _DropdownInputState extends State<_DropdownInput> @override Widget build(BuildContext context) { final theme = Theme.of(context); - return DropdownButtonFormField(g + return DropdownButtonFormField( key: Key(widget.property.hashCode.toString()), value: widget.property.valueDisplay, autovalidateMode: AutovalidateMode.onUserInteraction, From 94d905c6f828a25b21fa106c7024d99aed9b41ff Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 25 Feb 2025 10:29:13 -0800 Subject: [PATCH 5/5] Use instance equality instead --- .../property_editor/property_editor_types.dart | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart index bc1d7001a84..f212ec63ccc 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart @@ -167,18 +167,6 @@ class EditableProperty extends EditableArgument { Object? convertFromInputString(String? _) { throw UnimplementedError(); } - - @override - bool operator ==(Object other) { - return other is EditableProperty && - other.name == name && - other.type == type && - other.value == value && - other.hasArgument == hasArgument; - } - - @override - int get hashCode => Object.hash(name, type, value, hasArgument); } mixin NumericProperty on EditableProperty {