Bug 1438835 - [VirtualizedTree] keep inner state for when the tree is moused down, to help with deciding when to keyboard focus on tree container. r=nchevobbe

MozReview-Commit-ID: 6W9N242QmmU

devtools/client/shared/components/VirtualizedTree.js

@@ -238,6 +238,7 @@ class Tree extends Component {
       scroll: 0,
       height: window.innerHeight,
       seen: new Set(),
+      mouseDown: false
     };
 
     this._onExpand = oncePerAnimationFrame(this._onExpand).bind(this);
@@ -271,6 +272,15 @@ class Tree extends Component {
     this._updateHeight();
   }
 
+  shouldComponentUpdate(nextProps, nextState) {
+    let { scroll, height, seen, mouseDown } = this.state;
+
+    return scroll !== nextState.scroll ||
+           height !== nextState.height ||
+           seen !== nextState.seen ||
+           mouseDown === nextState.mouseDown;
+  }
+
   componentWillUnmount() {
     window.removeEventListener("resize", this._updateHeight);
   }
@@ -329,12 +339,7 @@ class Tree extends Component {
    * Updates the state's height based on clientHeight.
    */
   _updateHeight() {
-    if (this.refs.tree.clientHeight &&
-        this.refs.tree.clientHeight !== this.state.height) {
-      this.setState({
-        height: this.refs.tree.clientHeight
-      });
-    }
+    this.setState({ height: this.refs.tree.clientHeight });
   }
 
   /**
@@ -685,19 +690,17 @@ class Tree extends Component {
         onKeyPress: this._preventArrowKeyScrolling,
         onKeyUp: this._preventArrowKeyScrolling,
         onScroll: this._onScroll,
-        onFocus: ({nativeEvent}) => {
-          if (focused || !nativeEvent || !this.refs.tree) {
+        onMouseDown: () => this.setState({ mouseDown: true }),
+        onMouseUp: () => this.setState({ mouseDown: false }),
+        onFocus: () => {
+          if (focused || this.state.mouseDown) {
             return;
           }
 
-          let { explicitOriginalTarget } = nativeEvent;
-          // Only set default focus to the first tree node if the focus came
-          // from outside the tree (e.g. by tabbing to the tree from other
-          // external elements).
-          if (explicitOriginalTarget !== this.refs.tree &&
-              !this.refs.tree.contains(explicitOriginalTarget)) {
-            this._focus(begin, toRender[0].item);
-          }
+          // Only set default focus to the first tree node if focused node is
+          // not yet set and the focus event is not the result of a mouse
+          // interarction.
+          this._focus(begin, toRender[0].item);
         },
         onClick: () => {
           // Focus should always remain on the tree container itself.

devtools/client/shared/components/test/mochitest/test_tree_05.html

@@ -17,11 +17,16 @@ Test focusing with the Tree component.
 <script src="head.js" type="application/javascript"></script>
 <script type="application/javascript">
 
-window.onload = Task.async(function* () {
+"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 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({},
@@ -33,6 +38,7 @@ window.onload = Task.async(function* () {
     }
 
     const tree = renderTree();
+    const treeElem = document.querySelector(".tree");
 
     isAccessibleTree(tree);
     TEST_TREE.expanded = new Set("ABCDEFGHIJKLMNO".split(""));
@@ -58,9 +64,32 @@ window.onload = Task.async(function* () {
       "--O:false",
     ], "G should be focused");
 
+    // When tree gets focus by means other than mouse, do not set first node as
+    // focused node when there is already a focused node.
+    Simulate.focus(treeElem);
+    await forceRender(tree);
+
+    isRenderedTree(document.body.textContent, [
+      "A:false",
+      "-B:false",
+      "--E:false",
+      "---K:false",
+      "---L:false",
+      "--F:false",
+      "--G:true",
+      "-C:false",
+      "--H:false",
+      "--I:false",
+      "-D:false",
+      "--J:false",
+      "M:false",
+      "-N:false",
+      "--O:false",
+    ], "G should remain focused");
+
     // Click the first tree node
     document.querySelector(".tree-node").click();
-    yield forceRender(tree);
+    await forceRender(tree);
 
     isRenderedTree(document.body.textContent, [
       "A:true",
@@ -79,12 +108,87 @@ window.onload = Task.async(function* () {
       "-N:false",
       "--O:false",
     ], "A should be focused");
-  } catch(e) {
+
+    // Mouse down and mouse up events set tree "mouseDown" state correctly.
+    ok(!tree.state.mouseDown, "Mouse down state is not set.");
+    Simulate.mouseDown(document.querySelector(".tree-node"));
+    ok(tree.state.mouseDown, "Mouse down state is set.");
+    Simulate.mouseUp(document.querySelector(".tree-node"));
+    ok(!tree.state.mouseDown, "Mouse down state is reset.");
+
+    // Unset focused tree state.
+    renderTree({ focused: null });
+    isRenderedTree(document.body.textContent, [
+      "A:false",
+      "-B:false",
+      "--E:false",
+      "---K:false",
+      "---L:false",
+      "--F:false",
+      "--G:false",
+      "-C:false",
+      "--H:false",
+      "--I:false",
+      "-D:false",
+      "--J:false",
+      "M:false",
+      "-N:false",
+      "--O:false",
+    ], "No node should be focused");
+
+    // When tree gets focus while mouse is down, do not set first node as
+    // focused node.
+    Simulate.mouseDown(document.querySelector(".tree-node"));
+    Simulate.focus(treeElem);
+    Simulate.mouseUp(document.querySelector(".tree-node"));
+    await forceRender(tree);
+
+    isRenderedTree(document.body.textContent, [
+      "A:false",
+      "-B:false",
+      "--E:false",
+      "---K:false",
+      "---L:false",
+      "--F:false",
+      "--G:false",
+      "-C:false",
+      "--H:false",
+      "--I:false",
+      "-D:false",
+      "--J:false",
+      "M:false",
+      "-N:false",
+      "--O:false",
+    ], "No node should have been focused");
+
+    // When tree gets focus by means other than mouse, set first node as focused
+    // node if no nodes are focused.
+    Simulate.focus(treeElem);
+    await forceRender(tree);
+
+    isRenderedTree(document.body.textContent, [
+      "A:true",
+      "-B:false",
+      "--E:false",
+      "---K:false",
+      "---L:false",
+      "--F:false",
+      "--G:false",
+      "-C:false",
+      "--H:false",
+      "--I:false",
+      "-D:false",
+      "--J:false",
+      "M:false",
+      "-N:false",
+      "--O:false",
+    ], "A should be focused");
+  } catch (e) {
     ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
   } finally {
     SimpleTest.finish();
   }
-});
+};
 </script>
 </pre>
 </body>