[Inspector V2] Sort and format properties in the properties table (fl…

…utter#8250)

packages/devtools_app/lib/src/screens/inspector_v2/widget_properties/properties_view.dart

@@ -6,6 +6,7 @@ import 'package:devtools_app_shared/ui.dart';
 import 'package:flutter/material.dart';
 
 import '../../../shared/analytics/constants.dart' as gac;
+import '../../../shared/console/widgets/description.dart';
 import '../../../shared/diagnostics/diagnostics_node.dart';
 import '../../../shared/primitives/utils.dart';
 import '../../../shared/ui/tab.dart';
@@ -109,7 +110,7 @@ class _DetailsTableState extends State<DetailsTable> {
 
 /// Displays a widget's properties, including the layout properties and a
 /// layout visualizer.
-class PropertiesView extends StatelessWidget {
+class PropertiesView extends StatefulWidget {
   const PropertiesView({
     super.key,
     required this.properties,
@@ -127,22 +128,52 @@ class PropertiesView extends StatelessWidget {
   final InspectorController controller;
   final ScrollController scrollController;
 
+  @override
+  State<PropertiesView> createState() => _PropertiesViewState();
+}
+
+class _PropertiesViewState extends State<PropertiesView> {
   RemoteDiagnosticsNode? get selectedNode =>
-      controller.selectedNode.value?.diagnostic;
+      widget.controller.selectedNode.value?.diagnostic;
+
+  bool get includeLayoutExplorer =>
+      (selectedNode?.isBoxLayout ?? false) && widget.layoutProperties != null;
+
+  WidgetSizes? get widgetWidths => widget.layoutProperties?.widgetWidths;
+
+  WidgetSizes? get widgetHeights => widget.layoutProperties?.widgetHeights;
 
-  bool get includeLayoutExplorer => selectedNode?.isBoxLayout ?? false;
+  List<RemoteDiagnosticsNode> _sortedProperties = <RemoteDiagnosticsNode>[];
 
-  WidgetSizes? get widgetWidths => layoutProperties?.widgetWidths;
+  @override
+  void initState() {
+    super.initState();
 
-  WidgetSizes? get widgetHeights => layoutProperties?.widgetHeights;
+    _sortedProperties = _filterAndSortPropertiesByLevel(widget.properties);
+  }
+
+  @override
+  void didUpdateWidget(PropertiesView oldWidget) {
+    super.didUpdateWidget(oldWidget);
+
+    if (widget.properties != oldWidget.properties) {
+      _sortedProperties = _filterAndSortPropertiesByLevel(widget.properties);
+    }
+  }
 
   @override
   Widget build(BuildContext context) {
     final layoutExplorerOffset = includeLayoutExplorer ? 1 : 0;
-
-    Widget? propertiesList;
+    // If there are no properties to display, include a single row that says as
+    // much.
+    final propertyRowsCount =
+        _sortedProperties.isEmpty ? 1 : _sortedProperties.length;
+    // If the layout explorer is available, it is the first row.
+    final totalRowsCount = propertyRowsCount + layoutExplorerOffset;
+
+    Widget? layoutPropertiesList;
     if (widgetWidths != null && widgetHeights != null) {
-      propertiesList = LayoutPropertiesList(
+      layoutPropertiesList = LayoutPropertiesList(
         widgetHeights: widgetHeights,
         widgetWidths: widgetWidths,
       );
@@ -155,11 +186,11 @@ class PropertiesView extends StatelessWidget {
                 PropertiesView.scaleFactorForVerticalLayout);
 
         return Scrollbar(
-          controller: scrollController,
+          controller: widget.scrollController,
           thumbVisibility: true,
           child: ListView.builder(
-            controller: scrollController,
-            itemCount: properties.length + layoutExplorerOffset,
+            controller: widget.scrollController,
+            itemCount: totalRowsCount,
             itemBuilder: (context, index) {
               if (index == 0 && includeLayoutExplorer) {
                 return DecoratedPropertiesTableRow(
@@ -174,34 +205,68 @@ class PropertiesView extends StatelessWidget {
                           height: PropertiesView.layoutExplorerHeight,
                           width: PropertiesView.layoutExplorerWidth,
                           child: BoxLayoutExplorerWidget(
-                            controller,
+                            widget.controller,
                             selectedNode: selectedNode,
-                            layoutProperties: layoutProperties,
+                            layoutProperties: widget.layoutProperties,
                           ),
                         ),
                       ),
-                      if (propertiesList != null)
+                      if (layoutPropertiesList != null)
                         Padding(
                           padding: horizontalLayout
                               ? const EdgeInsets.only(left: largeSpacing)
                               : const EdgeInsets.only(bottom: largeSpacing),
-                          child: propertiesList,
+                          child: layoutPropertiesList,
                         ),
                     ],
                   ),
                 );
               }
 
+              if (_sortedProperties.isEmpty && index == layoutExplorerOffset) {
+                return DecoratedPropertiesTableRow(
+                  index: index - layoutExplorerOffset,
+                  child: PaddedText(
+                    child: Text(
+                      'No widget properties to display.',
+                      style: Theme.of(context).regularTextStyle,
+                    ),
+                  ),
+                );
+              }
+
               return PropertyItem(
                 index: index - layoutExplorerOffset,
-                properties: properties,
+                properties: _sortedProperties,
               );
             },
           ),
         );
       },
     );
   }
+
+  /// Filters out properties with [DiagnosticLevel.hidden] and sorts properties
+  /// with [DiagnosticLevel.fine] behind all others.
+  List<RemoteDiagnosticsNode> _filterAndSortPropertiesByLevel(
+    List<RemoteDiagnosticsNode> properties,
+  ) {
+    final propertiesWithFineLevel = <RemoteDiagnosticsNode>[];
+    final propertiesWithOtherLevels = <RemoteDiagnosticsNode>[];
+
+    for (final property in properties) {
+      // Don't include properties that should be hidden:
+      if (property.level == DiagnosticLevel.hidden) continue;
+
+      if (property.level == DiagnosticLevel.fine) {
+        propertiesWithFineLevel.add(property);
+      } else {
+        propertiesWithOtherLevels.add(property);
+      }
+    }
+
+    return [...propertiesWithOtherLevels, ...propertiesWithFineLevel];
+  }
 }
 
 /// List of the widget's layout properties.
@@ -291,8 +356,7 @@ class LayoutPropertyItem extends StatelessWidget {
   Widget build(BuildContext context) {
     final theme = Theme.of(context);
 
-    return Padding(
-      padding: const EdgeInsets.all(densePadding),
+    return PaddedText(
       child: RichText(
         text: TextSpan(
           text: '$name: ',
@@ -402,8 +466,7 @@ class PropertyName extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    return Padding(
-      padding: const EdgeInsets.all(denseRowSpacing),
+    return PaddedText(
       child: Text(
         property.name ?? '',
         style: Theme.of(context).subtleTextStyle,
@@ -423,12 +486,31 @@ class PropertyValue extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    return Padding(
-      padding: const EdgeInsets.all(denseRowSpacing),
-      child: Text(
-        property.description ?? 'null',
+    return PaddedText(
+      child: DiagnosticsNodeDescription(
+        property,
+        includeName: false,
+        overflow: TextOverflow.visible,
         style: Theme.of(context).fixedFontStyle,
       ),
     );
   }
 }
+
+/// Wraps a text widget with the correct amount of padding for the table.
+class PaddedText extends StatelessWidget {
+  const PaddedText({
+    super.key,
+    required this.child,
+  });
+
+  final Widget child;
+
+  @override
+  Widget build(BuildContext context) {
+    return Padding(
+      padding: const EdgeInsets.all(denseRowSpacing),
+      child: child,
+    );
+  }
+}

packages/devtools_app/lib/src/shared/console/widgets/description.dart

@@ -19,9 +19,6 @@ import '../eval/inspector_tree.dart';
 import 'expandable_variable.dart';
 
 final _colorIconMaker = ColorIconMaker();
-final _customIconMaker = CustomIconMaker();
-final defaultIcon = _customIconMaker.fromInfo('Default');
-
 const _showRenderObjectPropertiesAsLinks = false;
 
 /// Presents the content of a single [RemoteDiagnosticsNode].
@@ -47,6 +44,8 @@ class DiagnosticsNodeDescription extends StatelessWidget {
     this.actionCallback,
     this.customDescription,
     this.customIconName,
+    this.includeName = true,
+    this.overflow,
   });
 
   final RemoteDiagnosticsNode? diagnostic;
@@ -64,6 +63,8 @@ class DiagnosticsNodeDescription extends StatelessWidget {
   final VoidCallback? actionCallback;
   final String? customDescription;
   final String? customIconName;
+  final bool includeName;
+  final TextOverflow? overflow;
 
   static Widget _paddedIcon(Widget icon) {
     return Padding(
@@ -254,6 +255,7 @@ class DiagnosticsNodeDescription extends StatelessWidget {
         multiline: multiline,
         actionLabel: actionLabel,
         actionCallback: actionCallback,
+        overflow: overflow ?? TextOverflow.ellipsis,
       ),
     );
   }
@@ -293,7 +295,22 @@ class DiagnosticsNodeDescription extends StatelessWidget {
       final propertyType = diagnosticLocal.propertyType;
       final properties = diagnosticLocal.valuePropertiesJson;
 
-      if (name?.isNotEmpty == true && diagnosticLocal.showName) {
+      final showDefaultValueLabel =
+          diagnosticLocal.level == DiagnosticLevel.fine &&
+              diagnosticLocal.hasDefaultValue;
+
+      // Show the "default" value label at the start if the property name isn't
+      // included:
+      if (showDefaultValueLabel && !includeName) {
+        children.add(
+          const Padding(
+            padding: EdgeInsets.only(right: denseSpacing),
+            child: DefaultValueLabel(),
+          ),
+        );
+      }
+
+      if (includeName && name?.isNotEmpty == true && diagnosticLocal.showName) {
         children.add(
           Text(
             '$name${diagnosticLocal.separator} ',
@@ -362,10 +379,15 @@ class DiagnosticsNodeDescription extends StatelessWidget {
         ),
       );
 
-      if (diagnosticLocal.level == DiagnosticLevel.fine &&
-          diagnosticLocal.hasDefaultValue) {
-        children.add(const Text(' '));
-        children.add(_paddedIcon(defaultIcon));
+      // Show the "default" value label at the end if the property name is
+      // included:
+      if (showDefaultValueLabel && includeName) {
+        children.add(
+          const Padding(
+            padding: EdgeInsets.only(left: denseSpacing),
+            child: DefaultValueLabel(),
+          ),
+        );
       }
     } else {
       // Non property, regular node case.
@@ -553,13 +575,39 @@ class DiagnosticsNodeDescription extends StatelessWidget {
   }
 }
 
+/// Label for a property with the default value.
+class DefaultValueLabel extends StatelessWidget {
+  const DefaultValueLabel({super.key});
+
+  @override
+  Widget build(BuildContext context) {
+    final theme = Theme.of(context);
+    final colorScheme = theme.colorScheme;
+    return Container(
+      padding: const EdgeInsets.symmetric(horizontal: denseSpacing),
+      decoration: BoxDecoration(
+        borderRadius: defaultBorderRadius,
+        color: colorScheme.secondary,
+      ),
+      child: Text(
+        'default',
+        style: theme.regularTextStyleWithColor(
+          colorScheme.onSecondary,
+          backgroundColor: colorScheme.secondary,
+        ),
+      ),
+    );
+  }
+}
+
 class DescriptionDisplay extends StatelessWidget {
   const DescriptionDisplay({
     super.key,
     required this.text,
     this.multiline = false,
     this.actionLabel,
     this.actionCallback,
+    this.overflow = TextOverflow.ellipsis,
   })  : assert(
           multiline ? actionLabel == null : true,
           'Action labels are not supported for multiline descriptions',
@@ -573,6 +621,7 @@ class DescriptionDisplay extends StatelessWidget {
   final bool multiline;
   final String? actionLabel;
   final VoidCallback? actionCallback;
+  final TextOverflow overflow;
 
   @override
   Widget build(BuildContext context) {
@@ -605,7 +654,7 @@ class DescriptionDisplay extends StatelessWidget {
     }
 
     return RichText(
-      overflow: TextOverflow.ellipsis,
+      overflow: overflow,
       text: text,
     );
   }

packages/devtools_app/lib/src/shared/ui/icons.dart

@@ -283,7 +283,10 @@ class FlutterMaterialIcons {
   FlutterMaterialIcons._();
 
   static Icon getIconForCodePoint(int charCode, ColorScheme colorScheme) {
-    return Icon(IconData(charCode), color: colorScheme.onPrimary);
+    return Icon(
+      IconData(charCode, fontFamily: 'MaterialIcons'),
+      color: colorScheme.onSurface,
+    );
   }
 }
 

packages/devtools_app/test/inspector_v2/inspector_integration_test.dart

@@ -122,11 +122,6 @@ void main() {
         );
 
         // Verify the properties are displayed:
-        verifyPropertyIsVisible(
-          name: 'widget',
-          value: 'Center',
-          tester: tester,
-        );
         verifyPropertyIsVisible(
           name: 'alignment',
           value: 'Alignment.center',
@@ -386,7 +381,7 @@ void verifyPropertyIsVisible({
   // Verify the property value is visible:
   final propertyValueFinder = find.descendant(
     of: find.byType(PropertyValue),
-    matching: find.text(value),
+    matching: find.richText(value),
   );
   expect(propertyValueFinder, findsOneWidget);