Bug 1544710 - ensure that selected row is always visible within TreeView after update. Clean up scroll into view operations across all uses of TreeView. r=mtigley
☠☠ backed out by f81b6439ae4e ☠ ☠
authorYura Zenevich <yura.zenevich@gmail.com>
Thu, 02 May 2019 04:32:33 +0000
changeset 531078 082c0dba73cbd801b3a17409723e128c7c075009
parent 531077 3e3bc1430b8284c8fe7e0a30b5f6702d5a48e041
child 531079 31bc333e0431801d1252d779905f87e501751e16
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmtigley
bugs1544710
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1544710 - ensure that selected row is always visible within TreeView after update. Clean up scroll into view operations across all uses of TreeView. r=mtigley Differential Revision: https://phabricator.services.mozilla.com/D29342
devtools/client/accessibility/accessibility.css
devtools/client/accessibility/components/AccessibilityRow.js
devtools/client/accessibility/components/AccessibilityTree.js
devtools/client/accessibility/test/browser/browser.ini
devtools/client/accessibility/test/browser/browser_accessibility_tree_audit.js
devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_long.js
devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_reset.js
devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_toolbar.js
devtools/client/accessibility/test/browser/head.js
devtools/client/shared/components/splitter/SplitBox.js
devtools/client/shared/components/tree/TreeRow.js
devtools/client/shared/components/tree/TreeView.js
--- a/devtools/client/accessibility/accessibility.css
+++ b/devtools/client/accessibility/accessibility.css
@@ -7,16 +7,17 @@
   --accessibility-toolbar-height: 24px;
   --accessibility-toolbar-height-tall: 35px;
   --accessibility-toolbar-focus: var(--blue-50);
   --accessibility-toolbar-focus-alpha30: rgba(10, 132, 255, 0.3);
   --accessibility-full-length-minus-splitter: calc(100% - 1px);
   --accessibility-horizontal-padding: 5px;
   --accessibility-horizontal-indent: 20px;
   --accessibility-properties-item-width: calc(100% - var(--accessibility-horizontal-indent));
+  --accessibility-tree-height: calc(100vh - var(--accessibility-toolbar-height) * 2 - 1px);
   --accessibility-arrow-horizontal-padding: 4px;
   --accessibility-tree-row-height: 21px;
   --accessibility-unfocused-tree-focused-node-background: var(--grey-20);
   --accessibility-unfocused-tree-focused-node-twisty-fill: var(--theme-icon-dimmed-color);
   --accessibility-link-color: var(--blue-60);
   --accessibility-link-color-active: var(--blue-70);
   --badge-active-background-color: var(--blue-50);
   --badge-active-border-color: #FFFFFFB3;
@@ -66,26 +67,26 @@ body {
   width: var(--accessibility-horizontal-indent);
 }
 
 .accessible .tree .node.focused .theme-twisty,
 .treeTable .treeRow.selected .theme-twisty {
   fill: var(--theme-selection-color);
 }
 
-.mainFrame .main-panel {
-  flex: 1 1 auto;
-  overflow: auto;
-}
-
 .mainFrame {
   height: 100%;
   color: var(--theme-toolbar-color);
 }
 
+.main-panel {
+  /* To compenstate for 1px splitter between the tree and sidebar. */
+  width: var(--accessibility-full-length-minus-splitter);
+}
+
 .devtools-button,
 .toggle-button {
   cursor: pointer;
 }
 
 .mainFrame .devtools-button.devtools-throbber::before,
 .mainFrame .toggle-button.devtools-throbber::before {
   /* Default for .devtools-throbber is set to 1em which is too big for the
@@ -220,34 +221,40 @@ body {
 }
 
 .description .link:active {
   color: var(--accessibility-link-color-active);
   text-decoration: underline;
 }
 
 /* TreeView Customization */
-.split-box:not(.horz) .main-panel {
-  height: calc(100vh - var(--accessibility-toolbar-height));
+.treeTable thead, .treeTable tbody {
+  display: block;
+}
+
+.treeTable tr {
+  width: 100%;
+  display: table;
 }
 
-.treeTable > thead {
-  position: sticky;
-  top: 0;
-  /* Bug 1466806 - fix expander arrow for expanding treeview rows rendering over the
-     thead */
-  z-index: 1;
+.treeTable tbody {
+  overflow-y: auto;
+}
+
+.split-box:not(.horz) .treeTable tbody {
+  height: var(--accessibility-tree-height);
 }
 
-.split-box:not(.horz) .treeTable {
-  /* To compenstate for 1px splitter between the tree and sidebar. */
-  width: var(--accessibility-full-length-minus-splitter);
+.split-box.horz .treeTable tbody {
+  /* Accessibility tree height depends on the height of the controlled panel
+     (sidebar) when in horz mode and also has an additional separator. */
+  height: calc(var(--accessibility-tree-height) - var(--split-box-controlled-panel-size) - 1px);
 }
 
-.split-box.horz .treeTable {
+.treeTable {
   width: 100%;
 }
 
 .treeTable .treeRow.highlighted:not(.selected) {
   background-color: var(--theme-selection-background-hover);
 }
 
 .treeTable.filtered .treeRow .treeLabelCell {
@@ -313,16 +320,17 @@ body {
 
 .mainFrame .treeTable .treeHeaderRow > .treeHeaderCell:first-child > .treeHeaderCellBox,
 .mainFrame .treeTable .treeHeaderRow > .treeHeaderCell > .treeHeaderCellBox {
   padding: 0;
   padding-inline-start: var(--accessibility-arrow-horizontal-padding);
 }
 
 .mainFrame .treeTable .treeHeaderCell {
+  width: 50%;
   border-bottom: 1px solid var(--theme-splitter-color);
   background: var(--theme-toolbar-background);
   font: message-box;
   font-size: var(--accessibility-font-size);
   height: var(--accessibility-toolbar-height);
   color: var(--theme-toolbar-color);
 }
 
--- a/devtools/client/accessibility/components/AccessibilityRow.js
+++ b/devtools/client/accessibility/components/AccessibilityRow.js
@@ -25,16 +25,18 @@ const { VALUE_FLASHING_DURATION, VALUE_H
 const { updateDetails } = require("../actions/details");
 const { unhighlight } = require("../actions/accessibles");
 
 const { L10N } = require("../utils/l10n");
 
 loader.lazyRequireGetter(this, "Menu", "devtools/client/framework/menu");
 loader.lazyRequireGetter(this, "MenuItem", "devtools/client/framework/menu-item");
 
+const { scrollIntoView } = require("devtools/client/shared/scroll");
+
 const JSON_URL_PREFIX = "data:application/json;charset=UTF-8,";
 
 const TELEMETRY_ACCESSIBLE_CONTEXT_MENU_OPENED =
   "devtools.accessibility.accessible_context_menu_opened";
 const TELEMETRY_ACCESSIBLE_CONTEXT_MENU_ITEM_ACTIVATED =
   "devtools.accessibility.accessible_context_menu_item_activated";
 
 class HighlightableTreeRowClass extends TreeRow {
@@ -66,17 +68,17 @@ class AccessibilityRow extends Component
       walker: PropTypes.object,
     };
   }
 
   componentDidMount() {
     const { selected, object } = this.props.member;
     if (selected) {
       this.unhighlight();
-      this.updateAndScrollIntoViewIfNeeded();
+      this.update();
       this.highlight(object, { duration: VALUE_HIGHLIGHT_DURATION });
     }
 
     if (this.props.highlighted) {
       this.scrollIntoView();
     }
   }
 
@@ -84,17 +86,17 @@ class AccessibilityRow extends Component
    * Update accessible object details that are going to be rendered inside the
    * accessible panel sidebar.
    */
   componentDidUpdate(prevProps) {
     const { selected, object } = this.props.member;
     // If row is selected, update corresponding accessible details.
     if (!prevProps.member.selected && selected) {
       this.unhighlight();
-      this.updateAndScrollIntoViewIfNeeded();
+      this.update();
       this.highlight(object, { duration: VALUE_HIGHLIGHT_DURATION });
     }
 
     if (this.props.highlighted) {
       this.scrollIntoView();
     }
 
     if (!selected && prevProps.member.value !== this.props.member.value) {
@@ -105,27 +107,26 @@ class AccessibilityRow extends Component
   scrollIntoView() {
     const row = findDOMNode(this);
     // Row might not be rendered in the DOM tree if it is filtered out during
     // audit.
     if (!row) {
       return;
     }
 
-    row.scrollIntoView({ block: "center" });
+    scrollIntoView(row, { container: document.querySelector(".treeTable > tbody") });
   }
 
-  updateAndScrollIntoViewIfNeeded() {
+  update() {
     const { dispatch, member: { object }, supports } = this.props;
     if (!gToolbox || !object.actorID) {
       return;
     }
 
     dispatch(updateDetails(gToolbox.walker, object, supports));
-    this.scrollIntoView();
     window.emit(EVENTS.NEW_ACCESSIBLE_FRONT_SELECTED, object);
   }
 
   flashValue() {
     const row = findDOMNode(this);
     // Row might not be rendered in the DOM tree if it is filtered out during
     // audit.
     if (!row) {
--- a/devtools/client/accessibility/components/AccessibilityTree.js
+++ b/devtools/client/accessibility/components/AccessibilityTree.js
@@ -186,17 +186,20 @@ class AccessibilityTree extends Componen
         label: L10N.getStr("accessibility.treeName"),
         header: true,
         expandedNodes: expanded,
         selected,
         onClickRow(nodePath, event) {
           if (event.target.classList.contains("theme-twisty")) {
             this.toggle(nodePath);
           }
-          this.selectRow(event.currentTarget);
+
+          this.selectRow(
+            this.rows.find(row => row.props.member.path === nodePath),
+            { preventAutoScroll: true });
         },
         onContextMenuTree: hasContextMenu && function(e) {
           // If context menu event is triggered on (or bubbled to) the TreeView, it was
           // done via keyboard. Open context menu for currently selected row.
           let row = this.getSelectedRow();
           if (!row) {
             return;
           }
--- a/devtools/client/accessibility/test/browser/browser.ini
+++ b/devtools/client/accessibility/test/browser/browser.ini
@@ -19,14 +19,15 @@ skip-if = (os == 'win' && processor == '
 skip-if = (os == 'win' && processor == 'aarch64') # bug 1533534
 [browser_accessibility_panel_highlighter.js]
 [browser_accessibility_panel_highlighter_multi_tab.js]
 skip-if = (os == 'linux' && debug && bits == 64) # Bug 1511247
 [browser_accessibility_relation_navigation.js]
 [browser_accessibility_reload.js]
 [browser_accessibility_sidebar_checks.js]
 [browser_accessibility_sidebar.js]
+[browser_accessibility_tree_audit_long.js]
 [browser_accessibility_tree_audit_reset.js]
 [browser_accessibility_tree_audit_toolbar.js]
 [browser_accessibility_tree_audit.js]
 [browser_accessibility_tree_contrast.js]
 [browser_accessibility_tree_nagivation.js]
 [browser_accessibility_tree.js]
--- a/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit.js
+++ b/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit.js
@@ -60,16 +60,17 @@ const tests = [{
     tree: [{
       role: "text leaf",
       name: `"Top level header "contrast`,
       badges: [ "contrast" ],
     }, {
       role: "text leaf",
       name: `"Second level header "contrast`,
       badges: [ "contrast" ],
+      selected: true,
     }],
   },
 }, {
   desc: "Click on the badge again.",
   setup: async ({ doc }) => {
     await toggleBadge(doc, 0, 0);
   },
   expected: {
@@ -85,16 +86,17 @@ const tests = [{
       badges: [ "contrast" ],
     }, {
       role: "heading",
       name: `"Second level header"`,
     }, {
       role: "text leaf",
       name: `"Second level header "contrast`,
       badges: [ "contrast" ],
+      selected: true,
     }],
   },
 }];
 
 /**
  * Simple test that checks content of the Accessibility panel tree when one of
  * the tree rows has a "contrast" badge and auditing is activated via badge.
  */
new file mode 100644
--- /dev/null
+++ b/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_long.js
@@ -0,0 +1,94 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/* global toggleFilter */
+
+const header = "<h1 style=\"color:rgba(255,0,0,0.1); " +
+               "background-color:rgba(255,255,255,1);\">header</h1>";
+
+const TEST_URI = `<html>
+  <head>
+    <meta charset="utf-8"/>
+    <title>Accessibility Panel Test</title>
+  </head>
+  <body>
+    ${header.repeat(20)}
+  </body>
+</html>`;
+
+const docRow = {
+  role: "document",
+  name: `"Accessibility Panel Test"`,
+};
+const headingRow = {
+  role: "heading",
+  name: `"header"`,
+};
+const textLeafRow = {
+  role: "text leaf",
+  name: `"header"contrast`,
+  badges: [ "contrast" ],
+};
+const audit = new Array(20).fill(textLeafRow);
+
+const auditInitial = audit.map(check => ({ ...check }));
+auditInitial[0].selected = true;
+
+const auditSecondLastSelected = audit.map(check => ({ ...check }));
+auditSecondLastSelected[18].selected = true;
+
+const resetAfterAudit = [docRow];
+for (let i = 0; i < 20; i++) {
+  resetAfterAudit.push(headingRow);
+  resetAfterAudit.push({ ...textLeafRow, selected: i === 18 });
+}
+
+/**
+* Test data has the format of:
+* {
+*   desc     {String}    description for better logging
+*   setup    {Function}  An optional setup that needs to be performed before
+*                        the state of the tree and the sidebar can be checked.
+*   expected {JSON}      An expected states for the tree and the sidebar.
+* }
+*/
+const tests = [{
+  desc: "Check initial state.",
+  expected: {
+    tree: [{ ...docRow, selected: true }],
+  },
+}, {
+  desc: "Run an audit from a11y panel toolbar by activating a filter.",
+  setup: async ({ doc }) => {
+    await toggleFilter(doc, 0);
+  },
+  expected: {
+    tree: auditInitial,
+  },
+}, {
+  desc: "Select a row that is guaranteed to have to be scrolled into view.",
+  setup: async ({ doc }) => {
+    await selectRow(doc, 18);
+  },
+  expected: {
+    tree: auditSecondLastSelected,
+  },
+}, {
+  desc: "Click on the filter again.",
+  setup: async ({ doc }) => {
+    await toggleFilter(doc, 0);
+  },
+  expected: {
+    tree: resetAfterAudit,
+  },
+}];
+
+/**
+* Simple test that checks content of the Accessibility panel tree when the
+* audit is activated via the panel's toolbar and the selection persists when
+* the filter is toggled off.
+*/
+addA11yPanelTestsTask(tests, TEST_URI,
+  "Test Accessibility panel tree with persistent selected row.");
--- a/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_reset.js
+++ b/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_reset.js
@@ -30,43 +30,46 @@ const TEST_URI = `<html>
 * }
 */
 const tests = [{
   desc: "Check initial state.",
   expected: {
     tree: [{
       role: "document",
       name: `"Accessibility Panel Test"`,
+      selected: true,
     }],
   },
 }, {
   desc: "Run an audit from a11y panel toolbar by activating a filter.",
   setup: async ({ doc }) => {
     await toggleFilter(doc, 0);
   },
   expected: {
     tree: [{
       role: "text leaf",
       name: `"Top level header "contrast`,
       badges: [ "contrast" ],
+      selected: true,
     }, {
       role: "text leaf",
       name: `"Second level header "contrast`,
       badges: [ "contrast" ],
     }],
   },
 }, {
   desc: "Select an accessible object.",
   setup: async (env) => {
     await selectAccessibleForNode(env, "body");
   },
   expected: {
     tree: [{
       role: "document",
       name: `"Accessibility Panel Test"`,
+      selected: true,
     }, {
       role: "heading",
       name: `"Top level header"`,
     }, {
       role: "text leaf",
       name: `"Top level header "contrast`,
       badges: [ "contrast" ],
     }, {
--- a/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_toolbar.js
+++ b/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_toolbar.js
@@ -30,28 +30,30 @@ const TEST_URI = `<html>
  * }
  */
 const tests = [{
   desc: "Check initial state.",
   expected: {
     tree: [{
       role: "document",
       name: `"Accessibility Panel Test"`,
+      selected: true,
     }],
   },
 }, {
   desc: "Run an audit from a11y panel toolbar by activating a filter.",
   setup: async ({ doc }) => {
     await toggleFilter(doc, 0);
   },
   expected: {
     tree: [{
       role: "text leaf",
       name: `"Top level header "contrast`,
       badges: [ "contrast" ],
+      selected: true,
     }, {
       role: "text leaf",
       name: `"Second level header "contrast`,
       badges: [ "contrast" ],
     }],
   },
 }, {
   desc: "Click on the filter again.",
@@ -64,16 +66,17 @@ const tests = [{
       name: `"Accessibility Panel Test"`,
     }, {
       role: "heading",
       name: `"Top level header"`,
     }, {
       role: "text leaf",
       name: `"Top level header "contrast`,
       badges: [ "contrast" ],
+      selected: true,
     }, {
       role: "heading",
       name: `"Second level header"`,
     }, {
       role: "text leaf",
       name: `"Second level header "contrast`,
       badges: [ "contrast" ],
     }],
--- a/devtools/client/accessibility/test/browser/head.js
+++ b/devtools/client/accessibility/test/browser/head.js
@@ -172,28 +172,90 @@ async function initAccessibilityPanel(ta
  */
 function compareBadges(badges, expected = []) {
   const badgeEls = badges ? [...badges.querySelectorAll(".badge")] : [];
   return badgeEls.length === expected.length &&
          badgeEls.every((badge, i) => badge.textContent === expected[i]);
 }
 
 /**
+ * Find an ancestor that is scrolled for a given DOMNode.
+ *
+ * @param {DOMNode} node
+ *        DOMNode that to find an ancestor for that is scrolled.
+ */
+function closestScrolledParent(node) {
+  if (node == null) {
+    return null;
+  }
+
+  if (node.scrollHeight > node.clientHeight) {
+    return node;
+  }
+
+  return closestScrolledParent(node.parentNode);
+}
+
+/**
+ * Check if a given element is visible to the user and is not scrolled off
+ * because of the overflow.
+ *
+ * @param   {Element} element
+ *          Element to be checked whether it is visible and is not scrolled off.
+ *
+ * @returns {Boolean}
+ *          True if the element is visible.
+ */
+function isVisible(element) {
+  const { top, bottom } = element.getBoundingClientRect();
+  const scrolledParent = closestScrolledParent(element.parentNode);
+  const scrolledParentRect = scrolledParent ? scrolledParent.getBoundingClientRect() :
+                                              null;
+  return !scrolledParent ||
+         (top >= scrolledParentRect.top && bottom <= scrolledParentRect.bottom);
+}
+
+/**
+ * Check selected styling and visibility for a given row in the accessibility
+ * tree.
+ * @param   {DOMNode} row
+ *          DOMNode for a given accessibility row.
+ * @param   {Boolean} expected
+ *          Expected selected state.
+ *
+ * @returns {Boolean}
+ *          True if visibility and styling matches expected selected state.
+ */
+function checkSelected(row, expected) {
+  if (!expected) {
+    return true;
+  }
+
+  if (row.classList.contains("selected") !== expected) {
+    return false;
+  }
+
+  return isVisible(row);
+}
+
+/**
  * Check the state of the accessibility tree.
  * @param  {document} doc       panel documnent.
  * @param  {Array}    expected  an array that represents an expected row list.
  */
 async function checkTreeState(doc, expected) {
   info("Checking tree state.");
   const hasExpectedStructure = await BrowserTestUtils.waitForCondition(() =>
-    [...doc.querySelectorAll(".treeRow")].every((row, i) =>
-      row.querySelector(".treeLabelCell").textContent === expected[i].role &&
-      row.querySelector(".treeValueCell").textContent === expected[i].name &&
-      compareBadges(row.querySelector(".badges"), expected[i].badges)),
-    "Wait for the right tree update.");
+    [...doc.querySelectorAll(".treeRow")].every((row, i) => {
+      const { role, name, badges, selected } = expected[i];
+      return row.querySelector(".treeLabelCell").textContent === role &&
+        row.querySelector(".treeValueCell").textContent === name &&
+        compareBadges(row.querySelector(".badges"), badges) &&
+        checkSelected(row, selected);
+    }), "Wait for the right tree update.");
 
   ok(hasExpectedStructure, "Tree structure is correct.");
 }
 
 /**
  * Check if relations object matches what is expected. Note: targets are matched by their
  * name and role.
  * @param  {Object} relations  Relations to test.
--- a/devtools/client/shared/components/splitter/SplitBox.js
+++ b/devtools/client/shared/components/splitter/SplitBox.js
@@ -198,17 +198,23 @@ class SplitBox extends Component {
   }
 
   // Rendering
 
   render() {
     const { endPanelControl, splitterSize, vert } = this.state;
     const { startPanel, endPanel, minSize, maxSize } = this.props;
 
-    const style = Object.assign({}, this.props.style);
+    const style = Object.assign({
+      // Set the size of the controlled panel (height or width depending on the
+      // current state). This can be used to help with styling of dependent
+      // panels.
+      "--split-box-controlled-panel-size":
+        `${vert ? this.state.width : this.state.height}`,
+    }, this.props.style);
 
     // Calculate class names list.
     let classNames = ["split-box"];
     classNames.push(vert ? "vert" : "horz");
     if (this.props.className) {
       classNames = classNames.concat(this.props.className.split(" "));
     }
 
--- a/devtools/client/shared/components/tree/TreeRow.js
+++ b/devtools/client/shared/components/tree/TreeRow.js
@@ -16,18 +16,16 @@ define(function(require, exports, module
   const dom = require("devtools/client/shared/vendor/react-dom-factories");
   const { findDOMNode } = require("devtools/client/shared/vendor/react-dom");
   const { tr } = dom;
 
   // Tree
   const TreeCell = createFactory(require("./TreeCell"));
   const LabelCell = createFactory(require("./LabelCell"));
 
-  // Scroll
-  const { scrollIntoViewIfNeeded } = require("devtools/client/shared/scroll");
   const { focusableSelector } = require("devtools/client/shared/focus");
 
   const UPDATE_ON_PROPS = [
     "name",
     "open",
     "value",
     "loading",
     "selected",
@@ -118,27 +116,16 @@ define(function(require, exports, module
         if (nextProps.member[prop] != this.props.member[prop]) {
           return true;
         }
       }
 
       return false;
     }
 
-    componentDidUpdate() {
-      if (this.props.member.selected) {
-        const row = findDOMNode(this);
-        // Because this is called asynchronously, context window might be
-        // already gone.
-        if (row.ownerDocument.defaultView) {
-          scrollIntoViewIfNeeded(row);
-        }
-      }
-    }
-
     componentWillUnmount() {
       this.observer.disconnect();
       this.observer = null;
     }
 
     /**
      * Makes sure that none of the focusable elements inside the row container
      * are tabbable if the row is not active. If the row is active and focus
--- a/devtools/client/shared/components/tree/TreeView.js
+++ b/devtools/client/shared/components/tree/TreeView.js
@@ -17,16 +17,18 @@ define(function(require, exports, module
   const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
   const dom = require("devtools/client/shared/vendor/react-dom-factories");
 
   // Reps
   const { ObjectProvider } = require("./ObjectProvider");
   const TreeRow = createFactory(require("./TreeRow"));
   const TreeHeader = createFactory(require("./TreeHeader"));
 
+  const { scrollIntoView } = require("devtools/client/shared/scroll");
+
   const SUPPORTED_KEYS = [
     "ArrowUp",
     "ArrowDown",
     "ArrowLeft",
     "ArrowRight",
     "End",
     "Home",
     "Enter",
@@ -235,17 +237,20 @@ define(function(require, exports, module
 
       this.setState(Object.assign({}, this.state, state));
     }
 
     componentDidUpdate() {
       const selected = this.getSelectedRow();
       if (!selected && this.rows.length > 0) {
         this.selectRow(this.rows[
-          Math.min(this.state.lastSelectedIndex, this.rows.length - 1)]);
+          Math.min(this.state.lastSelectedIndex, this.rows.length - 1)
+        ], { alignTo: "top" });
+      } else {
+        this._scrollIntoView(this.getSelectedRow());
       }
     }
 
     // Node expand/collapse
 
     toggle(nodePath) {
       const nodes = this.state.expandedNodes;
       if (this.isExpanded(nodePath)) {
@@ -286,49 +291,44 @@ define(function(require, exports, module
           break;
         case "ArrowLeft":
           if (row && row.props.member.open) {
             this.toggle(this.state.selected);
           } else {
             const parentRow = this.rows.slice(0, index).reverse().find(
               r => r.props.member.level < row.props.member.level);
             if (parentRow) {
-              this.selectRow(parentRow);
+              this.selectRow(parentRow, { alignTo: "top" });
             }
           }
           break;
         case "ArrowDown":
           const nextRow = this.rows[index + 1];
           if (nextRow) {
-            this.selectRow(nextRow);
+            this.selectRow(nextRow, { alignTo: "bottom" });
           }
           break;
         case "ArrowUp":
           const previousRow = this.rows[index - 1];
           if (previousRow) {
-            this.selectRow(previousRow);
+            this.selectRow(previousRow, { alignTo: "top" });
           }
           break;
         case "Home":
           const firstRow = this.rows[0];
 
           if (firstRow) {
-            // Due to the styling, the first row is sometimes overlapped by
-            // the table head. So we want to force the tree to scroll to the very top.
-            this.selectRow(firstRow, {
-              block: "end",
-              inline: "nearest",
-            });
+            this.selectRow(firstRow, { alignTo: "top" });
           }
           break;
 
         case "End":
           const lastRow = this.rows[this.rows.length - 1];
           if (lastRow) {
-            this.selectRow(lastRow);
+            this.selectRow(lastRow, { alignTo: "bottom" });
           }
           break;
 
         case "Enter":
         case " ":
           // On space or enter make selected row active. This means keyboard
           // focus handling is passed on to the tree row itself.
           if (this.treeRef.current === document.activeElement) {
@@ -362,17 +362,20 @@ define(function(require, exports, module
         return;
       }
 
       event.stopPropagation();
       const cell = event.target.closest("td");
       if (cell && cell.classList.contains("treeLabelCell")) {
         this.toggle(nodePath);
       }
-      this.selectRow(event.currentTarget);
+
+      this.selectRow(
+        this.rows.find(row => row.props.member.path === nodePath),
+        { preventAutoScroll: true });
     }
 
     onContextMenu(member, event) {
       const onContextMenuRow = this.props.onContextMenuRow;
       if (onContextMenuRow) {
         onContextMenuRow.call(this, member, event);
       }
     }
@@ -389,37 +392,58 @@ define(function(require, exports, module
       if (!row) {
         // If selected row is not found, return index of the first row.
         return 0;
       }
 
       return this.rows.indexOf(row);
     }
 
-    selectRow(row, scrollOptions = {block: "nearest"}) {
-      row = findDOMNode(row);
+    _scrollIntoView(row, options = {}) {
+      if (!this.treeRef.current || !row) {
+        return;
+      }
+
+      const { props: { member: { path } = {} } = {} } = row;
+      if (!path) {
+        return;
+      }
 
-      if (this.state.selected === row.id) {
-        row.scrollIntoView(scrollOptions);
+      const element = document.getElementById(path);
+      if (!element) {
+        return;
+      }
+
+      // For usability/accessibility purposes we do not want to scroll the
+      // thead. TreeView will scroll relative to the tbody.
+      const container = this.treeRef.current.tBodies[0];
+      scrollIntoView(element, { ...options, container });
+    }
+
+    selectRow(row, options) {
+      const { props: { member: { path } = {} } = {} } = row;
+      if (this.isSelected(path)) {
         return;
       }
 
       if (this.state.active != null) {
         if (this.treeRef.current !== document.activeElement) {
           this.treeRef.current.focus();
         }
       }
 
+      if (!options.preventAutoScroll) {
+        this._scrollIntoView(row, options);
+      }
+
       this.setState({
         ...this.state,
-        selected: row.id,
+        selected: path,
         active: null,
       });
-
-      row.scrollIntoView(scrollOptions);
     }
 
     activateRow(active) {
       this.setState({
         ...this.state,
         active,
       });
     }