Property editor validation and nullable argument fixes

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 ?? <String>[]);
         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;

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,

packages/devtools_app_shared/lib/src/utils/utils.dart

@@ -26,14 +26,18 @@ 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')`
 /// So it returns `indices` and not `indexs`.
 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