From 89b8270203da77308ca9c606bde068bf9dc6913b Mon Sep 17 00:00:00 2001
From: Yura Zenevich <yura.zenevich@gmail.com>
Date: Thu, 8 Mar 2018 15:51:09 -0500
Subject: [PATCH] Bug 1438912 - by default, do not navigate to next node on
 ArrowRight key. r=nchevobbe

MozReview-Commit-ID: KQlHIA7pYAn
---
 devtools/client/memory/components/Census.js   |  1 +
 .../client/memory/components/DominatorTree.js |  1 +
 .../client/memory/components/Individuals.js   |  1 +
 .../components/jit-optimizations.js           |  1 +
 .../performance/components/waterfall-tree.js  |  1 +
 .../shared/components/VirtualizedTree.js      | 10 ++++-
 .../test/mochitest/test_tree_06.html          | 41 ++++++++++++++++---
 7 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/devtools/client/memory/components/Census.js b/devtools/client/memory/components/Census.js
index 1525c6f764a9..11dc27bb7d29 100644
--- a/devtools/client/memory/components/Census.js
+++ b/devtools/client/memory/components/Census.js
@@ -49,6 +49,7 @@ class Census extends Component {
 
     return Tree({
       autoExpandDepth: 0,
+      preventNavigationOnArrowRight: false,
       focused: census.focused,
       getParent: node => {
         const parent = parentMap[node.id];
diff --git a/devtools/client/memory/components/DominatorTree.js b/devtools/client/memory/components/DominatorTree.js
index 12c9dc8b37e7..d83f0cf03394 100644
--- a/devtools/client/memory/components/DominatorTree.js
+++ b/devtools/client/memory/components/DominatorTree.js
@@ -134,6 +134,7 @@ class DominatorTree extends Component {
     return Tree({
       key: "dominator-tree-tree",
       autoExpandDepth: DOMINATOR_TREE_AUTO_EXPAND_DEPTH,
+      preventNavigationOnArrowRight: false,
       focused: dominatorTree.focused,
       getParent: node =>
         node instanceof DominatorTreeLazyChildren
diff --git a/devtools/client/memory/components/Individuals.js b/devtools/client/memory/components/Individuals.js
index c9ee79fd5fa9..e22affc3c705 100644
--- a/devtools/client/memory/components/Individuals.js
+++ b/devtools/client/memory/components/Individuals.js
@@ -35,6 +35,7 @@ class Individuals extends Component {
     return Tree({
       key: "individuals-tree",
       autoExpandDepth: 0,
+      preventNavigationOnArrowRight: false,
       focused: individuals.focused,
       getParent: node => null,
       getChildren: node => [],
diff --git a/devtools/client/performance/components/jit-optimizations.js b/devtools/client/performance/components/jit-optimizations.js
index f24464e5c51e..e5fe97f7070f 100644
--- a/devtools/client/performance/components/jit-optimizations.js
+++ b/devtools/client/performance/components/jit-optimizations.js
@@ -189,6 +189,7 @@ class JITOptimizations extends Component {
 
     return Tree({
       autoExpandDepth,
+      preventNavigationOnArrowRight: false,
       getParent: node => {
         let site = getSite(node.id);
         let parent;
diff --git a/devtools/client/performance/components/waterfall-tree.js b/devtools/client/performance/components/waterfall-tree.js
index c8eda9f69e57..6fd66006ba7f 100644
--- a/devtools/client/performance/components/waterfall-tree.js
+++ b/devtools/client/performance/components/waterfall-tree.js
@@ -163,6 +163,7 @@ class WaterfallTree extends Component {
 
   render() {
     return Tree({
+      preventNavigationOnArrowRight: false,
       getRoots: this._getRoots,
       getParent: this._getParent,
       getChildren: this._getChildren,
diff --git a/devtools/client/shared/components/VirtualizedTree.js b/devtools/client/shared/components/VirtualizedTree.js
index 84d726f7d61b..249ac70bd069 100644
--- a/devtools/client/shared/components/VirtualizedTree.js
+++ b/devtools/client/shared/components/VirtualizedTree.js
@@ -198,6 +198,10 @@ class Tree extends Component {
       // Handle when item is activated with a keyboard (using Space or Enter)
       onActivate: PropTypes.func,
 
+      // Indicates if pressing ArrowRight key should only expand expandable node
+      // or if the selection should also move to the next node.
+      preventNavigationOnArrowRight: PropTypes.bool,
+
       // The depth to which we should automatically expand new items.
       autoExpandDepth: PropTypes.number,
 
@@ -228,6 +232,7 @@ class Tree extends Component {
   static get defaultProps() {
     return {
       autoExpandDepth: AUTO_EXPAND_DEPTH,
+      preventNavigationOnArrowRight: true,
     };
   }
 
@@ -502,9 +507,10 @@ class Tree extends Component {
         break;
 
       case "ArrowRight":
-        if (!this.props.isExpanded(this.props.focused)) {
+        if (this.props.getChildren(this.props.focused).length &&
+            !this.props.isExpanded(this.props.focused)) {
           this._onExpand(this.props.focused);
-        } else {
+        } else if (!this.props.preventNavigationOnArrowRight) {
           this._focusNextNode();
         }
         break;
diff --git a/devtools/client/shared/components/test/mochitest/test_tree_06.html b/devtools/client/shared/components/test/mochitest/test_tree_06.html
index ad1b342f31e8..f6790675d148 100644
--- a/devtools/client/shared/components/test/mochitest/test_tree_06.html
+++ b/devtools/client/shared/components/test/mochitest/test_tree_06.html
@@ -16,12 +16,17 @@ Test keyboard navigation with the Tree component.
 <pre id="test">
 <script src="head.js" type="application/javascript"></script>
 <script type="application/javascript">
+
+"use strict";
+
 window.onload = async function () {
   try {
     const ReactDOM = browserRequire("devtools/client/shared/vendor/react-dom");
     const { createFactory } = browserRequire("devtools/client/shared/vendor/react");
-    const { Simulate } = browserRequire("devtools/client/shared/vendor/react-dom-test-utils");
-    const Tree = createFactory(browserRequire("devtools/client/shared/components/VirtualizedTree"));
+    const { Simulate } =
+      browserRequire("devtools/client/shared/vendor/react-dom-test-utils");
+    const Tree =
+      createFactory(browserRequire("devtools/client/shared/components/VirtualizedTree"));
 
     function renderTree(props) {
       const treeProps = Object.assign({},
@@ -175,7 +180,8 @@ window.onload = async function () {
       "M:false",
       "-N:false",
       "--O:true",
-    ], "After the DOWN, O should still be focused and we shouldn't have overflowed past it.");
+    ], "After the DOWN, O should still be focused " +
+       "and we shouldn't have overflowed past it.");
 
     // LEFT --------------------------------------------------------------------
 
@@ -246,7 +252,30 @@ window.onload = async function () {
       "--O:false",
     ], "After the RIGHT, E's children should be expanded again.");
 
-    info("Right to go to next item.");
+    info("Right on already expanded node.");
+    Simulate.keyDown(document.querySelector(".tree"), { key: "ArrowRight" });
+    await forceRender(tree);
+
+    isRenderedTree(document.body.textContent, [
+      "A:false",
+      "-B:false",
+      "--E:true",
+      "---K:false",
+      "---L:false",
+      "--F:false",
+      "--G:false",
+      "-C:false",
+      "--H:false",
+      "--I:false",
+      "-D:false",
+      "--J:false",
+      "M:false",
+      "-N:false",
+      "--O:false",
+    ], "After the RIGHT on already expanded node, E should remain focused.");
+
+    info("Right when preventNavigationOnArrowRight is unset to go to next item.");
+    renderTree({ focused: "E", preventNavigationOnArrowRight: false });
     Simulate.keyDown(document.querySelector(".tree"), { key: "ArrowRight" });
     await forceRender(tree);
 
@@ -275,6 +304,8 @@ window.onload = async function () {
       { key: "ArrowDown", metaKey: true },
       { key: "ArrowDown", shiftKey: true },
     ];
+    await forceRender(tree);
+
     for (let key of keysWithModifier) {
       Simulate.keyDown(document.querySelector(".tree"), key);
       await forceRender(tree);
@@ -297,7 +328,7 @@ window.onload = async function () {
         "--O:false",
       ], "After DOWN + (alt|ctrl|meta|shift), K should remain focused.");
     }
-  } catch(e) {
+  } catch (e) {
     ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
   } finally {
     SimpleTest.finish();