From 99232e2d304eab4f5d1e73a806350c973c170808 Mon Sep 17 00:00:00 2001 From: Kenzie Davisson <43759233+kenzieschmoll@users.noreply.github.com> Date: Thu, 8 Aug 2024 14:25:49 -0700 Subject: [PATCH] Make table columns sortable by default (#8175) --- .../src/screens/app_size/app_size_table.dart | 12 ------- .../deep_links_model.dart | 36 +++++++++++++++++++ .../inspector_v2/inspector_controller.dart | 2 +- .../diff/widgets/class_details/paths.dart | 3 -- .../diff/widgets/classes_table_diff.dart | 3 -- .../diff/widgets/classes_table_single.dart | 3 -- .../memory/panes/profile/profile_view.dart | 6 ++-- .../memory/panes/tracing/class_table.dart | 3 -- .../panes/rebuild_stats/rebuild_stats.dart | 8 ++--- .../profiler/panes/cpu_profile_columns.dart | 3 -- .../panes/method_table/method_table.dart | 3 -- .../isolate_statistics_view.dart | 3 ++ .../object_inspector/object_store.dart | 3 ++ .../process_memory_tree_columns.dart | 3 ++ .../vm_statistics/vm_statistics_view.dart | 9 +++++ .../lib/src/shared/table/table_data.dart | 6 ++-- .../lib/src/shared/ui/vm_flag_widgets.dart | 6 ++++ .../release_notes/NEXT_RELEASE_NOTES.md | 2 +- .../devtools_app/test/shared/table_test.dart | 24 ++++++------- 19 files changed, 85 insertions(+), 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 index 728e065611c..17807d66dd7 100644 --- 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 { @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 { 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 { 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 { @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 index ab835df8bc5..863d3259d9c 100644 --- 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 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 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 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 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 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 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 index 36893243dae..8dc2c46a68a 100644 --- 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 index e90457d9ff6..edc5ba8e0f6 100644 --- 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 { 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 index a7ba90bb3bd..d5f7baabaae 100644 --- 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 @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 index 0d19db877b7..8e2dfdb3dc8 100644 --- 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 @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 index 79832c05687..c7f9fcfbdfe 100644 --- 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 @override String getTooltip(ProfileRecord dataObject) => ''; - @override - bool get supportsSorting => true; - final ClassFilterData classFilterData; @override @@ -272,6 +269,9 @@ class _GCHeapNameColumn extends ColumnData { 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 index 28a6ad1dd6b..7135fc58f85 100644 --- 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 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 index f219dc74a92..146f3268d92 100644 --- 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 { @override String getValue(RebuildLocationStats dataObject) { - return dataObject.location.name ?? '???'; + return dataObject.location.name ?? ''; } } @@ -270,9 +270,9 @@ class _RebuildCountColumn extends ColumnData { 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 index c577e5a6c31..90440589ead 100644 --- 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 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 index c0fae4e3fe3..6b938b14d89 100644 --- 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 minWidthPx: _methodColumnMinWidth, ); - @override - bool get supportsSorting => true; - @override String getValue(MethodTableGraphNode dataObject) => dataObject.name; diff --git a/packages/devtools_app/lib/src/screens/vm_developer/isolate_statistics/isolate_statistics_view.dart b/packages/devtools_app/lib/src/screens/vm_developer/isolate_statistics/isolate_statistics_view.dart index 025fb87d380..430e3fbc005 100644 --- a/packages/devtools_app/lib/src/screens/vm_developer/isolate_statistics/isolate_statistics_view.dart +++ b/packages/devtools_app/lib/src/screens/vm_developer/isolate_statistics/isolate_statistics_view.dart @@ -294,6 +294,9 @@ class _PortNameColumn extends ColumnData { class _StackTraceViewerFrameColumn extends ColumnData { _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 index 23440efd7b6..64a7fcf7e4b 100644 --- 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 @override bool get includeHeader => true; + @override + bool get supportsSorting => false; + @override ObjRef getValue(ObjectStoreEntry dataObject) { return dataObject.value; diff --git a/packages/devtools_app/lib/src/screens/vm_developer/process_memory/process_memory_tree_columns.dart b/packages/devtools_app/lib/src/screens/vm_developer/process_memory/process_memory_tree_columns.dart index 7ecc4d72387..f5ad49efd8f 100644 --- a/packages/devtools_app/lib/src/screens/vm_developer/process_memory/process_memory_tree_columns.dart +++ b/packages/devtools_app/lib/src/screens/vm_developer/process_memory/process_memory_tree_columns.dart @@ -18,6 +18,9 @@ class DescriptionColumn extends ColumnData { @override String getValue(TreemapNode dataObject) => dataObject.caption ?? ''; + + @override + bool get supportsSorting => false; } class MemoryColumn extends SizeAndPercentageColumn { 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 index ab0d835bd50..ea213380793 100644 --- 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 index 1ee874c0eda..a669787ec7a 100644 --- a/packages/devtools_app/lib/src/shared/table/table_data.dart +++ b/packages/devtools_app/lib/src/shared/table/table_data.dart @@ -64,7 +64,7 @@ abstract class ColumnData { bool get includeHeader => true; - bool get supportsSorting => numeric; + bool get supportsSorting => true; int compare(T a, T b) { final valueA = getValue(a); @@ -72,7 +72,9 @@ abstract class ColumnData { 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 index f9906eae773..30a359c3111 100644 --- 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 index ea285bc1b29..07b358cb021 100644 --- 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 diff --git a/packages/devtools_app/test/shared/table_test.dart b/packages/devtools_app/test/shared/table_test.dart index e00958126a4..a528467fccd 100644 --- a/packages/devtools_app/test/shared/table_test.dart +++ b/packages/devtools_app/test/shared/table_test.dart @@ -1747,9 +1747,6 @@ class _NameColumn extends TreeColumnData { @override String getValue(TestData dataObject) => dataObject.name; - - @override - bool get supportsSorting => true; } class _NumberColumn extends ColumnData { @@ -1761,9 +1758,6 @@ class _NumberColumn extends ColumnData { @override int getValue(TestData dataObject) => dataObject.number; - - @override - bool get supportsSorting => true; } class _FlatNameColumn extends ColumnData { @@ -1775,9 +1769,6 @@ class _FlatNameColumn extends ColumnData { @override String getValue(TestData dataObject) => dataObject.name; - - @override - bool get supportsSorting => true; } class _PinnableFlatNameColumn extends ColumnData { @@ -1789,9 +1780,6 @@ class _PinnableFlatNameColumn extends ColumnData { @override String getValue(PinnableTestData dataObject) => dataObject.name; - - @override - bool get supportsSorting => true; } class _CombinedColumn extends ColumnData { @@ -1804,6 +1792,9 @@ class _CombinedColumn extends ColumnData { @override String getValue(TestData dataObject) => '${dataObject.name} ${dataObject.number}'; + + @override + bool get supportsSorting => false; } class _WideColumn extends ColumnData { @@ -1812,6 +1803,9 @@ class _WideColumn extends ColumnData { @override String getValue(TestData dataObject) => '${dataObject.name} ${dataObject.number} bla bla bla bla bla bla bla bla'; + + @override + bool get supportsSorting => false; } class _WideMinWidthColumn extends ColumnData { @@ -1824,6 +1818,9 @@ class _WideMinWidthColumn extends ColumnData { @override String getValue(TestData dataObject) => '${dataObject.name} ${dataObject.number} with min width'; + + @override + bool get supportsSorting => false; } class _VeryWideMinWidthColumn extends ColumnData { @@ -1836,4 +1833,7 @@ class _VeryWideMinWidthColumn extends ColumnData { @override String getValue(TestData dataObject) => '${dataObject.name} ${dataObject.number} with min width'; + + @override + bool get supportsSorting => false; }