From 7f27cb8b34cc1a2afb42b966b028c858e03da91c Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 16 Jan 2025 14:27:57 -0800 Subject: [PATCH 1/2] Property editor type fixes --- .../property_editor/property_editor_view.dart | 26 +++-- .../ide_shared/property_editor_test.dart | 97 +++++++++++++++++-- 2 files changed, 106 insertions(+), 17 deletions(-) diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart index 684bfbc599d..1486d760d38 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'package:devtools_app_shared/ui.dart'; +import 'package:devtools_app_shared/utils.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; @@ -137,7 +138,8 @@ class _PropertyInput extends StatefulWidget { } class _PropertyInputState extends State<_PropertyInput> { - String get typeError => 'Please enter a ${widget.argument.type}.'; + String get typeError => + 'Please enter ${addIndefiniteArticle(widget.argument.type)}.'; String currentValue = ''; @@ -150,12 +152,12 @@ class _PropertyInputState extends State<_PropertyInput> { label: Text(widget.argument.name), border: const OutlineInputBorder(), ); - - switch (widget.argument.type) { + final argType = widget.argument.type; + switch (argType) { case 'enum': case 'bool': final options = - widget.argument.type == 'bool' + argType == 'bool' ? ['true', 'false'] : (widget.argument.options ?? []); options.add(widget.argument.valueDisplay); @@ -183,11 +185,12 @@ class _PropertyInputState extends State<_PropertyInput> { case 'double': case 'int': case 'string': + final isNumeric = argType == 'double' || argType == 'int'; return TextFormField( initialValue: widget.argument.valueDisplay, enabled: widget.argument.isEditable, autovalidateMode: AutovalidateMode.onUserInteraction, - validator: _inputValidator, + validator: isNumeric ? _numericInputValidator : null, inputFormatters: [FilteringTextInputFormatter.singleLineFormatter], decoration: decoration, style: Theme.of(context).fixedFontStyle, @@ -211,8 +214,10 @@ class _PropertyInputState extends State<_PropertyInput> { final argName = widget.argument.name; // Can edit values to null. - if (widget.argument.isNullable && valueAsString == null || - (valueAsString == '' && widget.argument.type != 'string')) { + final valueIsNull = valueAsString == null || valueAsString == 'null'; + final valueIsEmpty = + widget.argument.type != 'string' && valueAsString == ''; + if (widget.argument.isNullable && (valueIsNull || valueIsEmpty)) { await widget.controller.editArgument(name: argName, value: null); return; } @@ -255,7 +260,12 @@ class _PropertyInputState extends State<_PropertyInput> { } } - String? _inputValidator(String? inputValue) { + String? _numericInputValidator(String? inputValue) { + // Permit sending null values with an empty input or with explicit "null". + final isNull = (inputValue ?? '').isEmpty || inputValue == 'null'; + if (widget.argument.isNullable && isNull) { + return null; + } final numValue = _toNumber(inputValue); if (numValue == null) { return typeError; diff --git a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart index 12d64cc5c6d..7a56c8c447f 100644 --- a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart +++ b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart @@ -225,13 +225,15 @@ void main() { textDocument: argThat(isNotNull, named: 'textDocument'), position: argThat(isNotNull, named: 'position'), name: argThat(isNotNull, named: 'name'), - value: argThat(isNotNull, named: 'value'), + value: argThat(anything, named: 'value'), ), ).thenAnswer((realInvocation) { final calledWithArgs = realInvocation.namedArguments; final name = calledWithArgs[const Symbol('name')]; final value = calledWithArgs[const Symbol('value')]; - nextEditCompleter.complete('$name: $value'); + nextEditCompleter.complete( + '$name: $value (TYPE: ${value?.runtimeType ?? 'null'})', + ); return Future.value(); }); }); @@ -248,7 +250,47 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('title: Brand New Title!')); + expect(nextEdit, equals('title: Brand New Title! (TYPE: String)')); + }); + }); + + testWidgets('editing a string input to null (title)', (tester) async { + return await tester.runAsync(() async { + // Load the property editor. + controller.initForTestsOnly(editableArgs: result1.args); + await tester.pumpWidget(wrap(propertyEditor)); + + // Edit the title. + final titleInput = _findTextFormField('title'); + await _inputText(titleInput, text: 'null', tester: tester); + + // Verify the edit is expected. + final nextEdit = await nextEditCompleter.future; + expect(nextEdit, equals('title: null (TYPE: null)')); + }); + }); + + testWidgets('editing a string input to empty string (title)', ( + tester, + ) async { + return await tester.runAsync(() async { + // Load the property editor. + controller.initForTestsOnly(editableArgs: result1.args); + await tester.pumpWidget(wrap(propertyEditor)); + + // Edit the title. + final titleInput = _findTextFormField('title'); + await _inputText(titleInput, text: '', tester: tester); + + // Verify the edit is expected. + final nextEdit = await nextEditCompleter.future; + expect( + nextEdit, + equals( + 'title: ' + ' (TYPE: String)', + ), + ); }); }); @@ -264,7 +306,23 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('height: 55.81')); + expect(nextEdit, equals('height: 55.81 (TYPE: double)')); + }); + }); + + testWidgets('editing a numeric input to null (height)', (tester) async { + return await tester.runAsync(() async { + // Load the property editor. + controller.initForTestsOnly(editableArgs: result1.args); + await tester.pumpWidget(wrap(propertyEditor)); + + // Edit the height. + final heightInput = _findTextFormField('height'); + await _inputText(heightInput, text: '', tester: tester); + + // Verify the edit is expected. + final nextEdit = await nextEditCompleter.future; + expect(nextEdit, equals('height: null (TYPE: null)')); }); }); @@ -285,7 +343,28 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('align: Alignment.topLeft')); + expect(nextEdit, equals('align: Alignment.topLeft (TYPE: String)')); + }); + }); + + testWidgets('editing a nullable enum input (align)', (tester) async { + return await tester.runAsync(() async { + // Load the property editor. + controller.initForTestsOnly(editableArgs: result2.args); + await tester.pumpWidget(wrap(propertyEditor)); + + // Select the align: null option. + final alignInput = _findDropdownButtonFormField('align'); + await _selectDropdownMenuItem( + alignInput, + optionToSelect: 'null', + currentlySelected: 'Alignment.center', + tester: tester, + ); + + // Verify the edit is expected. + final nextEdit = await nextEditCompleter.future; + expect(nextEdit, equals('align: null (TYPE: null)')); }); }); @@ -306,7 +385,7 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('softWrap: false')); + expect(nextEdit, equals('softWrap: false (TYPE: bool)')); }); }); }); @@ -433,7 +512,7 @@ final titleProperty = EditableArgument( type: 'string', isDefault: false, isEditable: true, - isNullable: false, + isNullable: true, isRequired: true, hasArgument: false, ); @@ -454,7 +533,7 @@ final heightProperty = EditableArgument( type: 'double', hasArgument: false, isEditable: true, - isNullable: false, + isNullable: true, value: 20.0, isDefault: true, isRequired: true, @@ -477,7 +556,7 @@ final softWrapProperty = EditableArgument( final alignProperty = EditableArgument( name: 'align', type: 'enum', - isNullable: false, + isNullable: true, hasArgument: true, isDefault: false, isRequired: false, From 814cd6174b351681e8edb5c917eaf7a38ae62615 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 16 Jan 2025 14:49:15 -0800 Subject: [PATCH 2/2] Committed in wrong directory --- packages/devtools_app_shared/lib/src/utils/utils.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/devtools_app_shared/lib/src/utils/utils.dart b/packages/devtools_app_shared/lib/src/utils/utils.dart index 3122a7ee152..809aa65b928 100644 --- a/packages/devtools_app_shared/lib/src/utils/utils.dart +++ b/packages/devtools_app_shared/lib/src/utils/utils.dart @@ -26,7 +26,7 @@ const tooltipWait = Duration(milliseconds: 500); const tooltipWaitLong = Duration(milliseconds: 1000); const tooltipWaitExtraLong = Duration(milliseconds: 1500); -/// Pluralizes a word, following english rules (1, many). +/// Pluralizes a word, following English rules (1, many). /// /// Pass a custom named `plural` for irregular plurals: /// `pluralize('index', count, plural: 'indices')` @@ -34,6 +34,10 @@ const tooltipWaitExtraLong = Duration(milliseconds: 1500); String pluralize(String word, int count, {String? plural}) => count == 1 ? word : (plural ?? '${word}s'); +/// Adds "a" or "an" to a word, following English rules. +String addIndefiniteArticle(String word) => + word.startsWith(RegExp(r'^[aeiouAEIOU]')) ? 'an $word' : 'a $word'; + bool isPrivateMember(String member) => member.startsWith('_'); /// Public properties first, then sort alphabetically