From 72848300d9542d38aad8df4b108b369f76a89895 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:26:46 -0800 Subject: [PATCH 1/9] Working, but needs polish --- .../inspector_v2/inspector_controller.dart | 17 +++++++++++++---- .../inspector_v2/inspector_tree_controller.dart | 11 ++++++++--- .../shared/console/eval/inspector_tree_v2.dart | 2 +- .../shared/diagnostics/inspector_service.dart | 3 +++ 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart index 0add997ae1f..b6793d16aaa 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart @@ -724,6 +724,7 @@ class InspectorController extends DisposableController final pendingSelectionFuture = group.getSelection( selectedDiagnostic, treeType, + implementationWidgetsHidden: implementationWidgetsHidden.value, ); try { @@ -732,6 +733,8 @@ class InspectorController extends DisposableController // Show an error and don't update the selected node in the tree if the // user selected an implementation widget in the app while implementation // widgets are hidden in the tree. + + // TODO: Update the following. if (implementationWidgetsHidden.value && newSelection != null) { final isInTree = valueToInspectorTreeNode.containsKey( newSelection.valueRef, @@ -952,7 +955,7 @@ class InspectorController extends DisposableController } } - void selectionChanged() { + void selectionChanged({bool notifyFlutterInspector = false}) { if (!visibleToUser) { return; } @@ -968,18 +971,24 @@ class InspectorController extends DisposableController setSelectedNode(node); unawaited(_addNodeToConsole(node)); - syncSelectionHelper(selection: selectedDiagnostic); + syncSelectionHelper( + selection: selectedDiagnostic, + notifyFlutterInspector: notifyFlutterInspector, + ); } } - void syncSelectionHelper({required RemoteDiagnosticsNode? selection}) { + void syncSelectionHelper({ + required RemoteDiagnosticsNode? selection, + bool notifyFlutterInspector = false, + }) { if (selection != null) { if (selection.isCreatedByLocalProject) { _navigateTo(selection); } } - if (selection != null) { + if (notifyFlutterInspector && selection != null) { unawaited(selection.setSelectionInspector(true)); } } diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_tree_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_tree_controller.dart index 45e3d0e144f..56784743d5a 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_tree_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_tree_controller.dart @@ -193,7 +193,10 @@ class InspectorTreeController extends DisposableController } } - bool setSelectedNode(InspectorTreeNode? node) { + bool setSelectedNode( + InspectorTreeNode? node, { + bool notifyFlutterInspector = false, + }) { if (node == _selection) return false; _selection?.selected = false; @@ -201,7 +204,9 @@ class InspectorTreeController extends DisposableController _selection?.selected = true; final configLocal = config; if (configLocal.onSelectionChange != null) { - configLocal.onSelectionChange!(); + configLocal.onSelectionChange!( + notifyFlutterInspector: notifyFlutterInspector, + ); } return true; } @@ -547,7 +552,7 @@ class InspectorTreeController extends DisposableController } void onSelectNode(InspectorTreeNode? node) { - setSelectedNode(node); + setSelectedNode(node, notifyFlutterInspector: true); ga.select(gac.inspector, gac.treeNodeSelection); final diagnostic = node?.diagnostic; if (diagnostic != null && diagnostic.groupIsHidden) { diff --git a/packages/devtools_app/lib/src/shared/console/eval/inspector_tree_v2.dart b/packages/devtools_app/lib/src/shared/console/eval/inspector_tree_v2.dart index cdc4c5bcb8f..79c6b59aa3d 100644 --- a/packages/devtools_app/lib/src/shared/console/eval/inspector_tree_v2.dart +++ b/packages/devtools_app/lib/src/shared/console/eval/inspector_tree_v2.dart @@ -199,7 +199,7 @@ class InspectorTreeConfig { }); final NodeAddedCallback? onNodeAdded; - final VoidCallback? onSelectionChange; + final void Function({bool notifyFlutterInspector})? onSelectionChange; final void Function(bool added)? onClientActiveChange; final TreeEventCallback? onExpand; } diff --git a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart index 093bfac112d..1291a7577ed 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart @@ -1025,6 +1025,7 @@ class ObjectGroup extends InspectorObjectGroupBase { RemoteDiagnosticsNode? previousSelection, FlutterTreeType treeType, { bool isSummaryTree = false, + bool implementationWidgetsHidden = false, }) async { // There is no reason to allow calling this method on a disposed group. assert(!disposed); @@ -1037,6 +1038,8 @@ class ObjectGroup extends InspectorObjectGroupBase { newSelection = await invokeServiceMethodReturningNodeInspectorRef( isSummaryTree ? WidgetInspectorServiceExtensions.getSelectedSummaryWidget.name + : implementationWidgetsHidden + ? WidgetInspectorServiceExtensions.getSelectedLocalWidget.name : WidgetInspectorServiceExtensions.getSelectedWidget.name, null, ); From 945550f21f3130f6a7685de5e84ab6966b2a2175 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:26:22 -0800 Subject: [PATCH 2/9] Done with clean up --- .../inspector_v2/inspector_controller.dart | 76 +++++++++++++------ .../shared/diagnostics/inspector_service.dart | 4 +- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart index b6793d16aaa..c3ae33428e6 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart @@ -31,6 +31,7 @@ import '../../shared/diagnostics/diagnostics_node.dart'; import '../../shared/diagnostics/inspector_service.dart'; import '../../shared/diagnostics/primitives/instance_ref.dart'; import '../../shared/globals.dart'; +import '../../shared/managers/notifications.dart'; import '../../shared/primitives/query_parameters.dart'; import '../../shared/primitives/utils.dart'; import '../inspector_shared/inspector_screen.dart'; @@ -724,33 +725,14 @@ class InspectorController extends DisposableController final pendingSelectionFuture = group.getSelection( selectedDiagnostic, treeType, - implementationWidgetsHidden: implementationWidgetsHidden.value, + // If implementation widgets are hidden, the only widgets in the tree are + // those that were created by the local project. + createdByLocalProjectOnly: implementationWidgetsHidden.value, ); try { final newSelection = await pendingSelectionFuture; - // Show an error and don't update the selected node in the tree if the - // user selected an implementation widget in the app while implementation - // widgets are hidden in the tree. - - // TODO: Update the following. - if (implementationWidgetsHidden.value && newSelection != null) { - final isInTree = valueToInspectorTreeNode.containsKey( - newSelection.valueRef, - ); - final hasParent = newSelection.parent != null; - final isImplementationWidget = !isInTree && !hasParent; - if (isImplementationWidget) { - notificationService.pushError( - 'Selected an implementation widget. Please toggle "Show Implementation Widgets" and select a widget from the device again.', - allowDuplicates: true, - isReportable: false, - ); - return; - } - } - if (_disposed || group.disposed) return; selectionGroups.promoteNext(); @@ -758,6 +740,11 @@ class InspectorController extends DisposableController subtreeRoot = newSelection; applyNewSelection(newSelection); + + await _maybeShowNotificationForSelectedNode( + selectedNode: newSelection, + group: group, + ); } catch (error, st) { if (selectionGroups.next == group) { _log.shout(error, error, st); @@ -814,6 +801,51 @@ class InspectorController extends DisposableController animateTo(selectedNode.value); } + static const _implementationWidgetMessage = + 'Selected an implementation widget'; + + static const _notificationDuration = Duration(seconds: 4); + + Future _maybeShowNotificationForSelectedNode({ + required RemoteDiagnosticsNode? selectedNode, + required ObjectGroup group, + }) async { + if (selectedNode == null) return; + if (!implementationWidgetsHidden.value) return; + + // Return early if we have a new selected node. + if (_selectionIsOutOfDate(selectedNode)) return; + + final possibleImplementationWidget = await group.getSelection( + selectedDiagnostic, + treeType, + ); + + // Return early if we have a new selected node. + if (_selectionIsOutOfDate(selectedNode)) return; + + final isImplementationWidget = + !(possibleImplementationWidget?.isCreatedByLocalProject ?? true); + if (isImplementationWidget) { + final selectedWidgetName = selectedNode.description ?? ''; + + // Return early if we have a new selected node. + if (_selectionIsOutOfDate(selectedNode)) return; + + notificationService.pushNotification( + NotificationMessage( + '$_implementationWidgetMessage${selectedWidgetName.isEmpty ? '' : ' of $selectedWidgetName'}.', + duration: _notificationDuration, + ), + allowDuplicates: false, + ); + } + } + + bool _selectionIsOutOfDate(RemoteDiagnosticsNode selected) { + return selected.valueRef != selectedNode.value?.diagnostic?.valueRef; + } + Future _loadPropertiesForNode(InspectorTreeNode? node) async { final widgetProperties = []; final renderProperties = []; diff --git a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart index 1291a7577ed..fd54e912c33 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart @@ -1025,7 +1025,7 @@ class ObjectGroup extends InspectorObjectGroupBase { RemoteDiagnosticsNode? previousSelection, FlutterTreeType treeType, { bool isSummaryTree = false, - bool implementationWidgetsHidden = false, + bool createdByLocalProjectOnly = false, }) async { // There is no reason to allow calling this method on a disposed group. assert(!disposed); @@ -1038,7 +1038,7 @@ class ObjectGroup extends InspectorObjectGroupBase { newSelection = await invokeServiceMethodReturningNodeInspectorRef( isSummaryTree ? WidgetInspectorServiceExtensions.getSelectedSummaryWidget.name - : implementationWidgetsHidden + : createdByLocalProjectOnly ? WidgetInspectorServiceExtensions.getSelectedLocalWidget.name : WidgetInspectorServiceExtensions.getSelectedWidget.name, null, From a011fb1cfa7851a5e6d767a3b9a25e3928879ff8 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 11 Dec 2024 16:34:34 -0800 Subject: [PATCH 3/9] Use already existing API --- .../lib/src/shared/console/eval/inspector_tree_v2.dart | 1 - .../lib/src/shared/diagnostics/inspector_service.dart | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/devtools_app/lib/src/shared/console/eval/inspector_tree_v2.dart b/packages/devtools_app/lib/src/shared/console/eval/inspector_tree_v2.dart index 79c6b59aa3d..200da533cc2 100644 --- a/packages/devtools_app/lib/src/shared/console/eval/inspector_tree_v2.dart +++ b/packages/devtools_app/lib/src/shared/console/eval/inspector_tree_v2.dart @@ -10,7 +10,6 @@ library; import 'package:devtools_app_shared/ui.dart'; -import 'package:flutter/foundation.dart'; import '../../diagnostics/diagnostics_node.dart'; import '../../ui/search.dart'; diff --git a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart index fd54e912c33..19d3d6ba509 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart @@ -1036,10 +1036,8 @@ class ObjectGroup extends InspectorObjectGroupBase { switch (treeType) { case FlutterTreeType.widget: newSelection = await invokeServiceMethodReturningNodeInspectorRef( - isSummaryTree + isSummaryTree || createdByLocalProjectOnly ? WidgetInspectorServiceExtensions.getSelectedSummaryWidget.name - : createdByLocalProjectOnly - ? WidgetInspectorServiceExtensions.getSelectedLocalWidget.name : WidgetInspectorServiceExtensions.getSelectedWidget.name, null, ); From 5d023fc2148eeebfb63ac8a613aa9c2cd6b87812 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 11 Dec 2024 16:43:36 -0800 Subject: [PATCH 4/9] Combine params --- .../lib/src/screens/inspector/inspector_controller.dart | 2 +- .../lib/src/screens/inspector_v2/inspector_controller.dart | 2 +- .../lib/src/shared/diagnostics/inspector_service.dart | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart index b0ef7300a0e..559ae299d3c 100644 --- a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart @@ -638,7 +638,7 @@ class InspectorController extends DisposableController final pendingSelectionFuture = group.getSelection( selectedDiagnostic, treeType, - isSummaryTree: isSummaryTree, + inLocalProject: isSummaryTree, ); final pendingDetailsFuture = diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart index c3ae33428e6..550dc710297 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart @@ -727,7 +727,7 @@ class InspectorController extends DisposableController treeType, // If implementation widgets are hidden, the only widgets in the tree are // those that were created by the local project. - createdByLocalProjectOnly: implementationWidgetsHidden.value, + inLocalProject: implementationWidgetsHidden.value, ); try { diff --git a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart index 19d3d6ba509..90a10458ea7 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart @@ -1024,8 +1024,7 @@ class ObjectGroup extends InspectorObjectGroupBase { Future getSelection( RemoteDiagnosticsNode? previousSelection, FlutterTreeType treeType, { - bool isSummaryTree = false, - bool createdByLocalProjectOnly = false, + bool inLocalProject = false, }) async { // There is no reason to allow calling this method on a disposed group. assert(!disposed); @@ -1036,7 +1035,7 @@ class ObjectGroup extends InspectorObjectGroupBase { switch (treeType) { case FlutterTreeType.widget: newSelection = await invokeServiceMethodReturningNodeInspectorRef( - isSummaryTree || createdByLocalProjectOnly + inLocalProject ? WidgetInspectorServiceExtensions.getSelectedSummaryWidget.name : WidgetInspectorServiceExtensions.getSelectedWidget.name, null, From aead64f39b8fe38b597943729d7148f43ef35197 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 12 Dec 2024 10:26:15 -0800 Subject: [PATCH 5/9] Add test case --- .../inspector_integration_test.dart | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/packages/devtools_app/test/screens/inspector_v2/inspector_integration_test.dart b/packages/devtools_app/test/screens/inspector_v2/inspector_integration_test.dart index d6d16db4a95..49a6efa45af 100644 --- a/packages/devtools_app/test/screens/inspector_v2/inspector_integration_test.dart +++ b/packages/devtools_app/test/screens/inspector_v2/inspector_integration_test.dart @@ -277,6 +277,77 @@ void main() { expect(find.richTextContaining('more widgets...'), findsNothing); }); + // TODO(elliette): Expand into test group for cases when: + // - selected widget is implementation widget and implementation widgets are hidden (this test case) + // - selected widget is implementation widget and implementation widgets are visible + // - selected widget is not implementation widget and implementation widgets are hidden + // - selected widget is not implementation widget and implementation widgets are visible + testWidgetsWithWindowSize('selecting implementation widget', windowSize, ( + WidgetTester tester, + ) async { + // Load the Inspector. + await _loadInspectorUI(tester); + await tester.pumpAndSettle(inspectorChangeSettleTime); + final state = + tester.state(find.byType(InspectorScreenBody)) + as InspectorScreenBodyState; + + // Find the first Text diagnostic node. + final diagnostics = state.controller.inspectorTree.rowsInTree.value.map( + (row) => row!.node.diagnostic, + ); + final textDiagnostic = + diagnostics.firstWhere((d) => d?.description == 'Text')!; + expect(textDiagnostic.isCreatedByLocalProject, isTrue); + + // Tap the "Show Implementation Widgets" button (selected by default). + final showImplementationWidgetsButton = find.descendant( + of: find.byType(DevToolsToggleButton), + matching: find.text('Show Implementation Widgets'), + ); + expect(showImplementationWidgetsButton, findsOneWidget); + await tester.tap(showImplementationWidgetsButton); + await tester.pumpAndSettle(inspectorChangeSettleTime); + + // Verify the Text diagnostic node is still in the tree. + final diagnosticsNow = state.controller.inspectorTree.rowsInTree.value.map( + (row) => row!.node.diagnostic, + ); + expect( + diagnosticsNow.any((d) => d?.valueRef == textDiagnostic.valueRef), + isTrue, + ); + + // Get the RichText child of the Text diagnostic node. + final service = serviceConnection.inspectorService as InspectorService; + final group = service.createObjectGroup('test-group'); + final textSubtree = await group.getDetailsSubtree(textDiagnostic); + final richTextDiagnostic = (await textSubtree!.children)!.firstWhere( + (child) => child.description == 'RichText', + ); + + // Verify the RichText child is an implementation node that is not in the tree. + expect(richTextDiagnostic.isCreatedByLocalProject, isFalse); + expect( + diagnosticsNow.any((d) => d?.valueRef == richTextDiagnostic.valueRef), + isFalse, + ); + + // Mimic selecting the RichText diagnostic node with the on-device inspector. + await group.setSelectionInspector(richTextDiagnostic.valueRef, false); + await tester.pumpAndSettle(inspectorChangeSettleTime); + + // Verify the Text node is now selected. + final selectedNode = state.controller.selectedNode.value; + expect(selectedNode!.diagnostic!.valueRef, equals(textDiagnostic.valueRef)); + + // Verify the notification about selecting an implementation widget is displayed. + expect( + find.text('Selected an implementation widget of Text.'), + findsOneWidget, + ); + }); + testWidgetsWithWindowSize('can revert to legacy inspector', windowSize, ( WidgetTester tester, ) async { From 0a99e392c1fb74f66faa67f2405a122be2178b7e Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 12 Dec 2024 11:09:33 -0800 Subject: [PATCH 6/9] Add more details to notification message --- .../screens/inspector_v2/inspector_controller.dart | 11 +++++++++-- .../inspector_v2/inspector_integration_test.dart | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart index ac52ced5e67..8ff6cdd9a33 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart @@ -832,16 +832,23 @@ class InspectorController extends DisposableController if (_selectionIsOutOfDate(selectedNode)) return; final isImplementationWidget = - !(possibleImplementationWidget?.isCreatedByLocalProject ?? true); + possibleImplementationWidget != null && + !possibleImplementationWidget.isCreatedByLocalProject; if (isImplementationWidget) { final selectedWidgetName = selectedNode.description ?? ''; + final implementationWidgetName = + possibleImplementationWidget.description ?? ''; // Return early if we have a new selected node. if (_selectionIsOutOfDate(selectedNode)) return; + final messageDetails = + selectedWidgetName.isEmpty + ? '' + : ' of $selectedWidgetName${implementationWidgetName.isEmpty ? '' : ': $implementationWidgetName'}'; notificationService.pushNotification( NotificationMessage( - '$_implementationWidgetMessage${selectedWidgetName.isEmpty ? '' : ' of $selectedWidgetName'}.', + '$_implementationWidgetMessage$messageDetails.', duration: _notificationDuration, ), allowDuplicates: false, diff --git a/packages/devtools_app/test/screens/inspector_v2/inspector_integration_test.dart b/packages/devtools_app/test/screens/inspector_v2/inspector_integration_test.dart index 49a6efa45af..0de6195d718 100644 --- a/packages/devtools_app/test/screens/inspector_v2/inspector_integration_test.dart +++ b/packages/devtools_app/test/screens/inspector_v2/inspector_integration_test.dart @@ -343,7 +343,7 @@ void main() { // Verify the notification about selecting an implementation widget is displayed. expect( - find.text('Selected an implementation widget of Text.'), + find.text('Selected an implementation widget of Text: RichText.'), findsOneWidget, ); }); From 3cc73dcd92597a24ec242423b09e148f35e96ab7 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 12 Dec 2024 11:12:07 -0800 Subject: [PATCH 7/9] Update release notes --- packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 9bce52cec04..68b74f64958 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -22,7 +22,8 @@ To learn more about DevTools, check out the settings to allow auto-refreshing the widget tree after a hot-reload. - [#8483](https://github.com/flutter/devtools/pull/8483) - Fixed an [issue](https://github.com/flutter/devtools/issues/8487) where the [new Inspector](https://docs.flutter.dev/tools/devtools/release-notes/release-notes-2.40.1#inspector-updates) would not load after a hot-restart in the legacy Inspector. - [#8491](https://github.com/flutter/devtools/pull/8491) - Fixed an [issue](https://github.com/flutter/devtools/issues/8465) preventing scrolling to an implementation widget in the [new Inspector](https://docs.flutter.dev/tools/devtools/release-notes/release-notes-2.40.1#inspector-updates) after selecting it on the device. - [#8494](https://github.com/flutter/devtools/pull/8494) - +- Selecting an implementation widget on the device while implementation widget's are hidden in the [new Inspector's](https://docs.flutter.dev/tools/devtools/release-notes/release-notes-2.40.1#inspector-updates) does not show an error. - [8625](https://github.com/flutter/devtools/pull/8625) + ## Performance updates TODO: Remove this section if there are not any general updates. From 0178182adf2c9d07ec31862f547bde5f82ba8ecc Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 12 Dec 2024 12:48:58 -0800 Subject: [PATCH 8/9] Changed param name to inLocalProjectOnly --- .../lib/src/screens/inspector/inspector_controller.dart | 2 +- .../lib/src/screens/inspector_v2/inspector_controller.dart | 2 +- .../lib/src/shared/diagnostics/inspector_service.dart | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart index 559ae299d3c..d65495a36cf 100644 --- a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart @@ -638,7 +638,7 @@ class InspectorController extends DisposableController final pendingSelectionFuture = group.getSelection( selectedDiagnostic, treeType, - inLocalProject: isSummaryTree, + inLocalProjectOnly: isSummaryTree, ); final pendingDetailsFuture = diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart index 8ff6cdd9a33..6293d1957c6 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart @@ -734,7 +734,7 @@ class InspectorController extends DisposableController treeType, // If implementation widgets are hidden, the only widgets in the tree are // those that were created by the local project. - inLocalProject: implementationWidgetsHidden.value, + inLocalProjectOnly: implementationWidgetsHidden.value, ); try { diff --git a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart index 90a10458ea7..bb35b2996af 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart @@ -1024,7 +1024,7 @@ class ObjectGroup extends InspectorObjectGroupBase { Future getSelection( RemoteDiagnosticsNode? previousSelection, FlutterTreeType treeType, { - bool inLocalProject = false, + bool inLocalProjectOnly = false, }) async { // There is no reason to allow calling this method on a disposed group. assert(!disposed); @@ -1035,7 +1035,7 @@ class ObjectGroup extends InspectorObjectGroupBase { switch (treeType) { case FlutterTreeType.widget: newSelection = await invokeServiceMethodReturningNodeInspectorRef( - inLocalProject + inLocalProjectOnly ? WidgetInspectorServiceExtensions.getSelectedSummaryWidget.name : WidgetInspectorServiceExtensions.getSelectedWidget.name, null, From c6628d9391b2e8fcb7ff8b3f7efa38ecc786a114 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 12 Dec 2024 13:32:31 -0800 Subject: [PATCH 9/9] Respond to PR comments --- .../inspector/inspector_controller.dart | 2 +- .../inspector_v2/inspector_controller.dart | 26 ++++++++++++++----- .../shared/diagnostics/inspector_service.dart | 4 +-- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart index d65495a36cf..f9d1f866c1c 100644 --- a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart @@ -638,7 +638,7 @@ class InspectorController extends DisposableController final pendingSelectionFuture = group.getSelection( selectedDiagnostic, treeType, - inLocalProjectOnly: isSummaryTree, + restrictToLocalProject: isSummaryTree, ); final pendingDetailsFuture = diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart index 6293d1957c6..79827334047 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart @@ -734,7 +734,7 @@ class InspectorController extends DisposableController treeType, // If implementation widgets are hidden, the only widgets in the tree are // those that were created by the local project. - inLocalProjectOnly: implementationWidgetsHidden.value, + restrictToLocalProject: implementationWidgetsHidden.value, ); try { @@ -817,11 +817,11 @@ class InspectorController extends DisposableController required RemoteDiagnosticsNode? selectedNode, required ObjectGroup group, }) async { - if (selectedNode == null) return; - if (!implementationWidgetsHidden.value) return; - - // Return early if we have a new selected node. - if (_selectionIsOutOfDate(selectedNode)) return; + if (selectedNode == null || + !implementationWidgetsHidden.value || + _selectionIsOutOfDate(selectedNode)) { + return; + } final possibleImplementationWidget = await group.getSelection( selectedDiagnostic, @@ -842,6 +842,8 @@ class InspectorController extends DisposableController // Return early if we have a new selected node. if (_selectionIsOutOfDate(selectedNode)) return; + // Show a notification that the user selected an implementation widget, + // e.g. "Selected an implementation widget of Text: RichText." final messageDetails = selectedWidgetName.isEmpty ? '' @@ -1001,6 +1003,12 @@ class InspectorController extends DisposableController } } + /// Handles updating the widget tree when the selecected widget changes. + /// + /// [notifyFlutterInspector] determines whether a request should be sent to + /// the Widget Inspector in the Flutter framework to update the on-device + /// selection. This should only be true if the the selection was changed due + /// to a user action in DevTools (e.g. clicking on a widget in the tree). void selectionChanged({bool notifyFlutterInspector = false}) { if (!visibleToUser) { return; @@ -1024,6 +1032,12 @@ class InspectorController extends DisposableController } } + /// Syncs the selection state after a new widgets was selected. + /// + /// [notifyFlutterInspector] determines whether a request should be sent to + /// the Widget Inspector in the Flutter framework to update the on-device + /// selection. This should only be true if the the selection was changed due + /// to a user action in DevTools (e.g. clicking on a widget in the tree). void syncSelectionHelper({ required RemoteDiagnosticsNode? selection, bool notifyFlutterInspector = false, diff --git a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart index bb35b2996af..40631de53ef 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart @@ -1024,7 +1024,7 @@ class ObjectGroup extends InspectorObjectGroupBase { Future getSelection( RemoteDiagnosticsNode? previousSelection, FlutterTreeType treeType, { - bool inLocalProjectOnly = false, + bool restrictToLocalProject = false, }) async { // There is no reason to allow calling this method on a disposed group. assert(!disposed); @@ -1035,7 +1035,7 @@ class ObjectGroup extends InspectorObjectGroupBase { switch (treeType) { case FlutterTreeType.widget: newSelection = await invokeServiceMethodReturningNodeInspectorRef( - inLocalProjectOnly + restrictToLocalProject ? WidgetInspectorServiceExtensions.getSelectedSummaryWidget.name : WidgetInspectorServiceExtensions.getSelectedWidget.name, null,