From b4340cd8616a15ac4785c8f3eb04de0d74a978ad Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Fri, 13 Mar 2020 16:44:24 -0700 Subject: [PATCH 1/4] Add sorting functionality to flutter tables. --- .../devtools_app/lib/src/flutter/table.dart | 153 +++++++++-- .../devtools_app/lib/src/html_tables.dart | 7 +- .../src/logging/flutter/logging_screen.dart | 19 +- .../flutter/performance_screen.dart | 9 +- .../performance/performance_controller.dart | 2 +- .../src/profiler/cpu_profile_controller.dart | 160 +++++++++++- .../flutter/cpu_profile_bottom_up.dart | 24 +- .../flutter/cpu_profile_call_tree.dart | 24 +- .../src/profiler/flutter/cpu_profiler.dart | 2 +- packages/devtools_app/lib/src/table_data.dart | 32 +-- .../src/timeline/flutter/event_details.dart | 4 +- .../timeline/flutter/timeline_controller.dart | 2 +- .../timeline/html_timeline_controller.dart | 2 +- packages/devtools_app/lib/src/utils.dart | 13 + .../test/cpu_profiler_controller_test.dart | 36 +-- .../devtools_app/test/flutter/table_test.dart | 238 +++++++++++++++++- .../test/performance_controller_test.dart | 4 +- .../lib/support/flutter_test_environment.dart | 6 +- 18 files changed, 635 insertions(+), 102 deletions(-) diff --git a/packages/devtools_app/lib/src/flutter/table.dart b/packages/devtools_app/lib/src/flutter/table.dart index 87f6b5bd7dc..659116e0fc5 100644 --- a/packages/devtools_app/lib/src/flutter/table.dart +++ b/packages/devtools_app/lib/src/flutter/table.dart @@ -5,6 +5,7 @@ import 'package:flutter/material.dart' hide TableRow; import '../table_data.dart'; import '../trees.dart'; import '../ui/theme.dart'; +import '../utils.dart'; import 'collapsible_mixin.dart'; import 'flutter_widgets/linked_scroll_controller.dart'; import 'theme.dart'; @@ -27,6 +28,8 @@ class FlatTable extends StatefulWidget { @required this.data, @required this.keyFactory, @required this.onItemSelected, + @required this.startingSortColumn, + @required this.startingSortDirection, }) : assert(columns != null), assert(keyFactory != null), assert(data != null), @@ -42,22 +45,28 @@ class FlatTable extends StatefulWidget { final ItemCallback onItemSelected; + final ColumnData startingSortColumn; + + final SortDirection startingSortDirection; + @override _FlatTableState createState() => _FlatTableState(); } -class _FlatTableState extends State> { +class _FlatTableState extends State> with TableSortMixin { List columnWidths; @override void initState() { super.initState(); + sortData(widget.startingSortColumn, widget.startingSortDirection); columnWidths = _computeColumnWidths(); } @override void didUpdateWidget(FlatTable oldWidget) { super.didUpdateWidget(oldWidget); + sortData(widget.startingSortColumn, widget.startingSortDirection); columnWidths = _computeColumnWidths(); } @@ -83,6 +92,9 @@ class _FlatTableState extends State> { columnWidths: columnWidths, startAtBottom: true, rowBuilder: _buildRow, + startingSortColumn: widget.startingSortColumn, + startingSortDirection: widget.startingSortDirection, + onSortChanged: _sortDataAndUpdate, ); } @@ -102,6 +114,17 @@ class _FlatTableState extends State> { backgroundColor: TableRow.colorFor(context, index), ); } + + void _sortDataAndUpdate(ColumnData column, SortDirection direction) { + setState(() { + sortData(column, direction); + }); + } + + @override + void sortData(ColumnData column, SortDirection direction) { + widget.data.sort((T a, T b) => _compareData(a, b, column, direction)); + } } // TODO(https://github.com/flutter/devtools/issues/1657) @@ -128,7 +151,10 @@ class TreeTable> extends StatefulWidget { @required this.treeColumn, @required this.dataRoots, @required this.keyFactory, + @required this.startingSortColumn, + @required this.startingSortDirection, }) : assert(columns.contains(treeColumn)), + assert(columns.contains(startingSortColumn)), assert(columns != null), assert(keyFactory != null), assert(dataRoots != null), @@ -146,12 +172,19 @@ class TreeTable> extends StatefulWidget { /// Factory that creates keys for each row in this table. final Key Function(T) keyFactory; + final ColumnData startingSortColumn; + + final SortDirection startingSortDirection; + @override _TreeTableState createState() => _TreeTableState(); } class _TreeTableState> extends State> - with TickerProviderStateMixin { + with TickerProviderStateMixin, TableSortMixin { + /// The number of items to show when animating out the tree table. + static const itemsToShowWhenAnimating = 50; + List items; List animatingChildren = []; Set animatingChildrenSet = {}; @@ -159,12 +192,10 @@ class _TreeTableState> extends State> List columnWidths; List rootsExpanded; - /// The number of items to show when animating out the tree table. - static const itemsToShowWhenAnimating = 50; - @override void initState() { super.initState(); + sortData(widget.startingSortColumn, widget.startingSortDirection); rootsExpanded = List.generate( widget.dataRoots.length, (index) => widget.dataRoots[index].isExpanded); _updateItems(); @@ -173,6 +204,7 @@ class _TreeTableState> extends State> @override void didUpdateWidget(TreeTable oldWidget) { super.didUpdateWidget(oldWidget); + sortData(widget.startingSortColumn, widget.startingSortDirection); rootsExpanded = List.generate( widget.dataRoots.length, (index) => widget.dataRoots[index].isExpanded); _updateItems(); @@ -245,7 +277,8 @@ class _TreeTableState> extends State> } final widths = []; for (ColumnData column in widget.columns) { - double width = column.getNodeIndentPx(deepest); + double width = + column.getNodeIndentPx(deepest, indentForTreeToggle: false); if (column.fixedWidthPx != null) { width += column.fixedWidthPx; } else { @@ -291,6 +324,9 @@ class _TreeTableState> extends State> itemCount: items.length, columnWidths: columnWidths, rowBuilder: _buildRow, + startingSortColumn: widget.startingSortColumn, + startingSortDirection: widget.startingSortDirection, + onSortChanged: _sortDataAndUpdate, ); } @@ -326,6 +362,24 @@ class _TreeTableState> extends State> if (animatingChildrenSet.contains(node)) return const SizedBox(); return rowForNode(node); } + + @override + void sortData(ColumnData column, SortDirection direction) { + void _sort(T dataObject) { + dataObject.children + ..sort((T a, T b) => _compareData(a, b, column, direction)) + ..forEach(_sort); + } + + widget.dataRoots.where((dataRoot) => dataRoot.level == 0).toList() + ..sort((T a, T b) => _compareData(a, b, column, direction)) + ..forEach(_sort); + } + + void _sortDataAndUpdate(ColumnData column, SortDirection direction) { + sortData(column, direction); + _updateItems(); + } } class _Table extends StatefulWidget { @@ -335,6 +389,9 @@ class _Table extends StatefulWidget { @required this.columns, @required this.columnWidths, @required this.rowBuilder, + @required this.startingSortColumn, + @required this.startingSortDirection, + @required this.onSortChanged, this.startAtBottom = false}) : super(key: key); @@ -343,6 +400,9 @@ class _Table extends StatefulWidget { final List> columns; final List columnWidths; final IndexedScrollableWidgetBuilder rowBuilder; + final ColumnData startingSortColumn; + final SortDirection startingSortDirection; + final Function(ColumnData column, SortDirection direction) onSortChanged; /// The width to assume for columns that don't specify a width. static const defaultColumnWidth = 500.0; @@ -358,16 +418,20 @@ class _Table extends StatefulWidget { static const rowHorizontalPadding = 16.0; @override - __TableState createState() => __TableState(); + _TableState createState() => _TableState(); } -class __TableState extends State<_Table> { +class _TableState extends State<_Table> { LinkedScrollControllerGroup _linkedHorizontalScrollControllerGroup; + ColumnData sortColumn; + SortDirection sortDirection; @override void initState() { super.initState(); _linkedHorizontalScrollControllerGroup = LinkedScrollControllerGroup(); + sortColumn = widget.startingSortColumn; + sortDirection = widget.startingSortDirection; } /// The width of all columns in the table, with additional padding. @@ -409,6 +473,9 @@ class __TableState extends State<_Table> { _linkedHorizontalScrollControllerGroup, columns: widget.columns, columnWidths: widget.columnWidths, + sortColumn: sortColumn, + sortDirection: sortDirection, + onSortChanged: _sortData, ), Expanded( child: Scrollbar( @@ -423,6 +490,12 @@ class __TableState extends State<_Table> { ); }); } + + void _sortData(ColumnData column, SortDirection direction) { + sortDirection = direction; + sortColumn = column; + widget.onSortChanged(column, direction); + } } /// Callback for when a specific item in a table is selected. @@ -441,8 +514,8 @@ class TableRow extends StatefulWidget { @required this.linkedScrollControllerGroup, @required this.node, @required this.columns, - @required this.onPressed, @required this.columnWidths, + @required this.onPressed, this.backgroundColor, this.expandableColumn, this.expansionChildren, @@ -450,7 +523,10 @@ class TableRow extends StatefulWidget { this.isExpanded = false, this.isExpandable = false, this.isShown = true, - }) : super(key: key); + }) : sortColumn = null, + sortDirection = null, + onSortChanged = null, + super(key: key); /// Constructs a [TableRow] that presents the column titles instead /// of any [node]. @@ -459,6 +535,9 @@ class TableRow extends StatefulWidget { @required this.linkedScrollControllerGroup, @required this.columns, @required this.columnWidths, + @required this.sortColumn, + @required this.sortDirection, + @required this.onSortChanged, this.onPressed, }) : node = null, isExpanded = false, @@ -509,6 +588,12 @@ class TableRow extends StatefulWidget { /// If null, defaults to `Theme.of(context).canvasColor`. final Color backgroundColor; + final ColumnData sortColumn; + + final SortDirection sortDirection; + + final Function(ColumnData column, SortDirection direction) onSortChanged; + /// Gets a color to use for this row at a given index. static Color colorFor(BuildContext context, int index) { final theme = Theme.of(context); @@ -619,12 +704,29 @@ class _TableRowState extends State> Widget content; final node = widget.node; if (node == null) { - content = Text( - column.title, - overflow: TextOverflow.ellipsis, + final isSortColumn = column.title == widget.sortColumn.title; + content = InkWell( + onTap: () => _handleSortChange(column), + child: Row( + children: [ + if (isSortColumn) + Icon( + widget.sortDirection == SortDirection.ascending + ? Icons.expand_less + : Icons.expand_more, + size: 16.0, + ), + const SizedBox(width: 4.0), + Text( + column.title, + overflow: TextOverflow.ellipsis, + ), + ], + ), ); } else { - final padding = column.getNodeIndentPx(node); + final padding = + column.getNodeIndentPx(node, indentForTreeToggle: false); content = Text( '${column.getDisplayValue(node)}', overflow: TextOverflow.ellipsis, @@ -689,4 +791,27 @@ class _TableRowState extends State> @override bool shouldShow() => widget.isShown; + + void _handleSortChange(ColumnData columnData) { + SortDirection direction; + if (columnData.title == widget.sortColumn.title) { + direction = widget.sortDirection.opposite(); + } else if (columnData.numeric) { + direction = SortDirection.descending; + } else { + direction = SortDirection.ascending; + } + widget.onSortChanged(columnData, direction); + } +} + +mixin TableSortMixin { + int _compareFactor(SortDirection direction) => + direction == SortDirection.ascending ? 1 : -1; + + int _compareData(T a, T b, ColumnData column, SortDirection direction) { + return column.compare(a, b) * _compareFactor(direction); + } + + void sortData(ColumnData column, SortDirection direction); } diff --git a/packages/devtools_app/lib/src/html_tables.dart b/packages/devtools_app/lib/src/html_tables.dart index 65327f219c4..3e0605ed448 100644 --- a/packages/devtools_app/lib/src/html_tables.dart +++ b/packages/devtools_app/lib/src/html_tables.dart @@ -12,6 +12,7 @@ import 'trees.dart'; import 'ui/html_custom.dart'; import 'ui/html_elements.dart'; import 'ui/primer.dart'; +import 'utils.dart'; class HtmlHoverCell extends HoverCellData { HtmlHoverCell(this.cell, T data) : super(data); @@ -170,7 +171,7 @@ class HtmlTable with HtmlSetStateMixin implements TableDataClient { } @override - void onColumnSortChanged(ColumnData column, SortOrder sortDirection) { + void onColumnSortChanged(ColumnData column, SortDirection sortDirection) { // Update the UI to reflect the new column sort order. // The base class will sort the actual data. @@ -178,8 +179,8 @@ class HtmlTable with HtmlSetStateMixin implements TableDataClient { for (ColumnData c in model.columns) { final CoreElement s = _spanForColumn[c]; if (c == column) { - s.toggleClass('up', sortDirection == SortOrder.ascending); - s.toggleClass('down', sortDirection != SortOrder.ascending); + s.toggleClass('up', sortDirection == SortDirection.ascending); + s.toggleClass('down', sortDirection != SortDirection.ascending); } else { s.toggleClass('up', false); s.toggleClass('down', false); diff --git a/packages/devtools_app/lib/src/logging/flutter/logging_screen.dart b/packages/devtools_app/lib/src/logging/flutter/logging_screen.dart index 71bb6b6fd5a..46bbdd91b5b 100644 --- a/packages/devtools_app/lib/src/logging/flutter/logging_screen.dart +++ b/packages/devtools_app/lib/src/logging/flutter/logging_screen.dart @@ -13,6 +13,7 @@ import '../../flutter/table.dart'; import '../../flutter/theme.dart'; import '../../table_data.dart'; import '../../ui/flutter/service_extension_widgets.dart'; +import '../../utils.dart'; import '../logging_controller.dart'; /// Presents logs from the connected app. @@ -86,15 +87,19 @@ class _LoggingScreenState extends State } class LogsTable extends StatelessWidget { - const LogsTable({Key key, this.data, this.onItemSelected}) : super(key: key); + LogsTable({ + Key key, + this.data, + this.onItemSelected, + }) : super(key: key); + final List data; final ItemCallback onItemSelected; - List> get columns => [ - _WhenColumn(), - _KindColumn(), - _MessageColumn((message) => message), - ]; + final ColumnData when = _WhenColumn(); + final ColumnData kind = _KindColumn(); + final ColumnData message = _MessageColumn((message) => message); + List> get columns => [when, kind, message]; @override Widget build(BuildContext context) { @@ -103,6 +108,8 @@ class LogsTable extends StatelessWidget { data: data, keyFactory: (LogData data) => ValueKey(data), onItemSelected: onItemSelected, + startingSortColumn: when, + startingSortDirection: SortDirection.ascending, ); } } diff --git a/packages/devtools_app/lib/src/performance/flutter/performance_screen.dart b/packages/devtools_app/lib/src/performance/flutter/performance_screen.dart index 4544d57dfd1..599aafd01d6 100644 --- a/packages/devtools_app/lib/src/performance/flutter/performance_screen.dart +++ b/packages/devtools_app/lib/src/performance/flutter/performance_screen.dart @@ -64,10 +64,9 @@ class _PerformanceScreenBodyState extends State recording = controller.recordingNotifier.value; }); }); - addAutoDisposeListener(controller.cpuProfilerController.processingNotifier, - () { + addAutoDisposeListener(controller.cpuProfilerController.processing, () { setState(() { - processing = controller.cpuProfilerController.processingNotifier.value; + processing = controller.cpuProfilerController.processing.value; }); }); addAutoDisposeListener( @@ -82,7 +81,7 @@ class _PerformanceScreenBodyState extends State @override Widget build(BuildContext context) { return ValueListenableBuilder( - valueListenable: controller.cpuProfilerController.profilerFlagNotifier, + valueListenable: controller.cpuProfilerController.profilerFlag, builder: (context, profilerFlag, _) { return profilerFlag.valueAsString == 'true' ? _buildPerformanceBody(controller) @@ -103,7 +102,7 @@ class _PerformanceScreenBodyState extends State ), Expanded( child: ValueListenableBuilder( - valueListenable: controller.cpuProfilerController.dataNotifier, + valueListenable: controller.cpuProfilerController.data, builder: (context, cpuProfileData, _) { if (cpuProfileData == CpuProfilerController.baseStateCpuProfileData || diff --git a/packages/devtools_app/lib/src/performance/performance_controller.dart b/packages/devtools_app/lib/src/performance/performance_controller.dart index e2d4ed3961c..ebd42759508 100644 --- a/packages/devtools_app/lib/src/performance/performance_controller.dart +++ b/packages/devtools_app/lib/src/performance/performance_controller.dart @@ -12,7 +12,7 @@ import '../utils.dart'; class PerformanceController { final cpuProfilerController = CpuProfilerController(); - CpuProfileData get cpuProfileData => cpuProfilerController.dataNotifier.value; + CpuProfileData get cpuProfileData => cpuProfilerController.data.value; /// Notifies that a CPU profile is currently being recorded. ValueListenable get recordingNotifier => _recordingNotifier; diff --git a/packages/devtools_app/lib/src/profiler/cpu_profile_controller.dart b/packages/devtools_app/lib/src/profiler/cpu_profile_controller.dart index 2296fa4ac6b..b1ce6e9bbd4 100644 --- a/packages/devtools_app/lib/src/profiler/cpu_profile_controller.dart +++ b/packages/devtools_app/lib/src/profiler/cpu_profile_controller.dart @@ -19,16 +19,15 @@ class CpuProfilerController { static CpuProfileData baseStateCpuProfileData = CpuProfileData.empty(); /// Notifies that new cpu profile data is available. - ValueListenable get dataNotifier => _dataNotifier; + ValueListenable get data => _dataNotifier; final _dataNotifier = ValueNotifier(baseStateCpuProfileData); /// Notifies that CPU profile data is currently being processed. - ValueListenable get processingNotifier => _processingNotifier; + ValueListenable get processing => _processingNotifier; final _processingNotifier = ValueNotifier(false); /// Notifies that a cpu stack frame was selected. - ValueListenable get selectedCpuStackFrameNotifier => - _selectedCpuStackFrameNotifier; + ValueListenable get selectedCpuStackFrame => _selectedCpuStackFrameNotifier; final _selectedCpuStackFrameNotifier = ValueNotifier(null); final service = CpuProfilerService(); @@ -36,15 +35,14 @@ class CpuProfilerController { final transformer = CpuProfileTransformer(); /// Notifies that the vm profiler flag has changed. - ValueListenable get profilerFlagNotifier => service.profilerFlagNotifier; + ValueListenable get profilerFlag => service.profilerFlagNotifier; /// Whether the profiler is enabled. /// - /// Clients interested in the current value of [profilerFlagNotifier] should + /// Clients interested in the current value of [profilerFlag] should /// use this getter. Otherwise, clients subscribing to change notifications, - /// should listen to [profilerFlagNotifier]. - bool get profilerEnabled => - profilerFlagNotifier.value.valueAsString == 'true'; + /// should listen to [profilerFlag]. + bool get profilerEnabled => profilerFlag.value.valueAsString == 'true'; Future enableCpuProfiler() { return service.enableCpuProfiler(); @@ -88,8 +86,8 @@ class CpuProfilerController { } void selectCpuStackFrame(CpuStackFrame stackFrame) { - if (stackFrame == dataNotifier.value.selectedStackFrame) return; - dataNotifier.value.selectedStackFrame = stackFrame; + if (stackFrame == data.value.selectedStackFrame) return; + data.value.selectedStackFrame = stackFrame; _selectedCpuStackFrameNotifier.value = stackFrame; } @@ -112,3 +110,143 @@ class CpuProfilerController { transformer.dispose(); } } + +final Map cpuProfileResponseJson = { + 'type': '_CpuProfileTimeline', + 'samplePeriod': 50, + 'stackDepth': 128, + 'sampleCount': 7, + 'timeSpan': 0.003678, + 'timeOriginMicros': 47377800463, + 'timeExtentMicros': 600, + 'stackFrames': goldenCpuProfileStackFrames, + 'traceEvents': goldenCpuProfileTraceEvents, +}; + +final Map goldenCpuProfileStackFrames = { + '140357727781376-1': { + 'category': 'Dart', + 'name': 'A', + 'resolvedUrl': 'B', + }, + '140357727781376-2': { + 'category': 'Dart', + 'name': 'A1', + 'parent': '140357727781376-1', + 'resolvedUrl': 'B', + }, + '140357727781376-3': { + 'category': 'Dart', + 'name': 'A2', + 'parent': '140357727781376-1', + 'resolvedUrl': 'A', + }, + '140357727781376-4': { + 'category': 'Dart', + 'name': 'A2-A child', + 'parent': '140357727781376-3', + 'resolvedUrl': 'B', + }, + '140357727781376-5': { + 'category': 'Dart', + 'name': 'A2-B child', + 'parent': '140357727781376-3', + 'resolvedUrl': 'A', + }, + '140357727781376-6': { + 'category': 'Dart', + 'name': 'A2-C child', + 'parent': '140357727781376-3', + 'resolvedUrl': 'C', + }, + '140357727781376-7': { + 'category': 'Dart', + 'name': 'B', + 'resolvedUrl': 'A', + }, + '140357727781376-8': { + 'category': 'Dart', + 'name': 'B1', + 'parent': '140357727781376-7', + 'resolvedUrl': 'B', + }, + '140357727781376-9': { + 'category': 'Dart', + 'name': 'B2', + 'parent': '140357727781376-7', + 'resolvedUrl': 'A', + }, +}; + +final List> goldenCpuProfileTraceEvents = [ + { + 'ph': 'P', + 'name': '', + 'pid': 77616, + 'tid': 42247, + 'ts': 47377800463, + 'cat': 'Dart', + 'args': {'mode': 'basic'}, + 'sf': '140357727781376-2' + }, + { + 'ph': 'P', + 'name': '', + 'pid': 77616, + 'tid': 42247, + 'ts': 47377800563, + 'cat': 'Dart', + 'args': {'mode': 'basic'}, + 'sf': '140357727781376-2' + }, + { + 'ph': 'P', + 'name': '', + 'pid': 77616, + 'tid': 42247, + 'ts': 47377800663, + 'cat': 'Dart', + 'args': {'mode': 'basic'}, + 'sf': '140357727781376-4' + }, + { + 'ph': 'P', + 'name': '', + 'pid': 77616, + 'tid': 42247, + 'ts': 47377800763, + 'cat': 'Dart', + 'args': {'mode': 'basic'}, + 'sf': '140357727781376-5' + }, + { + 'ph': 'P', + 'name': '', + 'pid': 77616, + 'tid': 42247, + 'ts': 47377800863, + 'cat': 'Dart', + 'args': {'mode': 'basic'}, + 'sf': '140357727781376-6' + }, + { + 'ph': 'P', + 'name': '', + 'pid': 77616, + 'tid': 42247, + 'ts': 47377800963, + 'cat': 'Dart', + 'args': {'mode': 'basic'}, + 'sf': '140357727781376-8' + }, + { + 'ph': 'P', + 'name': '', + 'pid': 77616, + 'tid': 42247, + 'ts': 47377801063, + 'cat': 'Dart', + 'args': {'mode': 'basic'}, + 'sf': '140357727781376-9' + }, +]; diff --git a/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_bottom_up.dart b/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_bottom_up.dart index 5b5c76097f6..6c29bb1123d 100644 --- a/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_bottom_up.dart +++ b/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_bottom_up.dart @@ -8,27 +8,41 @@ import '../../flutter/table.dart'; import '../../profiler/cpu_profile_columns.dart'; import '../../profiler/cpu_profile_model.dart'; import '../../table_data.dart'; +import '../../utils.dart'; /// A table of the CPU's bottom-up call tree. class CpuBottomUpTable extends StatelessWidget { factory CpuBottomUpTable(List bottomUpRoots, {Key key}) { final treeColumn = MethodNameColumn(); + final startingSortColumn = SelfTimeColumn(); final columns = List>.unmodifiable([ TotalTimeColumn(), - SelfTimeColumn(), + startingSortColumn, treeColumn, SourceColumn(), ]); - return CpuBottomUpTable._(key, bottomUpRoots, treeColumn, columns); + return CpuBottomUpTable._( + key, + bottomUpRoots, + treeColumn, + startingSortColumn, + columns, + ); } const CpuBottomUpTable._( - Key key, this.bottomUpRoots, this.treeColumn, this.columns) - : super(key: key); + Key key, + this.bottomUpRoots, + this.treeColumn, + this.startingSortColumn, + this.columns, + ) : super(key: key); final TreeColumnData treeColumn; + final ColumnData startingSortColumn; final List> columns; final List bottomUpRoots; + @override Widget build(BuildContext context) { return TreeTable( @@ -36,6 +50,8 @@ class CpuBottomUpTable extends StatelessWidget { columns: columns, treeColumn: treeColumn, keyFactory: (frame) => PageStorageKey(frame.id), + startingSortColumn: startingSortColumn, + startingSortDirection: SortDirection.descending, ); } } diff --git a/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_call_tree.dart b/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_call_tree.dart index 7a72a1bf121..06c5e517645 100644 --- a/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_call_tree.dart +++ b/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_call_tree.dart @@ -8,24 +8,38 @@ import '../../flutter/table.dart'; import '../../profiler/cpu_profile_columns.dart'; import '../../profiler/cpu_profile_model.dart'; import '../../table_data.dart'; +import '../../utils.dart'; /// A table of the CPU's top-down call tree. class CpuCallTreeTable extends StatelessWidget { factory CpuCallTreeTable(CpuProfileData data, {Key key}) { final treeColumn = MethodNameColumn(); + final startingSortColumn = TotalTimeColumn(); final columns = List>.unmodifiable([ - TotalTimeColumn(), + startingSortColumn, SelfTimeColumn(), treeColumn, SourceColumn(), ]); - return CpuCallTreeTable._(key, data, treeColumn, columns); + return CpuCallTreeTable._( + key, + data, + treeColumn, + startingSortColumn, + columns, + ); } - const CpuCallTreeTable._(Key key, this.data, this.treeColumn, this.columns) - : super(key: key); + const CpuCallTreeTable._( + Key key, + this.data, + this.treeColumn, + this.startingSortColumn, + this.columns, + ) : super(key: key); final TreeColumnData treeColumn; + final ColumnData startingSortColumn; final List> columns; final CpuProfileData data; @@ -36,6 +50,8 @@ class CpuCallTreeTable extends StatelessWidget { columns: columns, treeColumn: treeColumn, keyFactory: (frame) => PageStorageKey(frame.id), + startingSortColumn: startingSortColumn, + startingSortDirection: SortDirection.descending, ); } } diff --git a/packages/devtools_app/lib/src/profiler/flutter/cpu_profiler.dart b/packages/devtools_app/lib/src/profiler/flutter/cpu_profiler.dart index fe6910d4399..c6a769d9f86 100644 --- a/packages/devtools_app/lib/src/profiler/flutter/cpu_profiler.dart +++ b/packages/devtools_app/lib/src/profiler/flutter/cpu_profiler.dart @@ -130,7 +130,7 @@ class _CpuProfilerState extends State final cpuFlameChart = LayoutBuilder( builder: (context, constraints) { return ValueListenableBuilder( - valueListenable: widget.controller.selectedCpuStackFrameNotifier, + valueListenable: widget.controller.selectedCpuStackFrame, builder: (context, selectedStackFrame, _) { return CpuProfileFlameChart( widget.data, diff --git a/packages/devtools_app/lib/src/table_data.dart b/packages/devtools_app/lib/src/table_data.dart index c036d347b80..1e8fdb83978 100644 --- a/packages/devtools_app/lib/src/table_data.dart +++ b/packages/devtools_app/lib/src/table_data.dart @@ -27,7 +27,7 @@ abstract class TableDataClient { /// The way the columns are sorted have changed. /// /// Update the UI to reflect the new state. - void onColumnSortChanged(ColumnData column, SortOrder sortDirection); + void onColumnSortChanged(ColumnData column, SortDirection sortDirection); /// Selects by index. Note: This is index of the row as it's rendered /// and not necessarily for rows[] since it may be being rendered in reverse. @@ -65,12 +65,12 @@ class TableData extends Object { ColumnData _sortColumn; - SortOrder _sortDirection; + SortDirection _sortDirection; set sortColumn(ColumnData column) { _sortColumn = column; _sortDirection = - column.numeric ? SortOrder.descending : SortOrder.ascending; + column.numeric ? SortDirection.descending : SortDirection.ascending; } @protected @@ -163,7 +163,7 @@ class TableData extends Object { void _doSort() { final ColumnData column = _sortColumn; - final int direction = _sortDirection == SortOrder.ascending ? 1 : -1; + final int direction = _sortDirection == SortDirection.ascending ? 1 : -1; client?.onColumnSortChanged(column, _sortDirection); @@ -201,9 +201,9 @@ class TableData extends Object { } if (_sortColumn == column) { - _sortDirection = _sortDirection == SortOrder.ascending - ? SortOrder.descending - : SortOrder.ascending; + _sortDirection = _sortDirection == SortDirection.ascending + ? SortDirection.descending + : SortDirection.ascending; } else { sortColumn = column; } @@ -399,7 +399,11 @@ abstract class ColumnData { /// How much to indent the data object by. /// /// This should only be non-zero for [TreeColumnData]. - double getNodeIndentPx(T dataObject) => 0.0; + // TODO(kenz): remove `indentForTreeToggle` param once we delete the html app. + // we only added this so that we could tweak this API without breaking the + // dart:html app. + double getNodeIndentPx(T dataObject, {bool indentForTreeToggle = true}) => + 0.0; final ColumnAlignment alignment; @@ -461,10 +465,13 @@ abstract class TreeColumnData> extends ColumnData { Stream get onNodeCollapsed => nodeCollapsedController.stream; + // TODO(kenz): remove `indentForTreeToggle` param once we delete the html app. + // we only added this so that we could tweak this API without breaking the + // dart:html app. @override - double getNodeIndentPx(T dataObject) { + double getNodeIndentPx(T dataObject, {bool indentForTreeToggle = true}) { double indentWidth = dataObject.level * treeToggleWidth; - if (!dataObject.isExpandable) { + if (indentForTreeToggle && !dataObject.isExpandable) { // If the object is not expandable, we need to increase the width of our // spacer to account for the missing tree toggle. indentWidth += TreeColumnData.treeToggleWidth; @@ -478,8 +485,3 @@ enum ColumnAlignment { right, center, } - -enum SortOrder { - ascending, - descending, -} diff --git a/packages/devtools_app/lib/src/timeline/flutter/event_details.dart b/packages/devtools_app/lib/src/timeline/flutter/event_details.dart index 01853c36a9f..2535ad719d9 100644 --- a/packages/devtools_app/lib/src/timeline/flutter/event_details.dart +++ b/packages/devtools_app/lib/src/timeline/flutter/event_details.dart @@ -60,7 +60,7 @@ class EventDetails extends StatelessWidget { Widget _buildDetails(TimelineController controller) { if (selectedEvent.isUiEvent) { return ValueListenableBuilder( - valueListenable: controller.cpuProfilerController.profilerFlagNotifier, + valueListenable: controller.cpuProfilerController.profilerFlag, builder: (context, profilerFlag, _) { return profilerFlag.valueAsString == 'true' ? _buildCpuProfiler(controller.cpuProfilerController) @@ -73,7 +73,7 @@ class EventDetails extends StatelessWidget { Widget _buildCpuProfiler(CpuProfilerController cpuProfilerController) { return ValueListenableBuilder( - valueListenable: cpuProfilerController.dataNotifier, + valueListenable: cpuProfilerController.data, builder: (context, cpuProfileData, _) { if (cpuProfileData == null) { return _buildProcessingInfo(cpuProfilerController); diff --git a/packages/devtools_app/lib/src/timeline/flutter/timeline_controller.dart b/packages/devtools_app/lib/src/timeline/flutter/timeline_controller.dart index 62d04c068e3..1a9a0386e6f 100644 --- a/packages/devtools_app/lib/src/timeline/flutter/timeline_controller.dart +++ b/packages/devtools_app/lib/src/timeline/flutter/timeline_controller.dart @@ -163,7 +163,7 @@ class TimelineController implements DisposableController { startMicros: selectedEvent.time.start.inMicroseconds, extentMicros: selectedEvent.time.duration.inMicroseconds, ); - data.cpuProfileData = cpuProfilerController.dataNotifier.value; + data.cpuProfileData = cpuProfilerController.data.value; } Future get displayRefreshRate async { diff --git a/packages/devtools_app/lib/src/timeline/html_timeline_controller.dart b/packages/devtools_app/lib/src/timeline/html_timeline_controller.dart index 47ef01f4148..29d77567538 100644 --- a/packages/devtools_app/lib/src/timeline/html_timeline_controller.dart +++ b/packages/devtools_app/lib/src/timeline/html_timeline_controller.dart @@ -136,7 +136,7 @@ class TimelineController implements DisposableController { startMicros: selectedEvent.time.start.inMicroseconds, extentMicros: selectedEvent.time.duration.inMicroseconds, ); - timeline.data.cpuProfileData = cpuProfilerController.dataNotifier.value; + timeline.data.cpuProfileData = cpuProfilerController.data.value; } Future loadOfflineData(OfflineData offlineData) async { diff --git a/packages/devtools_app/lib/src/utils.dart b/packages/devtools_app/lib/src/utils.dart index 0a015de1393..1cd34fb4ad4 100644 --- a/packages/devtools_app/lib/src/utils.dart +++ b/packages/devtools_app/lib/src/utils.dart @@ -562,3 +562,16 @@ class Range { @override int get hashCode => hashValues(begin, end); } + +enum SortDirection { + ascending, + descending, +} + +extension SortDirectionExtension on SortDirection { + SortDirection opposite() { + return this == SortDirection.ascending + ? SortDirection.descending + : SortDirection.ascending; + } +} diff --git a/packages/devtools_app/test/cpu_profiler_controller_test.dart b/packages/devtools_app/test/cpu_profiler_controller_test.dart index 3d35c378d1c..9fd0c0bdfbc 100644 --- a/packages/devtools_app/test/cpu_profiler_controller_test.dart +++ b/packages/devtools_app/test/cpu_profiler_controller_test.dart @@ -24,78 +24,78 @@ void main() { await controller.pullAndProcessProfile(startMicros: 0, extentMicros: 100); controller.selectCpuStackFrame(testStackFrame); expect( - controller.dataNotifier.value, + controller.data.value, isNot(equals(CpuProfilerController.baseStateCpuProfileData)), ); expect( - controller.selectedCpuStackFrameNotifier.value, + controller.selectedCpuStackFrame.value, equals(testStackFrame), ); } test('pullAndProcessProfile', () async { expect( - controller.dataNotifier.value, + controller.data.value, equals(CpuProfilerController.baseStateCpuProfileData), ); - expect(controller.processingNotifier.value, false); + expect(controller.processing.value, false); // [startMicros] and [extentMicros] are arbitrary for testing. await controller.pullAndProcessProfile(startMicros: 0, extentMicros: 100); expect( - controller.dataNotifier.value, + controller.data.value, isNot(equals(CpuProfilerController.baseStateCpuProfileData)), ); - expect(controller.processingNotifier.value, false); + expect(controller.processing.value, false); await controller.clear(); expect( - controller.dataNotifier.value, + controller.data.value, equals(CpuProfilerController.baseStateCpuProfileData), ); }); test('selectCpuStackFrame', () async { expect( - controller.dataNotifier.value.selectedStackFrame, + controller.data.value.selectedStackFrame, isNull, ); - expect(controller.selectedCpuStackFrameNotifier.value, isNull); + expect(controller.selectedCpuStackFrame.value, isNull); controller.selectCpuStackFrame(testStackFrame); expect( - controller.dataNotifier.value.selectedStackFrame, + controller.data.value.selectedStackFrame, equals(testStackFrame), ); expect( - controller.selectedCpuStackFrameNotifier.value, + controller.selectedCpuStackFrame.value, equals(testStackFrame), ); await controller.clear(); - expect(controller.selectedCpuStackFrameNotifier.value, isNull); + expect(controller.selectedCpuStackFrame.value, isNull); }); test('reset', () async { await pullProfileAndSelectFrame(); controller.reset(); expect( - controller.dataNotifier.value, + controller.data.value, equals(CpuProfilerController.baseStateCpuProfileData), ); - expect(controller.selectedCpuStackFrameNotifier.value, isNull); - expect(controller.processingNotifier.value, isFalse); + expect(controller.selectedCpuStackFrame.value, isNull); + expect(controller.processing.value, isFalse); }); test('disposes', () { controller.dispose(); expect(() { - controller.dataNotifier.addListener(() {}); + controller.data.addListener(() {}); }, throwsA(anything)); expect(() { - controller.selectedCpuStackFrameNotifier.addListener(() {}); + controller.selectedCpuStackFrame.addListener(() {}); }, throwsA(anything)); expect(() { - controller.processingNotifier.addListener(() {}); + controller.processing.addListener(() {}); }, throwsA(anything)); }); }); diff --git a/packages/devtools_app/test/flutter/table_test.dart b/packages/devtools_app/test/flutter/table_test.dart index 649c4cb9783..824091d3a14 100644 --- a/packages/devtools_app/test/flutter/table_test.dart +++ b/packages/devtools_app/test/flutter/table_test.dart @@ -1,6 +1,7 @@ import 'package:devtools_app/src/flutter/table.dart'; import 'package:devtools_app/src/table_data.dart'; import 'package:devtools_app/src/trees.dart'; +import 'package:devtools_app/src/utils.dart'; import 'package:flutter/material.dart' hide TableRow; import 'package:flutter_test/flutter_test.dart'; @@ -9,8 +10,10 @@ import 'wrappers.dart'; void main() { group('FlatTable view', () { List flatData; + ColumnData flatNameColumn; setUp(() { + flatNameColumn = _FlatNameColumn(); flatData = [ TestData('Foo', 0), TestData('Bar', 1), @@ -26,10 +29,12 @@ void main() { testWidgets('displays with simple content', (WidgetTester tester) async { final table = FlatTable( - columns: [_FlatNameColumn()], + columns: [flatNameColumn], data: [TestData('empty', 0)], keyFactory: (d) => Key(d.name), onItemSelected: noop, + startingSortColumn: flatNameColumn, + startingSortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -37,16 +42,18 @@ void main() { }); testWidgetsWithWindowSize( - 'displays with tree column first', const Size(800.0, 1200.0), + 'displays with full content', const Size(800.0, 1200.0), (WidgetTester tester) async { final table = FlatTable( columns: [ - _FlatNameColumn(), + flatNameColumn, _NumberColumn(), ], data: flatData, onItemSelected: noop, keyFactory: (d) => Key(d.name), + startingSortColumn: flatNameColumn, + startingSortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -61,17 +68,103 @@ void main() { expect(find.byKey(const Key('Pop')), findsOneWidget); }); + testWidgets('starts with sorted data', (WidgetTester tester) async { + expect(flatData[0].name, equals('Foo')); + expect(flatData[1].name, equals('Bar')); + expect(flatData[2].name, equals('Baz')); + expect(flatData[3].name, equals('Qux')); + expect(flatData[4].name, equals('Snap')); + expect(flatData[5].name, equals('Crackle')); + expect(flatData[6].name, equals('Pop')); + expect(flatData[7].name, equals('Baz')); + expect(flatData[8].name, equals('Qux')); + final table = FlatTable( + columns: [ + flatNameColumn, + _NumberColumn(), + ], + data: flatData, + onItemSelected: noop, + keyFactory: (d) => Key(d.name), + startingSortColumn: flatNameColumn, + startingSortDirection: SortDirection.ascending, + ); + await tester.pumpWidget(wrap(table)); + expect(flatData[0].name, equals('Bar')); + expect(flatData[1].name, equals('Baz')); + expect(flatData[2].name, equals('Baz')); + expect(flatData[3].name, equals('Crackle')); + expect(flatData[4].name, equals('Foo')); + expect(flatData[5].name, equals('Pop')); + expect(flatData[6].name, equals('Qux')); + expect(flatData[7].name, equals('Qux')); + expect(flatData[8].name, equals('Snap')); + }); + + testWidgets('sorts data by column', (WidgetTester tester) async { + final table = FlatTable( + columns: [ + flatNameColumn, + _NumberColumn(), + ], + data: flatData, + onItemSelected: noop, + keyFactory: (d) => Key(d.name), + startingSortColumn: flatNameColumn, + startingSortDirection: SortDirection.ascending, + ); + await tester.pumpWidget(wrap(table)); + expect(flatData[0].name, equals('Bar')); + expect(flatData[1].name, equals('Baz')); + expect(flatData[2].name, equals('Baz')); + expect(flatData[3].name, equals('Crackle')); + expect(flatData[4].name, equals('Foo')); + expect(flatData[5].name, equals('Pop')); + expect(flatData[6].name, equals('Qux')); + expect(flatData[7].name, equals('Qux')); + expect(flatData[8].name, equals('Snap')); + + // Reverse the sort direction. + await tester.tap(find.text('FlatName')); + await tester.pumpAndSettle(); + + expect(flatData[8].name, equals('Bar')); + expect(flatData[7].name, equals('Baz')); + expect(flatData[6].name, equals('Baz')); + expect(flatData[5].name, equals('Crackle')); + expect(flatData[4].name, equals('Foo')); + expect(flatData[3].name, equals('Pop')); + expect(flatData[2].name, equals('Qux')); + expect(flatData[1].name, equals('Qux')); + expect(flatData[0].name, equals('Snap')); + + // Change the sort column. + await tester.tap(find.text('Number')); + await tester.pumpAndSettle(); + expect(flatData[0].name, equals('Foo')); + expect(flatData[1].name, equals('Bar')); + expect(flatData[2].name, equals('Baz')); + expect(flatData[3].name, equals('Qux')); + expect(flatData[4].name, equals('Snap')); + expect(flatData[5].name, equals('Pop')); + expect(flatData[6].name, equals('Crackle')); + expect(flatData[7].name, equals('Baz')); + expect(flatData[8].name, equals('Qux')); + }); + testWidgets('displays with many columns', (WidgetTester tester) async { final table = FlatTable( columns: [ _NumberColumn(), _CombinedColumn(), - _FlatNameColumn(), + flatNameColumn, _CombinedColumn(), ], data: flatData, onItemSelected: noop, keyFactory: (data) => Key(data.name), + startingSortColumn: flatNameColumn, + startingSortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap( SizedBox( @@ -88,10 +181,12 @@ void main() { final testData = TestData('empty', 0); const key = Key('empty'); final table = FlatTable( - columns: [_FlatNameColumn()], + columns: [flatNameColumn], data: [testData], keyFactory: (d) => Key(d.name), onItemSelected: (item) => selected = item, + startingSortColumn: flatNameColumn, + startingSortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -105,10 +200,12 @@ void main() { expect( () { FlatTable( - columns: [_FlatNameColumn()], + columns: [flatNameColumn], data: null, keyFactory: (d) => Key(d.name), onItemSelected: noop, + startingSortColumn: flatNameColumn, + startingSortDirection: SortDirection.ascending, ); }, throwsAssertionError, @@ -118,10 +215,12 @@ void main() { test('fails when a TreeNode cannot provide a key', () { expect(() { FlatTable( - columns: [_FlatNameColumn()], + columns: [flatNameColumn], data: flatData, keyFactory: null, onItemSelected: noop, + startingSortColumn: flatNameColumn, + startingSortDirection: SortDirection.ascending, ); }, throwsAssertionError); }); @@ -134,6 +233,7 @@ void main() { setUp(() { treeColumn = _NameColumn(); + _NumberColumn(); tree1 = TestData('Foo', 0) ..children.addAll([ TestData('Bar', 1) @@ -144,8 +244,8 @@ void main() { TestData('Crackle', 5), TestData('Pop', 5), ]), - TestData('Baz', 6), - TestData('Qux', 7), + TestData('Baz', 7), + TestData('Qux', 6), ]) ..expandCascading(); tree2 = TestData('Foo_2', 0) @@ -164,6 +264,8 @@ void main() { dataRoots: [TestData('empty', 0)], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -177,6 +279,8 @@ void main() { dataRoots: [tree1, tree2], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -210,6 +314,8 @@ void main() { dataRoots: [tree1, tree2], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -227,6 +333,8 @@ void main() { dataRoots: [TestData('root1', 0), TestData('root2', 1)], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.descending, ); await tester.pumpWidget(wrap(newTable)); expect(find.byKey(const Key('Foo')), findsNothing); @@ -250,6 +358,8 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.descending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -275,6 +385,8 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap( SizedBox( @@ -296,6 +408,8 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); await tester.pumpAndSettle(); @@ -311,6 +425,84 @@ void main() { expect(tree1.children[0].isExpanded, false); }); + testWidgets('starts with sorted data', (WidgetTester tester) async { + expect(tree1.children[0].name, equals('Bar')); + expect(tree1.children[0].children[0].name, equals('Baz')); + expect(tree1.children[0].children[1].name, equals('Qux')); + expect(tree1.children[0].children[2].name, equals('Snap')); + expect(tree1.children[0].children[3].name, equals('Crackle')); + expect(tree1.children[0].children[4].name, equals('Pop')); + expect(tree1.children[1].name, equals('Baz')); + expect(tree1.children[2].name, equals('Qux')); + final table = TreeTable( + columns: [ + _NumberColumn(), + treeColumn, + ], + dataRoots: [tree1], + treeColumn: treeColumn, + keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, + ); + await tester.pumpWidget(wrap(table)); + expect(tree1.children[0].name, equals('Bar')); + expect(tree1.children[0].children[0].name, equals('Baz')); + expect(tree1.children[0].children[1].name, equals('Crackle')); + expect(tree1.children[0].children[2].name, equals('Pop')); + expect(tree1.children[0].children[3].name, equals('Qux')); + expect(tree1.children[0].children[4].name, equals('Snap')); + expect(tree1.children[1].name, equals('Baz')); + expect(tree1.children[2].name, equals('Qux')); + }); + + testWidgets('sorts data by column', (WidgetTester tester) async { + final table = TreeTable( + columns: [ + _NumberColumn(), + treeColumn, + ], + dataRoots: [tree1], + treeColumn: treeColumn, + keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, + ); + await tester.pumpWidget(wrap(table)); + expect(tree1.children[0].name, equals('Bar')); + expect(tree1.children[0].children[0].name, equals('Baz')); + expect(tree1.children[0].children[1].name, equals('Crackle')); + expect(tree1.children[0].children[2].name, equals('Pop')); + expect(tree1.children[0].children[3].name, equals('Qux')); + expect(tree1.children[0].children[4].name, equals('Snap')); + expect(tree1.children[1].name, equals('Baz')); + expect(tree1.children[2].name, equals('Qux')); + + // Reverse the sort direction. + await tester.tap(find.text('Name')); + await tester.pumpAndSettle(); + expect(tree1.children[2].name, equals('Bar')); + expect(tree1.children[2].children[4].name, equals('Baz')); + expect(tree1.children[2].children[3].name, equals('Crackle')); + expect(tree1.children[2].children[2].name, equals('Pop')); + expect(tree1.children[2].children[1].name, equals('Qux')); + expect(tree1.children[2].children[0].name, equals('Snap')); + expect(tree1.children[1].name, equals('Baz')); + expect(tree1.children[0].name, equals('Qux')); + + // Change the sort column. + await tester.tap(find.text('Number')); + await tester.pumpAndSettle(); + expect(tree1.children[0].name, equals('Bar')); + expect(tree1.children[0].children[0].name, equals('Baz')); + expect(tree1.children[0].children[1].name, equals('Qux')); + expect(tree1.children[0].children[2].name, equals('Snap')); + expect(tree1.children[0].children[3].name, equals('Pop')); + expect(tree1.children[0].children[4].name, equals('Crackle')); + expect(tree1.children[1].name, equals('Qux')); + expect(tree1.children[2].name, equals('Baz')); + }); + testWidgets('properly colors rows with alternating colors', (WidgetTester tester) async { final data = TestData('Foo', 0) @@ -332,6 +524,8 @@ void main() { dataRoots: [data], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); final fooFinder = find.byKey(const Key('Foo')); @@ -396,6 +590,8 @@ void main() { dataRoots: null, treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); }, throwsAssertionError, @@ -409,6 +605,8 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: null, + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); }, throwsAssertionError); }); @@ -420,6 +618,8 @@ void main() { dataRoots: [tree1], treeColumn: null, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); }, throwsAssertionError); @@ -429,6 +629,8 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), + startingSortColumn: treeColumn, + startingSortDirection: SortDirection.ascending, ); }, throwsAssertionError); }); @@ -439,6 +641,9 @@ class TestData extends TreeNode { TestData(this.name, this.number); final String name; final int number; + + @override + String toString() => '$name - $number'; } class _NameColumn extends TreeColumnData { @@ -446,26 +651,35 @@ class _NameColumn extends TreeColumnData { @override String getValue(TestData dataObject) => dataObject.name; + + @override + bool get supportsSorting => true; } class _NumberColumn extends ColumnData { - _NumberColumn() : super('Name'); + _NumberColumn() : super('Number'); @override - String getValue(TestData dataObject) => 'dataObject.number'; + int getValue(TestData dataObject) => dataObject.number; @override double get fixedWidthPx => 400.0; + + @override + bool get supportsSorting => true; } class _FlatNameColumn extends ColumnData { - _FlatNameColumn() : super('Name'); + _FlatNameColumn() : super('FlatName'); @override String getValue(TestData dataObject) => dataObject.name; @override double get fixedWidthPx => 300.0; + + @override + bool get supportsSorting => true; } class _CombinedColumn extends ColumnData { diff --git a/packages/devtools_app/test/performance_controller_test.dart b/packages/devtools_app/test/performance_controller_test.dart index a787505f941..41121b646aa 100644 --- a/packages/devtools_app/test/performance_controller_test.dart +++ b/packages/devtools_app/test/performance_controller_test.dart @@ -34,11 +34,11 @@ void main() { }, throwsA(anything)); expect(() { - controller.cpuProfilerController.dataNotifier.addListener(() {}); + controller.cpuProfilerController.data.addListener(() {}); }, throwsA(anything)); expect(() { - controller.cpuProfilerController.selectedCpuStackFrameNotifier + controller.cpuProfilerController.selectedCpuStackFrame .addListener(() {}); }, throwsA(anything)); }); diff --git a/packages/devtools_testing/lib/support/flutter_test_environment.dart b/packages/devtools_testing/lib/support/flutter_test_environment.dart index 9aaf1ce786a..c2bcb3807bf 100644 --- a/packages/devtools_testing/lib/support/flutter_test_environment.dart +++ b/packages/devtools_testing/lib/support/flutter_test_environment.dart @@ -60,12 +60,14 @@ class FlutterTestEnvironment { // test, but it will also run as part of a final forced tear down so should // be tolerable to being called twice after a single test. Future Function() _beforeEveryTearDown; - set beforeEveryTearDown(Future Function() f) => _beforeEveryTearDown = f; + set beforeEveryTearDown(Future Function() f) => + _beforeEveryTearDown = f; // The function will be called before the final forced teardown at the end // of the test suite (which will then stop the Flutter app). Future Function() _beforeFinalTearDown; - set beforeFinalTearDown(Future Function() f) => _beforeFinalTearDown = f; + set beforeFinalTearDown(Future Function() f) => + _beforeFinalTearDown = f; bool _needsSetup = true; From da46ecebdcbc65f6460d111bf8a69c9bd3cb954f Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Mon, 16 Mar 2020 14:31:57 -0700 Subject: [PATCH 2/4] Review comments. --- .../lib/src/flutter/common_widgets.dart | 9 +- .../devtools_app/lib/src/flutter/screen.dart | 3 +- .../devtools_app/lib/src/flutter/table.dart | 139 ++++++----- .../devtools_app/lib/src/flutter/theme.dart | 2 + .../lib/src/info/flutter/info_screen.dart | 3 +- .../flutter/inspector_tree_flutter.dart | 5 +- .../flutter/layout_explorer/flex/arrow.dart | 11 +- .../flutter/layout_explorer/flex/flex.dart | 2 +- .../src/logging/flutter/logging_screen.dart | 4 +- .../lib/src/memory/flutter/memory_screen.dart | 9 +- .../flutter/performance_screen.dart | 9 +- .../performance/performance_controller.dart | 2 +- .../src/profiler/cpu_profile_controller.dart | 160 +----------- .../flutter/cpu_profile_bottom_up.dart | 8 +- .../flutter/cpu_profile_call_tree.dart | 8 +- .../src/profiler/flutter/cpu_profiler.dart | 2 +- packages/devtools_app/lib/src/table_data.dart | 4 +- .../src/timeline/flutter/event_details.dart | 4 +- .../timeline/flutter/timeline_controller.dart | 2 +- .../timeline/html_timeline_controller.dart | 2 +- .../ui/flutter/service_extension_widgets.dart | 5 +- packages/devtools_app/lib/src/utils.dart | 2 +- .../test/cpu_profiler_controller_test.dart | 36 +-- .../devtools_app/test/flutter/table_test.dart | 232 +++++++++--------- .../test/performance_controller_test.dart | 4 +- 25 files changed, 281 insertions(+), 386 deletions(-) diff --git a/packages/devtools_app/lib/src/flutter/common_widgets.dart b/packages/devtools_app/lib/src/flutter/common_widgets.dart index 372302c7e1b..0221984bc42 100644 --- a/packages/devtools_app/lib/src/flutter/common_widgets.dart +++ b/packages/devtools_app/lib/src/flutter/common_widgets.dart @@ -10,6 +10,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/scheduler.dart'; import '../ui/flutter/label.dart'; +import 'theme.dart'; const tooltipWait = Duration(milliseconds: 500); @@ -232,7 +233,7 @@ Widget _recordingStatus({Key key, String recordedObject}) { mainAxisAlignment: MainAxisAlignment.center, children: [ Text('Recording $recordedObject'), - const SizedBox(height: 16.0), + const SizedBox(height: defaultSpacing), const CircularProgressIndicator(), ], ); @@ -249,10 +250,10 @@ Widget processingInfo({ mainAxisAlignment: MainAxisAlignment.center, children: [ Text('Processing $processedObject'), - const SizedBox(height: 16.0), + const SizedBox(height: defaultSpacing), SizedBox( width: 200.0, - height: 16.0, + height: defaultSpacing, child: LinearProgressIndicator( value: progressValue, ), @@ -321,7 +322,7 @@ class ToggleButton extends StatelessWidget { waitDuration: tooltipWait, preferBelow: false, child: Padding( - padding: const EdgeInsets.symmetric(horizontal: 16.0), + padding: const EdgeInsets.symmetric(horizontal: defaultSpacing), child: MaterialIconLabel( icon, text, diff --git a/packages/devtools_app/lib/src/flutter/screen.dart b/packages/devtools_app/lib/src/flutter/screen.dart index 1fb19411a2c..c159e7f6ca0 100644 --- a/packages/devtools_app/lib/src/flutter/screen.dart +++ b/packages/devtools_app/lib/src/flutter/screen.dart @@ -14,6 +14,7 @@ import '../performance/flutter/performance_screen.dart'; import '../timeline/flutter/timeline_screen.dart'; import 'connect_screen.dart'; import 'scaffold.dart'; +import 'theme.dart'; /// Defines pages shown in the tabbar of the app. @immutable @@ -41,7 +42,7 @@ abstract class Screen { key: tabKey, child: Row( children: [ - Icon(icon, size: 16.0), + Icon(icon, size: defaultIconSize), Padding( padding: const EdgeInsets.only(left: 8.0), child: Text(title), diff --git a/packages/devtools_app/lib/src/flutter/table.dart b/packages/devtools_app/lib/src/flutter/table.dart index 659116e0fc5..ec76b1925e4 100644 --- a/packages/devtools_app/lib/src/flutter/table.dart +++ b/packages/devtools_app/lib/src/flutter/table.dart @@ -28,8 +28,8 @@ class FlatTable extends StatefulWidget { @required this.data, @required this.keyFactory, @required this.onItemSelected, - @required this.startingSortColumn, - @required this.startingSortDirection, + @required this.sortColumn, + @required this.sortDirection, }) : assert(columns != null), assert(keyFactory != null), assert(data != null), @@ -45,31 +45,43 @@ class FlatTable extends StatefulWidget { final ItemCallback onItemSelected; - final ColumnData startingSortColumn; + final ColumnData sortColumn; - final SortDirection startingSortDirection; + final SortDirection sortDirection; @override - _FlatTableState createState() => _FlatTableState(); + FlatTableState createState() => FlatTableState(); } -class _FlatTableState extends State> with TableSortMixin { +class FlatTableState extends State> + implements SortableTable { List columnWidths; + List data; + @override void initState() { super.initState(); - sortData(widget.startingSortColumn, widget.startingSortDirection); + _initData(); columnWidths = _computeColumnWidths(); } @override void didUpdateWidget(FlatTable oldWidget) { super.didUpdateWidget(oldWidget); - sortData(widget.startingSortColumn, widget.startingSortDirection); + if (widget.sortColumn != oldWidget.sortColumn || + widget.sortDirection != oldWidget.sortDirection || + !collectionEquals(widget.data, oldWidget.data)) { + _initData(); + } columnWidths = _computeColumnWidths(); } + void _initData() { + data = List.from(widget.data); + sortData(widget.sortColumn, widget.sortDirection); + } + List _computeColumnWidths() { final widths = []; for (ColumnData column in widget.columns) { @@ -92,8 +104,8 @@ class _FlatTableState extends State> with TableSortMixin { columnWidths: columnWidths, startAtBottom: true, rowBuilder: _buildRow, - startingSortColumn: widget.startingSortColumn, - startingSortDirection: widget.startingSortDirection, + sortColumn: widget.sortColumn, + sortDirection: widget.sortDirection, onSortChanged: _sortDataAndUpdate, ); } @@ -123,7 +135,7 @@ class _FlatTableState extends State> with TableSortMixin { @override void sortData(ColumnData column, SortDirection direction) { - widget.data.sort((T a, T b) => _compareData(a, b, column, direction)); + data.sort((T a, T b) => _compareData(a, b, column, direction)); } } @@ -137,7 +149,7 @@ class _FlatTableState extends State> with TableSortMixin { /// The [ColumnData] gives this table information about how to size its /// columns, how to present each row of `data`, and which rows to show. /// -/// To lay the contents out, this table's [_TreeTableState] creates a flattened +/// To lay the contents out, this table's [TreeTableState] creates a flattened /// list of the tree hierarchy. It then uses the nesting level of the /// deepest row in [dataRoots] to determine how wide to make the [treeColumn]. /// @@ -151,10 +163,10 @@ class TreeTable> extends StatefulWidget { @required this.treeColumn, @required this.dataRoots, @required this.keyFactory, - @required this.startingSortColumn, - @required this.startingSortDirection, + @required this.sortColumn, + @required this.sortDirection, }) : assert(columns.contains(treeColumn)), - assert(columns.contains(startingSortColumn)), + assert(columns.contains(sortColumn)), assert(columns != null), assert(keyFactory != null), assert(dataRoots != null), @@ -172,19 +184,22 @@ class TreeTable> extends StatefulWidget { /// Factory that creates keys for each row in this table. final Key Function(T) keyFactory; - final ColumnData startingSortColumn; + final ColumnData sortColumn; - final SortDirection startingSortDirection; + final SortDirection sortDirection; @override - _TreeTableState createState() => _TreeTableState(); + TreeTableState createState() => TreeTableState(); } -class _TreeTableState> extends State> - with TickerProviderStateMixin, TableSortMixin { +class TreeTableState> extends State> + with TickerProviderStateMixin + implements SortableTable { /// The number of items to show when animating out the tree table. static const itemsToShowWhenAnimating = 50; + List dataRoots; + List items; List animatingChildren = []; Set animatingChildrenSet = {}; @@ -195,21 +210,30 @@ class _TreeTableState> extends State> @override void initState() { super.initState(); - sortData(widget.startingSortColumn, widget.startingSortDirection); - rootsExpanded = List.generate( - widget.dataRoots.length, (index) => widget.dataRoots[index].isExpanded); + _initData(); + rootsExpanded = + List.generate(dataRoots.length, (index) => dataRoots[index].isExpanded); _updateItems(); } @override void didUpdateWidget(TreeTable oldWidget) { super.didUpdateWidget(oldWidget); - sortData(widget.startingSortColumn, widget.startingSortDirection); - rootsExpanded = List.generate( - widget.dataRoots.length, (index) => widget.dataRoots[index].isExpanded); + if (widget.sortColumn != oldWidget.sortColumn || + widget.sortDirection != oldWidget.sortDirection || + !collectionEquals(widget.dataRoots, oldWidget.dataRoots)) { + _initData(); + } + rootsExpanded = + List.generate(dataRoots.length, (index) => dataRoots[index].isExpanded); _updateItems(); } + void _initData() { + dataRoots = List.from(widget.dataRoots); + sortData(widget.sortColumn, widget.sortDirection); + } + @override void dispose() { super.dispose(); @@ -217,7 +241,7 @@ class _TreeTableState> extends State> void _updateItems() { setState(() { - items = _buildFlatList(widget.dataRoots); + items = _buildFlatList(dataRoots); // Leave enough space for the animating children during the animation. columnWidths = _computeColumnWidths([...items, ...animatingChildren]); }); @@ -255,8 +279,8 @@ class _TreeTableState> extends State> animatingChildrenSet = Set.of(animatingChildren); // Update the tracked expansion of the root node if needed. - if (widget.dataRoots.contains(node)) { - final rootIndex = widget.dataRoots.indexOf(node); + if (dataRoots.contains(node)) { + final rootIndex = dataRoots.indexOf(node); rootsExpanded[rootIndex] = node.isExpanded; } }); @@ -264,7 +288,7 @@ class _TreeTableState> extends State> } List _computeColumnWidths(List flattenedList) { - final firstRoot = widget.dataRoots.first; + final firstRoot = dataRoots.first; TreeNode deepest = firstRoot; // This will use the width of all rows in the table, even the rows // that are hidden by nesting. @@ -324,8 +348,8 @@ class _TreeTableState> extends State> itemCount: items.length, columnWidths: columnWidths, rowBuilder: _buildRow, - startingSortColumn: widget.startingSortColumn, - startingSortDirection: widget.startingSortDirection, + sortColumn: widget.sortColumn, + sortDirection: widget.sortDirection, onSortChanged: _sortDataAndUpdate, ); } @@ -367,12 +391,12 @@ class _TreeTableState> extends State> void sortData(ColumnData column, SortDirection direction) { void _sort(T dataObject) { dataObject.children - ..sort((T a, T b) => _compareData(a, b, column, direction)) + ..sort((T a, T b) => _compareData(a, b, column, direction)) ..forEach(_sort); } - widget.dataRoots.where((dataRoot) => dataRoot.level == 0).toList() - ..sort((T a, T b) => _compareData(a, b, column, direction)) + dataRoots.where((dataRoot) => dataRoot.level == 0).toList() + ..sort((T a, T b) => _compareData(a, b, column, direction)) ..forEach(_sort); } @@ -389,8 +413,8 @@ class _Table extends StatefulWidget { @required this.columns, @required this.columnWidths, @required this.rowBuilder, - @required this.startingSortColumn, - @required this.startingSortDirection, + @required this.sortColumn, + @required this.sortDirection, @required this.onSortChanged, this.startAtBottom = false}) : super(key: key); @@ -400,8 +424,8 @@ class _Table extends StatefulWidget { final List> columns; final List columnWidths; final IndexedScrollableWidgetBuilder rowBuilder; - final ColumnData startingSortColumn; - final SortDirection startingSortDirection; + final ColumnData sortColumn; + final SortDirection sortDirection; final Function(ColumnData column, SortDirection direction) onSortChanged; /// The width to assume for columns that don't specify a width. @@ -413,10 +437,6 @@ class _Table extends StatefulWidget { /// a height of 0 and a height of [defaultRowHeight]. static const defaultRowHeight = 42.0; - /// How much padding to place around the beginning - /// and end of each row in the table. - static const rowHorizontalPadding = 16.0; - @override _TableState createState() => _TableState(); } @@ -430,14 +450,13 @@ class _TableState extends State<_Table> { void initState() { super.initState(); _linkedHorizontalScrollControllerGroup = LinkedScrollControllerGroup(); - sortColumn = widget.startingSortColumn; - sortDirection = widget.startingSortDirection; + sortColumn = widget.sortColumn; + sortDirection = widget.sortDirection; } /// The width of all columns in the table, with additional padding. double get tableWidth => - widget.columnWidths.reduce((x, y) => x + y) + - (2 * _Table.rowHorizontalPadding); + widget.columnWidths.reduce((x, y) => x + y) + (2 * defaultSpacing); Widget _buildItem(BuildContext context, int index) { if (widget.startAtBottom) { @@ -714,7 +733,7 @@ class _TableRowState extends State> widget.sortDirection == SortDirection.ascending ? Icons.expand_less : Icons.expand_more, - size: 16.0, + size: defaultIconSize, ), const SizedBox(width: 4.0), Text( @@ -737,10 +756,10 @@ class _TableRowState extends State> turns: expandArrowAnimation, child: const Icon( Icons.expand_more, - size: 16.0, + size: defaultIconSize, ), ) - : const SizedBox(width: 16.0, height: 16.0); + : const SizedBox(width: defaultIconSize, height: defaultIconSize); content = Row( mainAxisSize: MainAxisSize.min, children: [ @@ -768,9 +787,7 @@ class _TableRowState extends State> } return Padding( - padding: const EdgeInsets.symmetric( - horizontal: _Table.rowHorizontalPadding, - ), + padding: const EdgeInsets.symmetric(horizontal: defaultSpacing), child: ListView.builder( scrollDirection: Axis.horizontal, controller: scrollController, @@ -795,7 +812,7 @@ class _TableRowState extends State> void _handleSortChange(ColumnData columnData) { SortDirection direction; if (columnData.title == widget.sortColumn.title) { - direction = widget.sortDirection.opposite(); + direction = widget.sortDirection.reverse(); } else if (columnData.numeric) { direction = SortDirection.descending; } else { @@ -805,13 +822,13 @@ class _TableRowState extends State> } } -mixin TableSortMixin { - int _compareFactor(SortDirection direction) => - direction == SortDirection.ascending ? 1 : -1; +abstract class SortableTable { + void sortData(ColumnData column, SortDirection direction); +} - int _compareData(T a, T b, ColumnData column, SortDirection direction) { - return column.compare(a, b) * _compareFactor(direction); - } +int _compareFactor(SortDirection direction) => + direction == SortDirection.ascending ? 1 : -1; - void sortData(ColumnData column, SortDirection direction); +int _compareData(T a, T b, ColumnData column, SortDirection direction) { + return column.compare(a, b) * _compareFactor(direction); } diff --git a/packages/devtools_app/lib/src/flutter/theme.dart b/packages/devtools_app/lib/src/flutter/theme.dart index 405f7f92e94..d363333db45 100644 --- a/packages/devtools_app/lib/src/flutter/theme.dart +++ b/packages/devtools_app/lib/src/flutter/theme.dart @@ -39,6 +39,8 @@ ThemeData _lightTheme() { } const buttonMinWidth = 36.0; +const defaultIconSize = 16.0; +const defaultSpacing = 16.0; /// Branded grey color. /// diff --git a/packages/devtools_app/lib/src/info/flutter/info_screen.dart b/packages/devtools_app/lib/src/info/flutter/info_screen.dart index a01040435bd..de925cf433c 100644 --- a/packages/devtools_app/lib/src/info/flutter/info_screen.dart +++ b/packages/devtools_app/lib/src/info/flutter/info_screen.dart @@ -9,6 +9,7 @@ import '../../../devtools.dart' as devtools; import '../../flutter/common_widgets.dart'; import '../../flutter/octicons.dart'; import '../../flutter/screen.dart'; +import '../../flutter/theme.dart'; import '../../version.dart'; import '../info_controller.dart'; @@ -76,7 +77,7 @@ class _InfoScreenBodyState extends State { const PaddedDivider(padding: EdgeInsets.only(top: 4.0, bottom: 0.0)), if (_flutterVersion != null) _VersionInformation(_flutterVersion), - const Padding(padding: EdgeInsets.only(top: 16.0)), + const Padding(padding: EdgeInsets.only(top: defaultSpacing)), // TODO(devoncarew): Move this information into an advanced page. Text( 'Dart VM Flag List', diff --git a/packages/devtools_app/lib/src/inspector/flutter/inspector_tree_flutter.dart b/packages/devtools_app/lib/src/inspector/flutter/inspector_tree_flutter.dart index e46b5c196ab..7fefc6bd96c 100644 --- a/packages/devtools_app/lib/src/inspector/flutter/inspector_tree_flutter.dart +++ b/packages/devtools_app/lib/src/inspector/flutter/inspector_tree_flutter.dart @@ -497,11 +497,12 @@ class InspectorRowContent extends StatelessWidget { turns: expandArrowAnimation, child: const Icon( Icons.expand_more, - size: 16.0, + size: defaultIconSize, ), ), ) - : const SizedBox(width: 16.0, height: 16.0), + : const SizedBox( + width: defaultSpacing, height: defaultSpacing), DecoratedBox( decoration: BoxDecoration( color: backgroundColor, diff --git a/packages/devtools_app/lib/src/inspector/flutter/layout_explorer/flex/arrow.dart b/packages/devtools_app/lib/src/inspector/flutter/layout_explorer/flex/arrow.dart index 38122643c2d..a97d26b80f3 100644 --- a/packages/devtools_app/lib/src/inspector/flutter/layout_explorer/flex/arrow.dart +++ b/packages/devtools_app/lib/src/inspector/flutter/layout_explorer/flex/arrow.dart @@ -8,8 +8,9 @@ import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; +import '../../../../flutter/theme.dart'; + const defaultArrowColor = Colors.white; -const defaultArrowHeadSize = 16.0; const defaultArrowStrokeWidth = 2.0; const defaultDistanceToArrow = 4.0; @@ -34,7 +35,7 @@ class ArrowWrapper extends StatelessWidget { this.child, @required ArrowType type, this.arrowColor = defaultArrowColor, - this.arrowHeadSize = defaultArrowHeadSize, + this.arrowHeadSize = defaultIconSize, this.arrowStrokeWidth = defaultArrowStrokeWidth, this.childMarginFromArrow = defaultDistanceToArrow, }) : assert(type != null), @@ -53,7 +54,7 @@ class ArrowWrapper extends StatelessWidget { this.child, @required this.direction, this.arrowColor = defaultArrowColor, - this.arrowHeadSize = defaultArrowHeadSize, + this.arrowHeadSize = defaultIconSize, this.arrowStrokeWidth = defaultArrowStrokeWidth, this.childMarginFromArrow = defaultDistanceToArrow, }) : assert(direction != null), @@ -144,7 +145,7 @@ class ArrowWrapper extends StatelessWidget { class ArrowWidget extends StatelessWidget { ArrowWidget({ this.color = defaultArrowColor, - this.headSize = defaultArrowHeadSize, + this.headSize = defaultIconSize, Key key, this.shouldDrawHead = true, this.strokeWidth = defaultArrowStrokeWidth, @@ -187,7 +188,7 @@ class ArrowWidget extends StatelessWidget { class _ArrowPainter extends CustomPainter { _ArrowPainter({ - this.headSize = defaultArrowHeadSize, + this.headSize = defaultIconSize, this.strokeWidth = defaultArrowStrokeWidth, this.color = defaultArrowColor, this.shouldDrawHead = true, diff --git a/packages/devtools_app/lib/src/inspector/flutter/layout_explorer/flex/flex.dart b/packages/devtools_app/lib/src/inspector/flutter/layout_explorer/flex/flex.dart index a171acb3e03..c5a73f9e93f 100644 --- a/packages/devtools_app/lib/src/inspector/flutter/layout_explorer/flex/flex.dart +++ b/packages/devtools_app/lib/src/inspector/flutter/layout_explorer/flex/flex.dart @@ -152,7 +152,7 @@ Widget dimensionDescription(TextSpan description, bool overflow) { Widget _visualizeWidthAndHeightWithConstraints({ @required Widget widget, @required LayoutProperties properties, - double arrowHeadSize = defaultArrowHeadSize, + double arrowHeadSize = defaultIconSize, }) { final showChildrenWidthsSum = properties is FlexLayoutProperties && properties.isOverflowWidth; diff --git a/packages/devtools_app/lib/src/logging/flutter/logging_screen.dart b/packages/devtools_app/lib/src/logging/flutter/logging_screen.dart index 46bbdd91b5b..da6ceee73d9 100644 --- a/packages/devtools_app/lib/src/logging/flutter/logging_screen.dart +++ b/packages/devtools_app/lib/src/logging/flutter/logging_screen.dart @@ -108,8 +108,8 @@ class LogsTable extends StatelessWidget { data: data, keyFactory: (LogData data) => ValueKey(data), onItemSelected: onItemSelected, - startingSortColumn: when, - startingSortDirection: SortDirection.ascending, + sortColumn: when, + sortDirection: SortDirection.ascending, ); } } diff --git a/packages/devtools_app/lib/src/memory/flutter/memory_screen.dart b/packages/devtools_app/lib/src/memory/flutter/memory_screen.dart index e51f3c9fdb7..a643522ce7a 100644 --- a/packages/devtools_app/lib/src/memory/flutter/memory_screen.dart +++ b/packages/devtools_app/lib/src/memory/flutter/memory_screen.dart @@ -8,6 +8,7 @@ import '../../flutter/controllers.dart'; import '../../flutter/octicons.dart'; import '../../flutter/screen.dart'; import '../../flutter/split.dart'; +import '../../flutter/theme.dart'; import '../../globals.dart'; import '../../ui/flutter/label.dart'; import '../../ui/material_icons.dart'; @@ -253,7 +254,7 @@ class MemoryBodyState extends State { minIncludeTextWidth: _primaryControlsMinVerboseWidth, ), ), - const SizedBox(width: 16.0), + const SizedBox(width: defaultSpacing), OutlineButton( key: MemoryScreen.clearButtonKey, // TODO(terry): Button needs to be Delete for offline data. @@ -265,7 +266,7 @@ class MemoryBodyState extends State { 'Clear', minIncludeTextWidth: _primaryControlsMinVerboseWidth, )), - const SizedBox(width: 16.0), + const SizedBox(width: defaultSpacing), _intervalDropdown(), ], ), @@ -283,7 +284,7 @@ class MemoryBodyState extends State { mainAxisAlignment: MainAxisAlignment.end, children: [ _memorySourceDropdown(), - const SizedBox(width: 16.0), + const SizedBox(width: defaultSpacing), Flexible( child: OutlineButton( key: MemoryScreen.snapshotButtonKey, @@ -317,7 +318,7 @@ class MemoryBodyState extends State { ), ), ), - const SizedBox(width: 16.0), + const SizedBox(width: defaultSpacing), Flexible( child: OutlineButton( key: MemoryScreen.exportButtonKey, diff --git a/packages/devtools_app/lib/src/performance/flutter/performance_screen.dart b/packages/devtools_app/lib/src/performance/flutter/performance_screen.dart index 599aafd01d6..4544d57dfd1 100644 --- a/packages/devtools_app/lib/src/performance/flutter/performance_screen.dart +++ b/packages/devtools_app/lib/src/performance/flutter/performance_screen.dart @@ -64,9 +64,10 @@ class _PerformanceScreenBodyState extends State recording = controller.recordingNotifier.value; }); }); - addAutoDisposeListener(controller.cpuProfilerController.processing, () { + addAutoDisposeListener(controller.cpuProfilerController.processingNotifier, + () { setState(() { - processing = controller.cpuProfilerController.processing.value; + processing = controller.cpuProfilerController.processingNotifier.value; }); }); addAutoDisposeListener( @@ -81,7 +82,7 @@ class _PerformanceScreenBodyState extends State @override Widget build(BuildContext context) { return ValueListenableBuilder( - valueListenable: controller.cpuProfilerController.profilerFlag, + valueListenable: controller.cpuProfilerController.profilerFlagNotifier, builder: (context, profilerFlag, _) { return profilerFlag.valueAsString == 'true' ? _buildPerformanceBody(controller) @@ -102,7 +103,7 @@ class _PerformanceScreenBodyState extends State ), Expanded( child: ValueListenableBuilder( - valueListenable: controller.cpuProfilerController.data, + valueListenable: controller.cpuProfilerController.dataNotifier, builder: (context, cpuProfileData, _) { if (cpuProfileData == CpuProfilerController.baseStateCpuProfileData || diff --git a/packages/devtools_app/lib/src/performance/performance_controller.dart b/packages/devtools_app/lib/src/performance/performance_controller.dart index ebd42759508..e2d4ed3961c 100644 --- a/packages/devtools_app/lib/src/performance/performance_controller.dart +++ b/packages/devtools_app/lib/src/performance/performance_controller.dart @@ -12,7 +12,7 @@ import '../utils.dart'; class PerformanceController { final cpuProfilerController = CpuProfilerController(); - CpuProfileData get cpuProfileData => cpuProfilerController.data.value; + CpuProfileData get cpuProfileData => cpuProfilerController.dataNotifier.value; /// Notifies that a CPU profile is currently being recorded. ValueListenable get recordingNotifier => _recordingNotifier; diff --git a/packages/devtools_app/lib/src/profiler/cpu_profile_controller.dart b/packages/devtools_app/lib/src/profiler/cpu_profile_controller.dart index b1ce6e9bbd4..2296fa4ac6b 100644 --- a/packages/devtools_app/lib/src/profiler/cpu_profile_controller.dart +++ b/packages/devtools_app/lib/src/profiler/cpu_profile_controller.dart @@ -19,15 +19,16 @@ class CpuProfilerController { static CpuProfileData baseStateCpuProfileData = CpuProfileData.empty(); /// Notifies that new cpu profile data is available. - ValueListenable get data => _dataNotifier; + ValueListenable get dataNotifier => _dataNotifier; final _dataNotifier = ValueNotifier(baseStateCpuProfileData); /// Notifies that CPU profile data is currently being processed. - ValueListenable get processing => _processingNotifier; + ValueListenable get processingNotifier => _processingNotifier; final _processingNotifier = ValueNotifier(false); /// Notifies that a cpu stack frame was selected. - ValueListenable get selectedCpuStackFrame => _selectedCpuStackFrameNotifier; + ValueListenable get selectedCpuStackFrameNotifier => + _selectedCpuStackFrameNotifier; final _selectedCpuStackFrameNotifier = ValueNotifier(null); final service = CpuProfilerService(); @@ -35,14 +36,15 @@ class CpuProfilerController { final transformer = CpuProfileTransformer(); /// Notifies that the vm profiler flag has changed. - ValueListenable get profilerFlag => service.profilerFlagNotifier; + ValueListenable get profilerFlagNotifier => service.profilerFlagNotifier; /// Whether the profiler is enabled. /// - /// Clients interested in the current value of [profilerFlag] should + /// Clients interested in the current value of [profilerFlagNotifier] should /// use this getter. Otherwise, clients subscribing to change notifications, - /// should listen to [profilerFlag]. - bool get profilerEnabled => profilerFlag.value.valueAsString == 'true'; + /// should listen to [profilerFlagNotifier]. + bool get profilerEnabled => + profilerFlagNotifier.value.valueAsString == 'true'; Future enableCpuProfiler() { return service.enableCpuProfiler(); @@ -86,8 +88,8 @@ class CpuProfilerController { } void selectCpuStackFrame(CpuStackFrame stackFrame) { - if (stackFrame == data.value.selectedStackFrame) return; - data.value.selectedStackFrame = stackFrame; + if (stackFrame == dataNotifier.value.selectedStackFrame) return; + dataNotifier.value.selectedStackFrame = stackFrame; _selectedCpuStackFrameNotifier.value = stackFrame; } @@ -110,143 +112,3 @@ class CpuProfilerController { transformer.dispose(); } } - -final Map cpuProfileResponseJson = { - 'type': '_CpuProfileTimeline', - 'samplePeriod': 50, - 'stackDepth': 128, - 'sampleCount': 7, - 'timeSpan': 0.003678, - 'timeOriginMicros': 47377800463, - 'timeExtentMicros': 600, - 'stackFrames': goldenCpuProfileStackFrames, - 'traceEvents': goldenCpuProfileTraceEvents, -}; - -final Map goldenCpuProfileStackFrames = { - '140357727781376-1': { - 'category': 'Dart', - 'name': 'A', - 'resolvedUrl': 'B', - }, - '140357727781376-2': { - 'category': 'Dart', - 'name': 'A1', - 'parent': '140357727781376-1', - 'resolvedUrl': 'B', - }, - '140357727781376-3': { - 'category': 'Dart', - 'name': 'A2', - 'parent': '140357727781376-1', - 'resolvedUrl': 'A', - }, - '140357727781376-4': { - 'category': 'Dart', - 'name': 'A2-A child', - 'parent': '140357727781376-3', - 'resolvedUrl': 'B', - }, - '140357727781376-5': { - 'category': 'Dart', - 'name': 'A2-B child', - 'parent': '140357727781376-3', - 'resolvedUrl': 'A', - }, - '140357727781376-6': { - 'category': 'Dart', - 'name': 'A2-C child', - 'parent': '140357727781376-3', - 'resolvedUrl': 'C', - }, - '140357727781376-7': { - 'category': 'Dart', - 'name': 'B', - 'resolvedUrl': 'A', - }, - '140357727781376-8': { - 'category': 'Dart', - 'name': 'B1', - 'parent': '140357727781376-7', - 'resolvedUrl': 'B', - }, - '140357727781376-9': { - 'category': 'Dart', - 'name': 'B2', - 'parent': '140357727781376-7', - 'resolvedUrl': 'A', - }, -}; - -final List> goldenCpuProfileTraceEvents = [ - { - 'ph': 'P', - 'name': '', - 'pid': 77616, - 'tid': 42247, - 'ts': 47377800463, - 'cat': 'Dart', - 'args': {'mode': 'basic'}, - 'sf': '140357727781376-2' - }, - { - 'ph': 'P', - 'name': '', - 'pid': 77616, - 'tid': 42247, - 'ts': 47377800563, - 'cat': 'Dart', - 'args': {'mode': 'basic'}, - 'sf': '140357727781376-2' - }, - { - 'ph': 'P', - 'name': '', - 'pid': 77616, - 'tid': 42247, - 'ts': 47377800663, - 'cat': 'Dart', - 'args': {'mode': 'basic'}, - 'sf': '140357727781376-4' - }, - { - 'ph': 'P', - 'name': '', - 'pid': 77616, - 'tid': 42247, - 'ts': 47377800763, - 'cat': 'Dart', - 'args': {'mode': 'basic'}, - 'sf': '140357727781376-5' - }, - { - 'ph': 'P', - 'name': '', - 'pid': 77616, - 'tid': 42247, - 'ts': 47377800863, - 'cat': 'Dart', - 'args': {'mode': 'basic'}, - 'sf': '140357727781376-6' - }, - { - 'ph': 'P', - 'name': '', - 'pid': 77616, - 'tid': 42247, - 'ts': 47377800963, - 'cat': 'Dart', - 'args': {'mode': 'basic'}, - 'sf': '140357727781376-8' - }, - { - 'ph': 'P', - 'name': '', - 'pid': 77616, - 'tid': 42247, - 'ts': 47377801063, - 'cat': 'Dart', - 'args': {'mode': 'basic'}, - 'sf': '140357727781376-9' - }, -]; diff --git a/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_bottom_up.dart b/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_bottom_up.dart index 6c29bb1123d..5c3dc620e2e 100644 --- a/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_bottom_up.dart +++ b/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_bottom_up.dart @@ -34,12 +34,12 @@ class CpuBottomUpTable extends StatelessWidget { Key key, this.bottomUpRoots, this.treeColumn, - this.startingSortColumn, + this.sortColumn, this.columns, ) : super(key: key); final TreeColumnData treeColumn; - final ColumnData startingSortColumn; + final ColumnData sortColumn; final List> columns; final List bottomUpRoots; @@ -50,8 +50,8 @@ class CpuBottomUpTable extends StatelessWidget { columns: columns, treeColumn: treeColumn, keyFactory: (frame) => PageStorageKey(frame.id), - startingSortColumn: startingSortColumn, - startingSortDirection: SortDirection.descending, + sortColumn: sortColumn, + sortDirection: SortDirection.descending, ); } } diff --git a/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_call_tree.dart b/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_call_tree.dart index 06c5e517645..269a9078645 100644 --- a/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_call_tree.dart +++ b/packages/devtools_app/lib/src/profiler/flutter/cpu_profile_call_tree.dart @@ -34,12 +34,12 @@ class CpuCallTreeTable extends StatelessWidget { Key key, this.data, this.treeColumn, - this.startingSortColumn, + this.sortColumn, this.columns, ) : super(key: key); final TreeColumnData treeColumn; - final ColumnData startingSortColumn; + final ColumnData sortColumn; final List> columns; final CpuProfileData data; @@ -50,8 +50,8 @@ class CpuCallTreeTable extends StatelessWidget { columns: columns, treeColumn: treeColumn, keyFactory: (frame) => PageStorageKey(frame.id), - startingSortColumn: startingSortColumn, - startingSortDirection: SortDirection.descending, + sortColumn: sortColumn, + sortDirection: SortDirection.descending, ); } } diff --git a/packages/devtools_app/lib/src/profiler/flutter/cpu_profiler.dart b/packages/devtools_app/lib/src/profiler/flutter/cpu_profiler.dart index c6a769d9f86..fe6910d4399 100644 --- a/packages/devtools_app/lib/src/profiler/flutter/cpu_profiler.dart +++ b/packages/devtools_app/lib/src/profiler/flutter/cpu_profiler.dart @@ -130,7 +130,7 @@ class _CpuProfilerState extends State final cpuFlameChart = LayoutBuilder( builder: (context, constraints) { return ValueListenableBuilder( - valueListenable: widget.controller.selectedCpuStackFrame, + valueListenable: widget.controller.selectedCpuStackFrameNotifier, builder: (context, selectedStackFrame, _) { return CpuProfileFlameChart( widget.data, diff --git a/packages/devtools_app/lib/src/table_data.dart b/packages/devtools_app/lib/src/table_data.dart index 1e8fdb83978..1d0fb7c2d0a 100644 --- a/packages/devtools_app/lib/src/table_data.dart +++ b/packages/devtools_app/lib/src/table_data.dart @@ -201,9 +201,7 @@ class TableData extends Object { } if (_sortColumn == column) { - _sortDirection = _sortDirection == SortDirection.ascending - ? SortDirection.descending - : SortDirection.ascending; + _sortDirection = _sortDirection.reverse(); } else { sortColumn = column; } diff --git a/packages/devtools_app/lib/src/timeline/flutter/event_details.dart b/packages/devtools_app/lib/src/timeline/flutter/event_details.dart index 2535ad719d9..01853c36a9f 100644 --- a/packages/devtools_app/lib/src/timeline/flutter/event_details.dart +++ b/packages/devtools_app/lib/src/timeline/flutter/event_details.dart @@ -60,7 +60,7 @@ class EventDetails extends StatelessWidget { Widget _buildDetails(TimelineController controller) { if (selectedEvent.isUiEvent) { return ValueListenableBuilder( - valueListenable: controller.cpuProfilerController.profilerFlag, + valueListenable: controller.cpuProfilerController.profilerFlagNotifier, builder: (context, profilerFlag, _) { return profilerFlag.valueAsString == 'true' ? _buildCpuProfiler(controller.cpuProfilerController) @@ -73,7 +73,7 @@ class EventDetails extends StatelessWidget { Widget _buildCpuProfiler(CpuProfilerController cpuProfilerController) { return ValueListenableBuilder( - valueListenable: cpuProfilerController.data, + valueListenable: cpuProfilerController.dataNotifier, builder: (context, cpuProfileData, _) { if (cpuProfileData == null) { return _buildProcessingInfo(cpuProfilerController); diff --git a/packages/devtools_app/lib/src/timeline/flutter/timeline_controller.dart b/packages/devtools_app/lib/src/timeline/flutter/timeline_controller.dart index 1a9a0386e6f..62d04c068e3 100644 --- a/packages/devtools_app/lib/src/timeline/flutter/timeline_controller.dart +++ b/packages/devtools_app/lib/src/timeline/flutter/timeline_controller.dart @@ -163,7 +163,7 @@ class TimelineController implements DisposableController { startMicros: selectedEvent.time.start.inMicroseconds, extentMicros: selectedEvent.time.duration.inMicroseconds, ); - data.cpuProfileData = cpuProfilerController.data.value; + data.cpuProfileData = cpuProfilerController.dataNotifier.value; } Future get displayRefreshRate async { diff --git a/packages/devtools_app/lib/src/timeline/html_timeline_controller.dart b/packages/devtools_app/lib/src/timeline/html_timeline_controller.dart index 29d77567538..47ef01f4148 100644 --- a/packages/devtools_app/lib/src/timeline/html_timeline_controller.dart +++ b/packages/devtools_app/lib/src/timeline/html_timeline_controller.dart @@ -136,7 +136,7 @@ class TimelineController implements DisposableController { startMicros: selectedEvent.time.start.inMicroseconds, extentMicros: selectedEvent.time.duration.inMicroseconds, ); - timeline.data.cpuProfileData = cpuProfilerController.data.value; + timeline.data.cpuProfileData = cpuProfilerController.dataNotifier.value; } Future loadOfflineData(OfflineData offlineData) async { diff --git a/packages/devtools_app/lib/src/ui/flutter/service_extension_widgets.dart b/packages/devtools_app/lib/src/ui/flutter/service_extension_widgets.dart index 0ccc82ef4a6..d54822ac979 100644 --- a/packages/devtools_app/lib/src/ui/flutter/service_extension_widgets.dart +++ b/packages/devtools_app/lib/src/ui/flutter/service_extension_widgets.dart @@ -8,6 +8,7 @@ import 'package:flutter/material.dart'; import '../../flutter/auto_dispose_mixin.dart'; import '../../flutter/common_widgets.dart'; +import '../../flutter/theme.dart'; import '../../globals.dart'; import '../../service_extensions.dart'; import '../../service_registrations.dart'; @@ -127,7 +128,7 @@ class _ServiceExtensionButtonGroupState waitDuration: tooltipWait, preferBelow: false, child: Padding( - padding: const EdgeInsets.symmetric(horizontal: 16.0), + padding: const EdgeInsets.symmetric(horizontal: defaultSpacing), child: Label( description.icon, description.description, @@ -323,7 +324,7 @@ class _ServiceExtensionToggleState extends State<_ServiceExtensionToggle> Text(widget.service.description), // The switch is padded on its sides by 16dp. // This balances out the tappable area. - const Padding(padding: EdgeInsets.only(left: 16.0)), + const Padding(padding: EdgeInsets.only(left: defaultSpacing)), ], ), ), diff --git a/packages/devtools_app/lib/src/utils.dart b/packages/devtools_app/lib/src/utils.dart index 1cd34fb4ad4..81110762746 100644 --- a/packages/devtools_app/lib/src/utils.dart +++ b/packages/devtools_app/lib/src/utils.dart @@ -569,7 +569,7 @@ enum SortDirection { } extension SortDirectionExtension on SortDirection { - SortDirection opposite() { + SortDirection reverse() { return this == SortDirection.ascending ? SortDirection.descending : SortDirection.ascending; diff --git a/packages/devtools_app/test/cpu_profiler_controller_test.dart b/packages/devtools_app/test/cpu_profiler_controller_test.dart index 9fd0c0bdfbc..3d35c378d1c 100644 --- a/packages/devtools_app/test/cpu_profiler_controller_test.dart +++ b/packages/devtools_app/test/cpu_profiler_controller_test.dart @@ -24,78 +24,78 @@ void main() { await controller.pullAndProcessProfile(startMicros: 0, extentMicros: 100); controller.selectCpuStackFrame(testStackFrame); expect( - controller.data.value, + controller.dataNotifier.value, isNot(equals(CpuProfilerController.baseStateCpuProfileData)), ); expect( - controller.selectedCpuStackFrame.value, + controller.selectedCpuStackFrameNotifier.value, equals(testStackFrame), ); } test('pullAndProcessProfile', () async { expect( - controller.data.value, + controller.dataNotifier.value, equals(CpuProfilerController.baseStateCpuProfileData), ); - expect(controller.processing.value, false); + expect(controller.processingNotifier.value, false); // [startMicros] and [extentMicros] are arbitrary for testing. await controller.pullAndProcessProfile(startMicros: 0, extentMicros: 100); expect( - controller.data.value, + controller.dataNotifier.value, isNot(equals(CpuProfilerController.baseStateCpuProfileData)), ); - expect(controller.processing.value, false); + expect(controller.processingNotifier.value, false); await controller.clear(); expect( - controller.data.value, + controller.dataNotifier.value, equals(CpuProfilerController.baseStateCpuProfileData), ); }); test('selectCpuStackFrame', () async { expect( - controller.data.value.selectedStackFrame, + controller.dataNotifier.value.selectedStackFrame, isNull, ); - expect(controller.selectedCpuStackFrame.value, isNull); + expect(controller.selectedCpuStackFrameNotifier.value, isNull); controller.selectCpuStackFrame(testStackFrame); expect( - controller.data.value.selectedStackFrame, + controller.dataNotifier.value.selectedStackFrame, equals(testStackFrame), ); expect( - controller.selectedCpuStackFrame.value, + controller.selectedCpuStackFrameNotifier.value, equals(testStackFrame), ); await controller.clear(); - expect(controller.selectedCpuStackFrame.value, isNull); + expect(controller.selectedCpuStackFrameNotifier.value, isNull); }); test('reset', () async { await pullProfileAndSelectFrame(); controller.reset(); expect( - controller.data.value, + controller.dataNotifier.value, equals(CpuProfilerController.baseStateCpuProfileData), ); - expect(controller.selectedCpuStackFrame.value, isNull); - expect(controller.processing.value, isFalse); + expect(controller.selectedCpuStackFrameNotifier.value, isNull); + expect(controller.processingNotifier.value, isFalse); }); test('disposes', () { controller.dispose(); expect(() { - controller.data.addListener(() {}); + controller.dataNotifier.addListener(() {}); }, throwsA(anything)); expect(() { - controller.selectedCpuStackFrame.addListener(() {}); + controller.selectedCpuStackFrameNotifier.addListener(() {}); }, throwsA(anything)); expect(() { - controller.processing.addListener(() {}); + controller.processingNotifier.addListener(() {}); }, throwsA(anything)); }); }); diff --git a/packages/devtools_app/test/flutter/table_test.dart b/packages/devtools_app/test/flutter/table_test.dart index 824091d3a14..fc9f15edd86 100644 --- a/packages/devtools_app/test/flutter/table_test.dart +++ b/packages/devtools_app/test/flutter/table_test.dart @@ -33,8 +33,8 @@ void main() { data: [TestData('empty', 0)], keyFactory: (d) => Key(d.name), onItemSelected: noop, - startingSortColumn: flatNameColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: flatNameColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -52,8 +52,8 @@ void main() { data: flatData, onItemSelected: noop, keyFactory: (d) => Key(d.name), - startingSortColumn: flatNameColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: flatNameColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -86,19 +86,21 @@ void main() { data: flatData, onItemSelected: noop, keyFactory: (d) => Key(d.name), - startingSortColumn: flatNameColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: flatNameColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); - expect(flatData[0].name, equals('Bar')); - expect(flatData[1].name, equals('Baz')); - expect(flatData[2].name, equals('Baz')); - expect(flatData[3].name, equals('Crackle')); - expect(flatData[4].name, equals('Foo')); - expect(flatData[5].name, equals('Pop')); - expect(flatData[6].name, equals('Qux')); - expect(flatData[7].name, equals('Qux')); - expect(flatData[8].name, equals('Snap')); + final FlatTableState state = tester.state(find.byWidget(table)); + final data = state.data; + expect(data[0].name, equals('Bar')); + expect(data[1].name, equals('Baz')); + expect(data[2].name, equals('Baz')); + expect(data[3].name, equals('Crackle')); + expect(data[4].name, equals('Foo')); + expect(data[5].name, equals('Pop')); + expect(data[6].name, equals('Qux')); + expect(data[7].name, equals('Qux')); + expect(data[8].name, equals('Snap')); }); testWidgets('sorts data by column', (WidgetTester tester) async { @@ -110,46 +112,48 @@ void main() { data: flatData, onItemSelected: noop, keyFactory: (d) => Key(d.name), - startingSortColumn: flatNameColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: flatNameColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); - expect(flatData[0].name, equals('Bar')); - expect(flatData[1].name, equals('Baz')); - expect(flatData[2].name, equals('Baz')); - expect(flatData[3].name, equals('Crackle')); - expect(flatData[4].name, equals('Foo')); - expect(flatData[5].name, equals('Pop')); - expect(flatData[6].name, equals('Qux')); - expect(flatData[7].name, equals('Qux')); - expect(flatData[8].name, equals('Snap')); + final FlatTableState state = tester.state(find.byWidget(table)); + final data = state.data; + expect(data[0].name, equals('Bar')); + expect(data[1].name, equals('Baz')); + expect(data[2].name, equals('Baz')); + expect(data[3].name, equals('Crackle')); + expect(data[4].name, equals('Foo')); + expect(data[5].name, equals('Pop')); + expect(data[6].name, equals('Qux')); + expect(data[7].name, equals('Qux')); + expect(data[8].name, equals('Snap')); // Reverse the sort direction. await tester.tap(find.text('FlatName')); await tester.pumpAndSettle(); - expect(flatData[8].name, equals('Bar')); - expect(flatData[7].name, equals('Baz')); - expect(flatData[6].name, equals('Baz')); - expect(flatData[5].name, equals('Crackle')); - expect(flatData[4].name, equals('Foo')); - expect(flatData[3].name, equals('Pop')); - expect(flatData[2].name, equals('Qux')); - expect(flatData[1].name, equals('Qux')); - expect(flatData[0].name, equals('Snap')); + expect(data[8].name, equals('Bar')); + expect(data[7].name, equals('Baz')); + expect(data[6].name, equals('Baz')); + expect(data[5].name, equals('Crackle')); + expect(data[4].name, equals('Foo')); + expect(data[3].name, equals('Pop')); + expect(data[2].name, equals('Qux')); + expect(data[1].name, equals('Qux')); + expect(data[0].name, equals('Snap')); // Change the sort column. await tester.tap(find.text('Number')); await tester.pumpAndSettle(); - expect(flatData[0].name, equals('Foo')); - expect(flatData[1].name, equals('Bar')); - expect(flatData[2].name, equals('Baz')); - expect(flatData[3].name, equals('Qux')); - expect(flatData[4].name, equals('Snap')); - expect(flatData[5].name, equals('Pop')); - expect(flatData[6].name, equals('Crackle')); - expect(flatData[7].name, equals('Baz')); - expect(flatData[8].name, equals('Qux')); + expect(data[0].name, equals('Foo')); + expect(data[1].name, equals('Bar')); + expect(data[2].name, equals('Baz')); + expect(data[3].name, equals('Qux')); + expect(data[4].name, equals('Snap')); + expect(data[5].name, equals('Pop')); + expect(data[6].name, equals('Crackle')); + expect(data[7].name, equals('Baz')); + expect(data[8].name, equals('Qux')); }); testWidgets('displays with many columns', (WidgetTester tester) async { @@ -163,8 +167,8 @@ void main() { data: flatData, onItemSelected: noop, keyFactory: (data) => Key(data.name), - startingSortColumn: flatNameColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: flatNameColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap( SizedBox( @@ -185,8 +189,8 @@ void main() { data: [testData], keyFactory: (d) => Key(d.name), onItemSelected: (item) => selected = item, - startingSortColumn: flatNameColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: flatNameColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -204,8 +208,8 @@ void main() { data: null, keyFactory: (d) => Key(d.name), onItemSelected: noop, - startingSortColumn: flatNameColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: flatNameColumn, + sortDirection: SortDirection.ascending, ); }, throwsAssertionError, @@ -219,8 +223,8 @@ void main() { data: flatData, keyFactory: null, onItemSelected: noop, - startingSortColumn: flatNameColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: flatNameColumn, + sortDirection: SortDirection.ascending, ); }, throwsAssertionError); }); @@ -264,8 +268,8 @@ void main() { dataRoots: [TestData('empty', 0)], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -279,8 +283,8 @@ void main() { dataRoots: [tree1, tree2], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -314,8 +318,8 @@ void main() { dataRoots: [tree1, tree2], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -333,8 +337,8 @@ void main() { dataRoots: [TestData('root1', 0), TestData('root2', 1)], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.descending, + sortColumn: treeColumn, + sortDirection: SortDirection.descending, ); await tester.pumpWidget(wrap(newTable)); expect(find.byKey(const Key('Foo')), findsNothing); @@ -358,8 +362,8 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.descending, + sortColumn: treeColumn, + sortDirection: SortDirection.descending, ); await tester.pumpWidget(wrap(table)); expect(find.byWidget(table), findsOneWidget); @@ -385,8 +389,8 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap( SizedBox( @@ -408,11 +412,12 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); await tester.pumpAndSettle(); + expect(tree1.isExpanded, true); await tester.tap(find.byKey(const Key('Foo'))); await tester.pumpAndSettle(); @@ -442,18 +447,20 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); - expect(tree1.children[0].name, equals('Bar')); - expect(tree1.children[0].children[0].name, equals('Baz')); - expect(tree1.children[0].children[1].name, equals('Crackle')); - expect(tree1.children[0].children[2].name, equals('Pop')); - expect(tree1.children[0].children[3].name, equals('Qux')); - expect(tree1.children[0].children[4].name, equals('Snap')); - expect(tree1.children[1].name, equals('Baz')); - expect(tree1.children[2].name, equals('Qux')); + final TreeTableState state = tester.state(find.byWidget(table)); + final tree = state.dataRoots[0]; + expect(tree.children[0].name, equals('Bar')); + expect(tree.children[0].children[0].name, equals('Baz')); + expect(tree.children[0].children[1].name, equals('Crackle')); + expect(tree.children[0].children[2].name, equals('Pop')); + expect(tree.children[0].children[3].name, equals('Qux')); + expect(tree.children[0].children[4].name, equals('Snap')); + expect(tree.children[1].name, equals('Baz')); + expect(tree.children[2].name, equals('Qux')); }); testWidgets('sorts data by column', (WidgetTester tester) async { @@ -465,42 +472,43 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); await tester.pumpWidget(wrap(table)); - expect(tree1.children[0].name, equals('Bar')); - expect(tree1.children[0].children[0].name, equals('Baz')); - expect(tree1.children[0].children[1].name, equals('Crackle')); - expect(tree1.children[0].children[2].name, equals('Pop')); - expect(tree1.children[0].children[3].name, equals('Qux')); - expect(tree1.children[0].children[4].name, equals('Snap')); - expect(tree1.children[1].name, equals('Baz')); - expect(tree1.children[2].name, equals('Qux')); + final TreeTableState state = tester.state(find.byWidget(table)); + final tree = state.dataRoots[0]; + expect(tree.children[0].name, equals('Bar')); + expect(tree.children[0].children[0].name, equals('Baz')); + expect(tree.children[0].children[1].name, equals('Crackle')); + expect(tree.children[0].children[2].name, equals('Pop')); + expect(tree.children[0].children[4].name, equals('Snap')); + expect(tree.children[1].name, equals('Baz')); + expect(tree.children[2].name, equals('Qux')); // Reverse the sort direction. await tester.tap(find.text('Name')); await tester.pumpAndSettle(); - expect(tree1.children[2].name, equals('Bar')); - expect(tree1.children[2].children[4].name, equals('Baz')); - expect(tree1.children[2].children[3].name, equals('Crackle')); - expect(tree1.children[2].children[2].name, equals('Pop')); - expect(tree1.children[2].children[1].name, equals('Qux')); - expect(tree1.children[2].children[0].name, equals('Snap')); - expect(tree1.children[1].name, equals('Baz')); - expect(tree1.children[0].name, equals('Qux')); + expect(tree.children[2].name, equals('Bar')); + expect(tree.children[2].children[4].name, equals('Baz')); + expect(tree.children[2].children[3].name, equals('Crackle')); + expect(tree.children[2].children[2].name, equals('Pop')); + expect(tree.children[2].children[1].name, equals('Qux')); + expect(tree.children[2].children[0].name, equals('Snap')); + expect(tree.children[1].name, equals('Baz')); + expect(tree.children[0].name, equals('Qux')); // Change the sort column. await tester.tap(find.text('Number')); await tester.pumpAndSettle(); - expect(tree1.children[0].name, equals('Bar')); - expect(tree1.children[0].children[0].name, equals('Baz')); - expect(tree1.children[0].children[1].name, equals('Qux')); - expect(tree1.children[0].children[2].name, equals('Snap')); - expect(tree1.children[0].children[3].name, equals('Pop')); - expect(tree1.children[0].children[4].name, equals('Crackle')); - expect(tree1.children[1].name, equals('Qux')); - expect(tree1.children[2].name, equals('Baz')); + expect(tree.children[0].name, equals('Bar')); + expect(tree.children[0].children[0].name, equals('Baz')); + expect(tree.children[0].children[1].name, equals('Qux')); + expect(tree.children[0].children[2].name, equals('Snap')); + expect(tree.children[0].children[3].name, equals('Pop')); + expect(tree.children[0].children[4].name, equals('Crackle')); + expect(tree.children[1].name, equals('Qux')); + expect(tree.children[2].name, equals('Baz')); }); testWidgets('properly colors rows with alternating colors', @@ -524,8 +532,8 @@ void main() { dataRoots: [data], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); final fooFinder = find.byKey(const Key('Foo')); @@ -590,8 +598,8 @@ void main() { dataRoots: null, treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); }, throwsAssertionError, @@ -605,8 +613,8 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: null, - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); }, throwsAssertionError); }); @@ -618,8 +626,8 @@ void main() { dataRoots: [tree1], treeColumn: null, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); }, throwsAssertionError); @@ -629,8 +637,8 @@ void main() { dataRoots: [tree1], treeColumn: treeColumn, keyFactory: (d) => Key(d.name), - startingSortColumn: treeColumn, - startingSortDirection: SortDirection.ascending, + sortColumn: treeColumn, + sortDirection: SortDirection.ascending, ); }, throwsAssertionError); }); diff --git a/packages/devtools_app/test/performance_controller_test.dart b/packages/devtools_app/test/performance_controller_test.dart index 41121b646aa..a787505f941 100644 --- a/packages/devtools_app/test/performance_controller_test.dart +++ b/packages/devtools_app/test/performance_controller_test.dart @@ -34,11 +34,11 @@ void main() { }, throwsA(anything)); expect(() { - controller.cpuProfilerController.data.addListener(() {}); + controller.cpuProfilerController.dataNotifier.addListener(() {}); }, throwsA(anything)); expect(() { - controller.cpuProfilerController.selectedCpuStackFrame + controller.cpuProfilerController.selectedCpuStackFrameNotifier .addListener(() {}); }, throwsA(anything)); }); From 7549718d524d7ebfa715818873a9c9d56e1e5f98 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Mon, 16 Mar 2020 15:05:18 -0700 Subject: [PATCH 3/4] Fix broken test --- packages/devtools_app/lib/src/profiler/cpu_profile_columns.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devtools_app/lib/src/profiler/cpu_profile_columns.dart b/packages/devtools_app/lib/src/profiler/cpu_profile_columns.dart index 0451725185d..7a18c5fa0f4 100644 --- a/packages/devtools_app/lib/src/profiler/cpu_profile_columns.dart +++ b/packages/devtools_app/lib/src/profiler/cpu_profile_columns.dart @@ -3,7 +3,7 @@ import '../url_utils.dart'; import '../utils.dart'; import 'cpu_profile_model.dart'; -const _timeColumnWidthPx = 145.0; +const _timeColumnWidthPx = 160.0; class SelfTimeColumn extends ColumnData { SelfTimeColumn() : super('Self Time', fixedWidthPx: _timeColumnWidthPx); From 9eba47953ea7c4c50ff475e0a9ecefa86d2da299 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Mon, 16 Mar 2020 16:08:26 -0700 Subject: [PATCH 4/4] Fix sorting bug and tweak column header alignment --- .../devtools_app/lib/src/flutter/table.dart | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/devtools_app/lib/src/flutter/table.dart b/packages/devtools_app/lib/src/flutter/table.dart index cf3adff6e87..df38f6f84e0 100644 --- a/packages/devtools_app/lib/src/flutter/table.dart +++ b/packages/devtools_app/lib/src/flutter/table.dart @@ -394,14 +394,15 @@ class TreeTableState> extends State> @override void sortData(ColumnData column, SortDirection direction) { + final sortFunction = (T a, T b) => _compareData(a, b, column, direction); void _sort(T dataObject) { dataObject.children - ..sort((T a, T b) => _compareData(a, b, column, direction)) + ..sort(sortFunction) ..forEach(_sort); } - dataRoots.where((dataRoot) => dataRoot.level == 0).toList() - ..sort((T a, T b) => _compareData(a, b, column, direction)) + dataRoots + ..sort(sortFunction) ..forEach(_sort); } @@ -733,6 +734,18 @@ class _TableRowState extends State> } } + MainAxisAlignment _mainAxisAlignmentFor(ColumnData column) { + switch (column.alignment) { + case ColumnAlignment.center: + return MainAxisAlignment.center; + case ColumnAlignment.right: + return MainAxisAlignment.end; + case ColumnAlignment.left: + default: + return MainAxisAlignment.start; + } + } + /// Presents the content of this row. Widget tableRowFor(BuildContext context) { final fontStyle = fixedFontStyle(context); @@ -745,6 +758,7 @@ class _TableRowState extends State> content = InkWell( onTap: () => _handleSortChange(column), child: Row( + mainAxisAlignment: _mainAxisAlignmentFor(column), children: [ if (isSortColumn) Icon( @@ -753,7 +767,7 @@ class _TableRowState extends State> : Icons.expand_more, size: defaultIconSize, ), - const SizedBox(width: 4.0), + const SizedBox(height: _Table.defaultRowHeight, width: 4.0), Text( column.title, overflow: TextOverflow.ellipsis,