Bug 1415919 - Improve how JSON Viewer scrolls selected row into view. draft
authorOriol Brufau <oriol-bugzilla@hotmail.com>
Thu, 09 Nov 2017 19:23:16 +0100
changeset 696360 ae77be2bc066563b5468ca1acb7d3f8bf0110365
parent 695627 85c21448545e20ddb5b45009ea9d1afc2319d417
child 739837 749beba20e325e63b4b47dc17767a70ea1a3e935
push id88690
push userbmo:oriol-bugzilla@hotmail.com
push dateFri, 10 Nov 2017 14:49:06 +0000
bugs1415919
milestone58.0a1
Bug 1415919 - Improve how JSON Viewer scrolls selected row into view. MozReview-Commit-ID: 2OV9IU6dmcp
devtools/client/jsonview/test/browser_jsonview_row_selection.js
devtools/client/shared/components/tree/TreeView.js
--- a/devtools/client/jsonview/test/browser_jsonview_row_selection.js
+++ b/devtools/client/jsonview/test/browser_jsonview_row_selection.js
@@ -57,12 +57,61 @@ add_task(async function () {
     await assertRowSelected(i);
   }
 
   // Now synthetize the keyup, this shouldn't change selected row.
   await BrowserTestUtils.synthesizeKey("VK_DOWN", {type: "keyup"}, tab.linkedBrowser);
   await assertRowSelected(numRows - 1);
 });
 
+add_task(async function () {
+  info("Test 3 JSON row selection started");
+
+  // Create a JSON with a row taller than the panel.
+  let json = JSON.stringify([0, "a ".repeat(1e4), 1]);
+  await addJsonViewTab("data:application/json," + encodeURI(json));
+
+  is(await getElementCount(".treeRow"), 3, "Got the expected number of rows.");
+  await assertRowSelected(null);
+  await evalInContent("var scroller = $('.jsonPanelBox .panelContent')");
+  await evalInContent("var row = $('.treeRow:nth-child(2)')");
+  ok(await evalInContent("scroller.clientHeight < row.clientHeight"),
+     "The row is taller than the scroller.");
+  is(await evalInContent("scroller.scrollTop"), 0, "Initially scrolled to the top.");
+
+  // Select the tall row.
+  await evalInContent("row.click()");
+  await assertRowSelected(2);
+  is(await evalInContent("scroller.scrollTop"), await evalInContent("row.offsetTop"),
+     "Scrolled to the top of the row.");
+
+  // Select the last row.
+  await evalInContent("$('.treeRow:last-child').click()");
+  await assertRowSelected(3);
+  is(await evalInContent("scroller.scrollTop + scroller.offsetHeight"),
+     await evalInContent("scroller.scrollHeight"), "Scrolled to the bottom.");
+
+  // Select the tall row.
+  await evalInContent("row.click()");
+  await assertRowSelected(2);
+  is(await evalInContent("scroller.scrollTop + scroller.offsetHeight"),
+     await evalInContent("row.offsetTop + row.offsetHeight"),
+     "Scrolled to the bottom of the row.");
+
+  // Scroll up a bit, so that both the top and bottom of the row are not visible.
+  let scroll = await evalInContent(
+    "scroller.scrollTop = Math.ceil((scroller.scrollTop + row.offsetTop) / 2)");
+  ok(await evalInContent("scroller.scrollTop > row.offsetTop"),
+     "The top of the row is not visible.");
+  ok(await evalInContent("scroller.scrollTop + scroller.offsetHeight")
+     < await evalInContent("row.offsetTop + row.offsetHeight"),
+     "The bottom of the row is not visible.");
+
+  // Select the tall row.
+  await evalInContent("row.click()");
+  await assertRowSelected(2);
+  is(await evalInContent("scroller.scrollTop"), scroll, "Scroll did not change");
+});
+
 async function assertRowSelected(rowNum) {
   let idx = evalInContent("[].indexOf.call($$('.treeRow'), $('.treeRow.selected'))");
   is(await idx + 1, +rowNum, `${rowNum ? "The row #" + rowNum : "No row"} is selected.`);
 }
--- a/devtools/client/shared/components/tree/TreeView.js
+++ b/devtools/client/shared/components/tree/TreeView.js
@@ -282,17 +282,29 @@ define(function (require, exports, modul
       return rows.find(row => this.isSelected(row.props.member.path));
     }
 
     selectRow(row) {
       row = findDOMNode(row);
       this.setState(Object.assign({}, this.state, {
         selected: row.id
       }));
-      row.scrollIntoView({block: "nearest"});
+
+      // If the top or bottom side of the row is not visible and there is available space
+      // beyond the opposite one, then attempt to scroll the hidden side into view, but
+      // without hiding the visible side.
+      let scroller = scrollContainer(row);
+      if (!scroller) {
+        return;
+      }
+      let scrollToTop = row.offsetTop;
+      let scrollToBottom = scrollToTop + row.offsetHeight - scroller.offsetHeight;
+      let max = Math.max(scrollToTop, scrollToBottom);
+      let min = Math.min(scrollToTop, scrollToBottom);
+      scroller.scrollTop = Math.max(min, Math.min(max, scroller.scrollTop));
     }
 
     isSelected(nodePath) {
       return nodePath === this.state.selected;
     }
 
     // Filtering & Sorting
 
@@ -497,11 +509,23 @@ define(function (require, exports, modul
     // The default column is usually the first one.
     return [{id: "default"}, ...columns];
   }
 
   function isLongString(value) {
     return typeof value == "string" && value.length > 50;
   }
 
+  function scrollContainer(element) {
+    let parent = element.parentElement;
+    let window = element.ownerDocument.defaultView;
+    if (!parent || !window) {
+      return null;
+    }
+    if (window.getComputedStyle(parent).overflowY != "visible") {
+      return parent;
+    }
+    return scrollContainer(parent);
+  }
+
   // Exports from this module
   module.exports = TreeView;
 });