Fix inspector widget tree visibility issues during keyboard navigation

muhammadkamel · muhammadkamel · commit 45c2a194e9ba · 2026-04-30T00:01:40.000+03:00
Resolves two regressions surfaced when navigating the inspector widget
tree with the arrow-left key.

1. Clicking a still-visible row used to call `expandPath` on the clicked
   node, which re-expanded the clicked node itself and undid any subtree
   collapses the user had just performed via the arrow-left key. Removed
   the call from `onSelectNode`; programmatic selection flows (search,
   on-device pick) continue to call `expandPath` via `syncTreeSelection`,
   so external selection still works correctly.

2. Collapsing all the way to the root via the arrow-left key shrank the
   visible rows down to a single row, which the inspector treated as a
   "still loading" state and replaced with a spinner — hiding the user's
   `[root]` row. Gated that branch on `!firstInspectorTreeLoadCompleted`
   so the spinner only shows during the initial load; afterwards a
   one-row tree renders as the legitimate single-row state.

Adds regression tests covering both behaviors.
diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart
@@ -575,7 +575,12 @@ class InspectorTreeController extends DisposableController
     if (diagnostic != null && diagnostic.groupIsHidden) {
       diagnostic.hideableGroupLeader?.toggleHiddenGroup();
     }
-    expandPath(node);
+    // Intentionally do NOT call expandPath(node) here. User clicks happen on
+    // already-visible rows, so ancestors are already expanded; calling
+    // expandPath would also re-expand the clicked node itself, undoing any
+    // collapse the user just performed via the left arrow key. Programmatic
+    // selection paths (search, on-device pick) call expandPath themselves via
+    // [InspectorController.syncTreeSelection].
   }
 
   Rect getBoundingBox(InspectorTreeRow row) {
@@ -1144,8 +1149,12 @@ class _InspectorTreeState extends State<InspectorTree>
       valueListenable: treeControllerLocal.rowsInTree,
       builder: (context, rows, _) {
         // Note: The inspector rows contain only the fake root node when the
-        // inspector tree is shutdown.
-        if (rows.length <= 1) {
+        // inspector tree is shutdown. Only show the loading indicator on the
+        // initial tree load (before [firstInspectorTreeLoadCompleted] is set);
+        // after that, a one-row tree is the legitimate result of the user
+        // collapsing all the way to the root via the keyboard, and we should
+        // render the tree (with its single row) rather than a spinner.
+        if (rows.length <= 1 && !controller.firstInspectorTreeLoadCompleted) {
           // This works around a bug when Scrollbars are present on a short lived
           // widget.
           return const SizedBox(child: CenteredCircularProgressIndicator());
diff --git a/packages/devtools_app/test/screens/inspector/inspector_tree_test.dart b/packages/devtools_app/test/screens/inspector/inspector_tree_test.dart
@@ -71,6 +71,10 @@ void main() {
         ..appendChild(InspectorTreeNode()));
   }
 
+  List<InspectorTreeNode> visibleNodes(InspectorTreeController controller) {
+    return controller.rowsInTree.value.map((row) => row!.node).toList();
+  }
+
   group('InspectorTreeController', () {
     testWidgets('Row with negative index regression test', (
       WidgetTester tester,
@@ -363,5 +367,212 @@ void main() {
         expect(capturedNotify, isTrue);
       },
     );
+
+    testWidgets(
+      'arrowLeft key collapses selected node without removing previous rows',
+      (WidgetTester tester) async {
+        final treeController = buildTreeController(
+          onSelectionChange: ({bool notifyFlutterInspector = false}) {},
+        );
+
+        final root = treeController.root!;
+        final previousSibling = root.children.first;
+        final selectedSibling = root.children.last
+          ..appendChild(InspectorTreeNode())
+          ..appendChild(InspectorTreeNode());
+        final selectedSiblingFirstChild = selectedSibling.children.first;
+        final selectedSiblingSecondChild = selectedSibling.children.last;
+        treeController.root = root;
+        treeController.setSelectedNode(selectedSibling);
+
+        await pumpInspectorTree(tester, treeController: treeController);
+
+        expect(visibleNodes(treeController), [
+          root,
+          previousSibling,
+          selectedSibling,
+          selectedSiblingFirstChild,
+          selectedSiblingSecondChild,
+        ]);
+
+        await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
+        await tester.pump();
+
+        expect(selectedSibling.isExpanded, isFalse);
+        expect(treeController.selection, selectedSibling);
+        expect(visibleNodes(treeController), [
+          root,
+          previousSibling,
+          selectedSibling,
+        ]);
+      },
+    );
+
+    testWidgets(
+      'arrowLeft key on collapsed child selects parent without changing rows',
+      (WidgetTester tester) async {
+        final treeController = buildTreeController(
+          onSelectionChange: ({bool notifyFlutterInspector = false}) {},
+        );
+
+        final root = treeController.root!;
+        final previousSibling = root.children.first;
+        final parent = root.children.last
+          ..appendChild(InspectorTreeNode())
+          ..appendChild(InspectorTreeNode());
+        final child = parent.children.first..isExpanded = false;
+        final nextSibling = parent.children.last;
+        treeController.root = root;
+        treeController.setSelectedNode(child);
+
+        await pumpInspectorTree(tester, treeController: treeController);
+
+        final rowsBeforeArrowLeft = visibleNodes(treeController);
+        expect(rowsBeforeArrowLeft, [
+          root,
+          previousSibling,
+          parent,
+          child,
+          nextSibling,
+        ]);
+
+        await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
+        await tester.pump();
+
+        expect(treeController.selection, parent);
+        expect(visibleNodes(treeController), rowsBeforeArrowLeft);
+      },
+    );
+
+    testWidgets('arrowLeft key does not put the tree into the loading state', (
+      WidgetTester tester,
+    ) async {
+      final treeController = buildTreeController(
+        onSelectionChange: ({bool notifyFlutterInspector = false}) {},
+      );
+
+      await pumpInspectorTree(tester, treeController: treeController);
+
+      final root = treeController.root!;
+      treeController.setSelectedNode(root);
+      await tester.pump();
+
+      await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
+      await tester.pump();
+
+      // After collapsing the root, only the root row remains visible. The
+      // tree must still render that row instead of a loading indicator.
+      expect(find.byType(CenteredCircularProgressIndicator), findsNothing);
+      expect(visibleNodes(treeController), [root]);
+    });
+
+    testWidgets(
+      'onSelectNode does not re-expand collapsed siblings of the clicked node',
+      (WidgetTester tester) async {
+        // Regression test: clicking a still-visible row used to call
+        // expandPath on the clicked node, which re-expanded the clicked node
+        // itself and undid any subtree collapses the user had just performed
+        // via the arrow-left key.
+        final treeController = buildTreeController(
+          onSelectionChange: ({bool notifyFlutterInspector = false}) {},
+        );
+
+        final root = treeController.root!;
+        final firstChild = root.children.first
+          ..appendChild(InspectorTreeNode())
+          ..appendChild(InspectorTreeNode());
+        final secondChild = root.children.last;
+        treeController.root = root;
+        treeController.setSelectedNode(firstChild);
+
+        await pumpInspectorTree(tester, treeController: treeController);
+
+        // Collapse [firstChild] so its grandchildren are hidden.
+        await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft);
+        await tester.pump();
+        expect(firstChild.isExpanded, isFalse);
+        expect(visibleNodes(treeController), [root, firstChild, secondChild]);
+
+        // Selecting another visible row must not re-expand [firstChild].
+        treeController.onSelectNode(secondChild);
+        await tester.pump();
+
+        expect(firstChild.isExpanded, isFalse);
+        expect(visibleNodes(treeController), [root, firstChild, secondChild]);
+      },
+    );
+
+    testWidgets(
+      'onSelectNode does not re-expand a node the user just collapsed by '
+      'clicking it',
+      (WidgetTester tester) async {
+        // Regression test: clicking a row used to call expandPath on the
+        // clicked node itself, so a user could not select a node in its
+        // collapsed state.
+        final treeController = buildTreeController(
+          onSelectionChange: ({bool notifyFlutterInspector = false}) {},
+        );
+
+        final root = treeController.root!;
+        final firstChild = root.children.first
+          ..appendChild(InspectorTreeNode())
+          ..isExpanded = false;
+        treeController.root = root;
+
+        await pumpInspectorTree(tester, treeController: treeController);
+
+        treeController.onSelectNode(firstChild);
+        await tester.pump();
+
+        expect(treeController.selection, firstChild);
+        expect(firstChild.isExpanded, isFalse);
+      },
+    );
+  });
+
+  group('InspectorTree loading indicator', () {
+    testWidgets(
+      'shows a loading indicator while the initial tree load is in progress',
+      (WidgetTester tester) async {
+        // Before the first inspector tree load completes, a tree with at most
+        // a single row represents the "still loading" state and should render
+        // a progress indicator instead of the bare row.
+        inspectorController.firstInspectorTreeLoadCompleted = false;
+
+        final treeController = InspectorTreeController()
+          ..config = InspectorTreeConfig(
+            onNodeAdded: (_, _) {},
+            onClientActiveChange: (_) {},
+          )
+          ..root = InspectorTreeNode();
+
+        await pumpInspectorTree(tester, treeController: treeController);
+
+        expect(find.byType(CenteredCircularProgressIndicator), findsOneWidget);
+      },
+    );
+
+    testWidgets(
+      'renders a single-row tree (no spinner) after the initial load has '
+      'completed',
+      (WidgetTester tester) async {
+        // Regression test: collapsing the root via the arrow-left key shrinks
+        // the visible rows down to a single row. Before the fix, the
+        // [InspectorTree] widget treated a one-row tree as "loading" and
+        // showed a spinner, hiding the user's [root] row.
+        inspectorController.firstInspectorTreeLoadCompleted = true;
+
+        final treeController = InspectorTreeController()
+          ..config = InspectorTreeConfig(
+            onNodeAdded: (_, _) {},
+            onClientActiveChange: (_) {},
+          )
+          ..root = InspectorTreeNode();
+
+        await pumpInspectorTree(tester, treeController: treeController);
+
+        expect(find.byType(CenteredCircularProgressIndicator), findsNothing);
+      },
+    );
   });
 }
