[Property Editor] Use defaultValue instead of isDefault to determine default value (#8884)

Author: elliette

Diff for: flutter-candidate.txt

@@ -1 +1 @@
-de872dd864518d436062354052ea8604c38e8c94
+e7cb7a30bb1c5c10ca86436ed10ce0a76b939d2f

Diff for: packages/devtools_app/lib/src/shared/editor/api_classes.dart

@@ -92,6 +92,7 @@ abstract class Field {
   static const debugSession = 'debugSession';
   static const debugSessionId = 'debugSessionId';
   static const debugSessions = 'debugSessions';
+  static const defaultValue = 'defaultValue';
   static const device = 'device';
   static const deviceId = 'deviceId';
   static const devices = 'devices';
@@ -108,7 +109,6 @@ abstract class Field {
   static const hasArgument = 'hasArgument';
   static const id = 'id';
   static const isDarkMode = 'isDarkMode';
-  static const isDefault = 'isDefault';
   static const isEditable = 'isEditable';
   static const isNullable = 'isNullable';
   static const isRequired = 'isRequired';
@@ -515,13 +515,14 @@ class EditableArgument with Serializable {
   EditableArgument({
     required this.name,
     required this.type,
-    required this.value,
     required this.hasArgument,
-    required this.isDefault,
     required this.isNullable,
     required this.isRequired,
     required this.isEditable,
+    required this.hasDefault,
     this.options,
+    this.value,
+    this.defaultValue,
     this.displayValue,
     this.errorText,
   });
@@ -530,13 +531,14 @@ class EditableArgument with Serializable {
     : this(
         name: map[Field.name] as String,
         type: map[Field.type] as String,
-        value: map[Field.value],
         hasArgument: (map[Field.hasArgument] as bool?) ?? false,
-        isDefault: (map[Field.isDefault] as bool?) ?? false,
         isNullable: (map[Field.isNullable] as bool?) ?? false,
         isRequired: (map[Field.isRequired] as bool?) ?? false,
         isEditable: (map[Field.isEditable] as bool?) ?? true,
+        hasDefault: map.containsKey(Field.defaultValue),
         options: (map[Field.options] as List<Object?>?)?.cast<String>(),
+        value: map[Field.value],
+        defaultValue: map[Field.defaultValue],
         displayValue: map[Field.displayValue] as String?,
         errorText: map[Field.errorText] as String?,
       );
@@ -556,9 +558,6 @@ class EditableArgument with Serializable {
   /// Whether an explicit argument exists for this parameter in the code.
   final bool hasArgument;
 
-  /// Whether the value is the default for this parameter.
-  final bool isDefault;
-
   /// Whether this argument can be `null`.
   final bool isNullable;
 
@@ -576,6 +575,15 @@ class EditableArgument with Serializable {
   /// This will only be included if the parameter's [type] is "enum".
   final List<String>? options;
 
+  /// Whether the argument has an explicit default value.
+  ///
+  /// This is used to distinguish whether the [defaultValue] is actually `null`
+  /// or is not provided.
+  final bool hasDefault;
+
+  /// The default value for this parameter.
+  final Object? defaultValue;
+
   /// A string that can be displayed to indicate the value for this argument.
   ///
   /// This is populated in cases where the source code is not literally the same
@@ -584,15 +592,19 @@ class EditableArgument with Serializable {
 
   final String? errorText;
 
-  String get valueDisplay => displayValue ?? value.toString();
+  String get valueDisplay => displayValue ?? currentValue.toString();
+
+  bool get isDefault => hasDefault && currentValue == defaultValue;
+
+  Object? get currentValue => hasArgument ? value : defaultValue;
 
   @override
   Map<String, Object?> toJson() => {
     Field.name: name,
     Field.type: type,
     Field.value: value,
     Field.hasArgument: hasArgument,
-    Field.isDefault: isDefault,
+    Field.defaultValue: defaultValue,
     Field.isNullable: isNullable,
     Field.isRequired: isRequired,
     Field.isEditable: isEditable,

Diff for: packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart

@@ -64,7 +64,8 @@ class EditableEnum extends EditableProperty with FiniteValuesProperty {
   String get dartType => options?.first.split('.').first ?? type;
 
   @override
-  String get valueDisplay => _enumShorthand(displayValue ?? value.toString());
+  String get valueDisplay =>
+      _enumShorthand(displayValue ?? currentValue.toString());
 
   @override
   Set<String> get propertyOptions {
@@ -93,8 +94,9 @@ class EditableProperty extends EditableArgument {
         name: argument.name,
         type: argument.type,
         value: argument.value,
+        hasDefault: argument.hasDefault,
         hasArgument: argument.hasArgument,
-        isDefault: argument.isDefault,
+        defaultValue: argument.defaultValue,
         isNullable: argument.isNullable,
         isRequired: argument.isRequired,
         isEditable: argument.isEditable,

Diff for: packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart

@@ -154,16 +154,19 @@ void main() {
       expect(widthInput, findsOneWidget);
       expect(heightInput, findsOneWidget);
 
-      // Verify the labels are expected.
-      expect(_labelForInput(titleInput, matching: 'set'), findsNothing);
-      expect(_labelForInput(titleInput, matching: 'default'), findsNothing);
-      expect(_labelForInput(widthInput, matching: 'set'), findsNothing);
-      expect(_labelForInput(widthInput, matching: 'default'), findsNothing);
-      expect(_labelForInput(heightInput, matching: 'set'), findsNothing);
-      expect(_labelForInput(heightInput, matching: 'default'), findsOneWidget);
-
-      // Verify required comments exist.
-      expect(_requiredTextForInput(titleInput), findsOneWidget);
+      // Verify the labels and required are expected.
+      _labelsAndRequiredTextAreExpected(
+        titleInput,
+        inputExpectations: titleInputExpectations,
+      );
+      _labelsAndRequiredTextAreExpected(
+        widthInput,
+        inputExpectations: widthInputExpectations,
+      );
+      _labelsAndRequiredTextAreExpected(
+        heightInput,
+        inputExpectations: heightInputExpectations,
+      );
     });
 
     testWidgets('inputs are expected for second group of editable arguments', (
@@ -184,14 +187,15 @@ void main() {
       expect(softWrapInput, findsOneWidget);
       expect(alignInput, findsOneWidget);
 
-      // Verify the labels are expected.
-      expect(_labelForInput(softWrapInput, matching: 'set'), findsNothing);
-      expect(
-        _labelForInput(softWrapInput, matching: 'default'),
-        findsOneWidget,
+      // Verify the labels and required are expected.
+      _labelsAndRequiredTextAreExpected(
+        softWrapInput,
+        inputExpectations: softWrapInputExpectations,
+      );
+      _labelsAndRequiredTextAreExpected(
+        alignInput,
+        inputExpectations: alignInputExpectations,
       );
-      expect(_labelForInput(alignInput, matching: 'set'), findsOneWidget);
-      expect(_labelForInput(alignInput, matching: 'default'), findsNothing);
     });
 
     testWidgets('softWrap input has expected options', (tester) async {
@@ -537,6 +541,34 @@ Finder _labelForInput(Finder inputFinder, {required String matching}) {
 Finder _requiredTextForInput(Finder inputFinder) =>
     _helperTextForInput(inputFinder, matching: '*required');
 
+void _labelsAndRequiredTextAreExpected(
+  Finder inputFinder, {
+  required Map<String, bool> inputExpectations,
+}) {
+  // Check for the existence/non-existence of the "set" badge.
+  final shouldBeSet = inputExpectations['isSet'] == true;
+  expect(
+    _labelForInput(inputFinder, matching: 'set'),
+    shouldBeSet ? findsOneWidget : findsNothing,
+    reason: 'Expected to find ${shouldBeSet ? 'a' : 'no'} "set" badge.',
+  );
+  // Check for the existence/non-existence of the "default" badge.
+  final shouldBeDefault = inputExpectations['isDefault'] == true;
+  expect(
+    _labelForInput(inputFinder, matching: 'default'),
+    shouldBeDefault ? findsOneWidget : findsNothing,
+    reason: 'Expected to find ${shouldBeDefault ? 'a' : 'no'} "default" badge.',
+  );
+  // Check for the existence/non-existence of the required text ('*').
+  final shouldBeRequired = inputExpectations['isRequired'] == true;
+  expect(
+    _requiredTextForInput(inputFinder),
+    shouldBeRequired ? findsOneWidget : findsNothing,
+    reason:
+        'Expected to find ${shouldBeRequired ? 'the' : 'no'} "required" indicator.',
+  );
+}
+
 Finder _helperTextForInput(Finder inputFinder, {required String matching}) {
   final rowFinder = find.ancestor(of: inputFinder, matching: find.byType(Row));
   return find.descendant(of: rowFinder, matching: find.richText(matching));
@@ -653,63 +685,79 @@ final activeLocationChangedEvent2 = ActiveLocationChangedEvent(
 );
 
 // Result 1
-final titleProperty = EditableArgument(
-  name: 'title',
-  value: 'Hello world!',
-  type: 'string',
-  isDefault: false,
-  isEditable: true,
-  isNullable: true,
-  isRequired: true,
-  hasArgument: false,
-);
-final widthProperty = EditableArgument(
-  name: 'width',
-  displayValue: '100.0',
-  type: 'double',
-  isEditable: false,
-  isDefault: false,
-  errorText: 'Some reason for why this can\'t be edited.',
-  isNullable: false,
-  value: 20.0,
-  isRequired: false,
-  hasArgument: false,
-);
-final heightProperty = EditableArgument(
-  name: 'height',
-  type: 'double',
-  hasArgument: false,
-  isEditable: true,
-  isNullable: true,
-  value: 20.0,
-  isDefault: true,
-  isRequired: false,
-);
+final titleProperty = EditableArgument.fromJson({
+  'name': 'title',
+  'value': 'Hello world!',
+  'type': 'string',
+  'isEditable': true,
+  'isNullable': true,
+  'isRequired': true,
+  'hasArgument': true,
+});
+final titleInputExpectations = {
+  'isSet': true,
+  'isRequired': true,
+  'isDefault': false,
+};
+
+final widthProperty = EditableArgument.fromJson({
+  'name': 'width',
+  'displayValue': 'myWidth',
+  'type': 'double',
+  'errorText': 'Some reason why this can\'t be edited.',
+  'isNullable': false,
+  'isRequired': false,
+  'hasArgument': true,
+});
+final widthInputExpectations = {
+  'isSet': true,
+  'isRequired': false,
+  'isDefault': false,
+};
+
+final heightProperty = EditableArgument.fromJson({
+  'name': 'height',
+  'type': 'double',
+  'hasArgument': false,
+  'isEditable': true,
+  'isNullable': true,
+  'defaultValue': 20.0,
+  'isRequired': false,
+});
+final heightInputExpectations = {
+  'isSet': false,
+  'isRequired': false,
+  'isDefault': true,
+};
 final result1 = EditableArgumentsResult(
   args: [titleProperty, widthProperty, heightProperty],
 );
 
 // Result 2
-final softWrapProperty = EditableArgument(
-  name: 'softWrap',
-  type: 'bool',
-  isNullable: false,
-  value: true,
-  isDefault: true,
-  hasArgument: false,
-  isEditable: true,
-  isRequired: false,
-);
-final alignProperty = EditableArgument(
-  name: 'align',
-  type: 'enum',
-  isNullable: true,
-  hasArgument: true,
-  isDefault: false,
-  isRequired: false,
-  isEditable: true,
-  value: 'Alignment.center',
-  options: [
+final softWrapProperty = EditableArgument.fromJson({
+  'name': 'softWrap',
+  'type': 'bool',
+  'isNullable': false,
+  'defaultValue': true,
+  'hasArgument': false,
+  'isEditable': true,
+  'isRequired': false,
+});
+final softWrapInputExpectations = {
+  'isSet': false,
+  'isRequired': false,
+  'isDefault': true,
+};
+final alignProperty = EditableArgument.fromJson({
+  'name': 'align',
+  'type': 'enum',
+  'isNullable': true,
+  'hasArgument': true,
+  'defaultValue': 'Alignment.bottomLeft',
+  'isRequired': false,
+  'isEditable': true,
+  'value': 'Alignment.center',
+  'options': [
     'Alignment.bottomCenter',
     'Alignment.bottomLeft',
     'Alignment.bottomRight',
@@ -720,7 +768,12 @@ final alignProperty = EditableArgument(
     'Alignment.topLeft',
     'Alignment.topRight',
   ],
-);
+});
+final alignInputExpectations = {
+  'isSet': true,
+  'isRequired': false,
+  'isDefault': false,
+};
 final result2 = EditableArgumentsResult(
   args: [softWrapProperty, alignProperty],
 );