Make table columns sortable by default (#8175)

flutter · Aug 8, 2024 · 99232e2 · 99232e2
1 parent 5596674
commit 99232e2
Show file tree

Hide file tree

Showing 19 changed files with 85 additions and 53 deletions.
diff --git a/packages/devtools_app/lib/src/screens/app_size/app_size_table.dart b/packages/devtools_app/lib/src/screens/app_size/app_size_table.dart
@@ -87,9 +87,6 @@ class _NameColumn extends TreeColumnData<TreemapNode> {
   @override
   String? getCaption(TreemapNode dataObject) => dataObject.caption;
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   String getTooltip(TreemapNode dataObject) => dataObject.displayText();
 
@@ -116,9 +113,6 @@ class _SizeColumn extends ColumnData<TreemapNode> {
     return prettyPrintBytes(dataObject.byteSize, includeUnit: true)!;
   }
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   int compare(TreemapNode a, TreemapNode b) {
     final valueA = getValue(a);
@@ -145,9 +139,6 @@ class _SizePercentageColumn extends ColumnData<TreemapNode> {
   String getDisplayValue(TreemapNode dataObject) =>
       '${getValue(dataObject).toStringAsFixed(2)} %';
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   int compare(TreemapNode a, TreemapNode b) {
     final valueA = getValue(a);
@@ -221,9 +212,6 @@ class _DiffColumn extends ColumnData<TreemapNode> {
   @override
   String getDisplayValue(TreemapNode dataObject) => dataObject.prettyByteSize();
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   int compare(TreemapNode a, TreemapNode b) {
     final valueA = getValue(a);

diff --git a/packages/devtools_app/lib/src/screens/deep_link_validation/deep_links_model.dart b/packages/devtools_app/lib/src/screens/deep_link_validation/deep_links_model.dart
@@ -366,6 +366,12 @@ class DomainColumn extends ColumnData<LinkData>
   DeepLinksController controller;
   SortingOption? sortingOption;
 
+  @override
+  bool get supportsSorting {
+    // Sorting options for this column are handled manually by [controller].
+    return false;
+  }
+
   @override
   Widget? buildHeader(
     BuildContext context,
@@ -429,6 +435,12 @@ class PathColumn extends ColumnData<LinkData>
   DeepLinksController controller;
   SortingOption? sortingOption;
 
+  @override
+  bool get supportsSorting {
+    // Sorting options for this column are handled manually by [controller].
+    return false;
+  }
+
   @override
   Widget? buildHeader(
     BuildContext context,
@@ -503,6 +515,13 @@ class SchemeColumn extends ColumnData<LinkData>
 
   DeepLinksController controller;
 
+  @override
+  bool get supportsSorting {
+    // The sorting action for this column is used instead to provide filter
+    // options. This is handled manually by [controller].
+    return false;
+  }
+
   @override
   Widget? buildHeader(
     BuildContext context,
@@ -551,6 +570,13 @@ class OSColumn extends ColumnData<LinkData>
 
   DeepLinksController controller;
 
+  @override
+  bool get supportsSorting {
+    // The sorting action for this column is used instead to provide filter
+    // options. This is handled manually by [controller].
+    return false;
+  }
+
   @override
   Widget? buildHeader(
     BuildContext context,
@@ -600,6 +626,13 @@ class StatusColumn extends ColumnData<LinkData>
 
   TableViewType viewType;
 
+  @override
+  bool get supportsSorting {
+    // The sorting action for this column is used instead to provide filter
+    // options. This is handled manually by [controller].
+    return false;
+  }
+
   @override
   String getValue(LinkData dataObject) {
     if (dataObject.domainErrors.isNotEmpty) {
@@ -678,6 +711,9 @@ class NavigationColumn extends ColumnData<LinkData>
           fixedWidthPx: scaleByFontFactor(40),
         );
 
+  @override
+  bool get supportsSorting => false;
+
   @override
   String getValue(LinkData dataObject) => '';
 

diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart
@@ -677,7 +677,7 @@ class InspectorController extends DisposableController
     }
     _selectedNodeProperties.value = (
       widgetProperties: widgetProperties,
-      renderProperties: renderProperties
+      renderProperties: renderProperties,
     );
   }
 

diff --git a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/class_details/paths.dart b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/class_details/paths.dart
@@ -28,9 +28,6 @@ class _RetainingPathColumn extends ColumnData<PathData> {
   String? getValue(PathData record) =>
       record.path.toShortString(inverted: true);
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   String getTooltip(PathData record) => '';
 }

diff --git a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_diff.dart b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_diff.dart
@@ -43,9 +43,6 @@ class _ClassNameColumn extends ColumnData<DiffClassData>
   @override
   String? getValue(DiffClassData data) => data.className.className;
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   // Tooltip is removed, because it is provided by [HeapClassView].
   String getTooltip(DiffClassData data) => '';

diff --git a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_single.dart b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_single.dart
@@ -36,9 +36,6 @@ class _ClassNameColumn extends ColumnData<SingleClassData>
   @override
   String? getValue(SingleClassData data) => data.className.className;
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   // We are removing the tooltip, because it is provided by [HeapClassView].
   String getTooltip(SingleClassData data) => '';

diff --git a/packages/devtools_app/lib/src/screens/memory/panes/profile/profile_view.dart b/packages/devtools_app/lib/src/screens/memory/panes/profile/profile_view.dart
@@ -49,9 +49,6 @@ class _FieldClassNameColumn extends ColumnData<ProfileRecord>
   @override
   String getTooltip(ProfileRecord dataObject) => '';
 
-  @override
-  bool get supportsSorting => true;
-
   final ClassFilterData classFilterData;
 
   @override
@@ -272,6 +269,9 @@ class _GCHeapNameColumn extends ColumnData<AdaptedProfile> {
   String? getValue(AdaptedProfile dataObject) {
     return 'GC Statistics';
   }
+
+  @override
+  bool get supportsSorting => false;
 }
 
 class _GCHeapUsageColumn extends _GCHeapStatsColumn {

diff --git a/packages/devtools_app/lib/src/screens/memory/panes/tracing/class_table.dart b/packages/devtools_app/lib/src/screens/memory/panes/tracing/class_table.dart
@@ -80,9 +80,6 @@ class _ClassNameColumn extends ColumnData<TracedClass>
 
   final String? rootPackage;
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   Widget build(
     BuildContext context,

diff --git a/packages/devtools_app/lib/src/screens/performance/panes/rebuild_stats/rebuild_stats.dart b/packages/devtools_app/lib/src/screens/performance/panes/rebuild_stats/rebuild_stats.dart
@@ -234,7 +234,7 @@ class _WidgetColumn extends ColumnData<RebuildLocationStats> {
 
   @override
   String getValue(RebuildLocationStats dataObject) {
-    return dataObject.location.name ?? '???';
+    return dataObject.location.name ?? '<unknown>';
   }
 }
 
@@ -270,9 +270,9 @@ class _RebuildCountColumn extends ColumnData<RebuildLocationStats> {
   final int metricIndex;
 
   @override
-  int getValue(RebuildLocationStats dataObject) =>
-      dataObject.buildCounts[metricIndex];
+  bool get numeric => true;
 
   @override
-  bool get numeric => true;
+  int getValue(RebuildLocationStats dataObject) =>
+      dataObject.buildCounts[metricIndex];
 }
diff --git a/packages/devtools_app/lib/src/screens/profiler/panes/cpu_profile_columns.dart b/packages/devtools_app/lib/src/screens/profiler/panes/cpu_profile_columns.dart
@@ -52,9 +52,6 @@ class MethodAndSourceColumn extends TreeColumnData<CpuStackFrame>
     return dataObject.name;
   }
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   Widget? build(
     BuildContext context,

diff --git a/packages/devtools_app/lib/src/screens/profiler/panes/method_table/method_table.dart b/packages/devtools_app/lib/src/screens/profiler/panes/method_table/method_table.dart
@@ -254,9 +254,6 @@ class _MethodColumn extends ColumnData<MethodTableGraphNode>
           minWidthPx: _methodColumnMinWidth,
         );
 
-  @override
-  bool get supportsSorting => true;
-
   @override
   String getValue(MethodTableGraphNode dataObject) => dataObject.name;
 

diff --git a/...devtools_app/lib/src/screens/vm_developer/isolate_statistics/isolate_statistics_view.dart b/...devtools_app/lib/src/screens/vm_developer/isolate_statistics/isolate_statistics_view.dart
@@ -294,6 +294,9 @@ class _PortNameColumn extends ColumnData<InstanceRef> {
 class _StackTraceViewerFrameColumn extends ColumnData<String> {
   _StackTraceViewerFrameColumn() : super.wide('Frame');
 
+  @override
+  bool get supportsSorting => false;
+
   @override
   String getValue(String frame) => frame;
 

diff --git a/packages/devtools_app/lib/src/screens/vm_developer/object_inspector/object_store.dart b/packages/devtools_app/lib/src/screens/vm_developer/object_inspector/object_store.dart
@@ -34,6 +34,9 @@ class _ObjectColumn extends ColumnData<ObjectStoreEntry>
   @override
   bool get includeHeader => true;
 
+  @override
+  bool get supportsSorting => false;
+
   @override
   ObjRef getValue(ObjectStoreEntry dataObject) {
     return dataObject.value;

diff --git a/...devtools_app/lib/src/screens/vm_developer/process_memory/process_memory_tree_columns.dart b/...devtools_app/lib/src/screens/vm_developer/process_memory/process_memory_tree_columns.dart
@@ -18,6 +18,9 @@ class DescriptionColumn extends ColumnData<TreemapNode> {
 
   @override
   String getValue(TreemapNode dataObject) => dataObject.caption ?? '';
+
+  @override
+  bool get supportsSorting => false;
 }
 
 class MemoryColumn extends SizeAndPercentageColumn<TreemapNode> {

diff --git a/packages/devtools_app/lib/src/screens/vm_developer/vm_statistics/vm_statistics_view.dart b/packages/devtools_app/lib/src/screens/vm_developer/vm_statistics/vm_statistics_view.dart
@@ -246,6 +246,9 @@ class _IsolateNewSpaceColumn extends _IsolateMemoryColumn {
 
   @override
   int getUsage(Isolate i) => i.newSpaceUsage;
+
+  @override
+  bool get numeric => true;
 }
 
 class _IsolateOldSpaceColumn extends _IsolateMemoryColumn {
@@ -256,6 +259,9 @@ class _IsolateOldSpaceColumn extends _IsolateMemoryColumn {
 
   @override
   int getUsage(Isolate i) => i.oldSpaceUsage;
+
+  @override
+  bool get numeric => true;
 }
 
 class _IsolateHeapColumn extends _IsolateMemoryColumn {
@@ -266,6 +272,9 @@ class _IsolateHeapColumn extends _IsolateMemoryColumn {
 
   @override
   int getUsage(Isolate i) => i.dartHeapSize;
+
+  @override
+  bool get numeric => true;
 }
 
 /// Displays general statistics about running isolates including:

diff --git a/packages/devtools_app/lib/src/shared/table/table_data.dart b/packages/devtools_app/lib/src/shared/table/table_data.dart
@@ -64,15 +64,17 @@ abstract class ColumnData<T> {
 
   bool get includeHeader => true;
 
-  bool get supportsSorting => numeric;
+  bool get supportsSorting => true;
 
   int compare(T a, T b) {
     final valueA = getValue(a);
     final valueB = getValue(b);
     if (valueA == null && valueB == null) return 0;
     if (valueA == null) return -1;
     if (valueB == null) return 1;
-    return (valueA as Comparable).compareTo(valueB as Comparable);
+
+    if (valueA is! Comparable || valueB is! Comparable) return 0;
+    return valueA.compareTo(valueB);
   }
 
   /// Get the cell's value from the given [dataObject].

diff --git a/packages/devtools_app/lib/src/shared/ui/vm_flag_widgets.dart b/packages/devtools_app/lib/src/shared/ui/vm_flag_widgets.dart
@@ -271,6 +271,9 @@ class _DescriptionColumn extends ColumnData<_DialogFlag> {
   @override
   String getValue(_DialogFlag dataObject) => dataObject.description ?? '';
 
+  @override
+  bool get supportsSorting => false;
+
   @override
   String getTooltip(_DialogFlag dataObject) => getValue(dataObject);
 }
@@ -286,6 +289,9 @@ class _ValueColumn extends ColumnData<_DialogFlag> {
 
   @override
   String getValue(_DialogFlag dataObject) => dataObject.value ?? '';
+
+  @override
+  bool get supportsSorting => false;
 }
 
 class _DialogFlag {

diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
@@ -10,7 +10,7 @@ To learn more about DevTools, check out the
 
 ## General updates
 
-TODO: Remove this section if there are not any general updates.
+* Changed table columns to be sortable by default. - [#8175](https://github.com/flutter/devtools/pull/8175)
 
 ## Inspector updates