Bug 1415728 - Change JSON Viewer's selected row on keydown instead of on keyup. r=Honza
authorOriol Brufau <oriol-bugzilla@hotmail.com>
Thu, 09 Nov 2017 01:16:36 +0100
changeset 444328 07b7f8df735ef181ac18371da35df93393e2aefe
parent 444327 4c25a8edd5db8f8f2bfbbe51f1050ef48d495659
child 444329 c6dc3b5ff98968a10dcd6356d1d10fe06f5b0f01
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1415728
milestone58.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 1415728 - Change JSON Viewer's selected row on keydown instead of on keyup. r=Honza MozReview-Commit-ID: 3qXc1VxM2Rc
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
@@ -1,44 +1,68 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-add_task(function* () {
-  info("Test JSON row selection started");
+add_task(async function () {
+  info("Test 1 JSON row selection started");
 
   // Create a tall JSON so that there is a scrollbar.
   let numRows = 1e3;
   let json = JSON.stringify(Array(numRows).fill().map((_, i) => i));
-  let tab = yield addJsonViewTab("data:application/json," + json);
+  let tab = await addJsonViewTab("data:application/json," + json);
 
-  is(yield getElementCount(".treeRow"), numRows, "Got the expected number of rows.");
-  yield assertRowSelected(null);
-  yield evalInContent("var scroller = $('.jsonPanelBox .panelContent')");
-  ok(yield evalInContent("scroller.clientHeight < scroller.scrollHeight"),
+  is(await getElementCount(".treeRow"), numRows, "Got the expected number of rows.");
+  await assertRowSelected(null);
+  await evalInContent("var scroller = $('.jsonPanelBox .panelContent')");
+  ok(await evalInContent("scroller.clientHeight < scroller.scrollHeight"),
      "There is a scrollbar.");
-  is(yield evalInContent("scroller.scrollTop"), 0, "Initially scrolled to the top.");
+  is(await evalInContent("scroller.scrollTop"), 0, "Initially scrolled to the top.");
 
   // Click to select last row.
-  yield evalInContent("$('.treeRow:last-child').click()");
-  yield assertRowSelected(numRows);
-  is(yield evalInContent("scroller.scrollTop + scroller.clientHeight"),
-     yield evalInContent("scroller.scrollHeight"), "Scrolled to the bottom.");
+  await evalInContent("$('.treeRow:last-child').click()");
+  await assertRowSelected(numRows);
+  is(await evalInContent("scroller.scrollTop + scroller.clientHeight"),
+     await evalInContent("scroller.scrollHeight"), "Scrolled to the bottom.");
 
   // Click to select 2nd row.
-  yield evalInContent("$('.treeRow:nth-child(2)').click()");
-  yield assertRowSelected(2);
-  ok(yield evalInContent("scroller.scrollTop > 0"), "Not scrolled to the top.");
+  await evalInContent("$('.treeRow:nth-child(2)').click()");
+  await assertRowSelected(2);
+  ok(await evalInContent("scroller.scrollTop > 0"), "Not scrolled to the top.");
 
   // Synthetize up arrow key to select first row.
-  yield evalInContent("$('.treeTable').focus()");
-  yield BrowserTestUtils.synthesizeKey("VK_UP", {}, tab.linkedBrowser);
-  yield assertRowSelected(1);
-  is(yield evalInContent("scroller.scrollTop"), 0, "Scrolled to the top.");
+  await evalInContent("$('.treeTable').focus()");
+  await BrowserTestUtils.synthesizeKey("VK_UP", {}, tab.linkedBrowser);
+  await assertRowSelected(1);
+  is(await evalInContent("scroller.scrollTop"), 0, "Scrolled to the top.");
+});
+
+add_task(async function () {
+  info("Test 2 JSON row selection started");
+
+  let numRows = 4;
+  let tab = await addJsonViewTab("data:application/json,[0,1,2,3]");
+
+  is(await getElementCount(".treeRow"), numRows, "Got the expected number of rows.");
+  await assertRowSelected(null);
+
+  // Click to select first row.
+  await clickJsonNode(".treeRow:first-child");
+  await assertRowSelected(1);
+
+  // Synthetize multiple down arrow keydowns to select following rows.
+  for (let i = 2; i < numRows; ++i) {
+    await BrowserTestUtils.synthesizeKey("VK_DOWN", {type: "keydown"}, tab.linkedBrowser);
+    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);
 });
 
 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
@@ -125,17 +125,16 @@ define(function (require, exports, modul
         expandedNodes: props.expandedNodes,
         columns: ensureDefaultColumn(props.columns),
         selected: null
       };
 
       this.toggle = this.toggle.bind(this);
       this.isExpanded = this.isExpanded.bind(this);
       this.onKeyDown = this.onKeyDown.bind(this);
-      this.onKeyUp = this.onKeyUp.bind(this);
       this.onClickRow = this.onClickRow.bind(this);
       this.getSelectedRow = this.getSelectedRow.bind(this);
       this.selectRow = this.selectRow.bind(this);
       this.isSelected = this.isSelected.bind(this);
       this.onFilter = this.onFilter.bind(this);
       this.onSort = this.onSort.bind(this);
       this.getMembers = this.getMembers.bind(this);
       this.renderRows = this.renderRows.bind(this);
@@ -223,23 +222,20 @@ define(function (require, exports, modul
 
     isExpanded(nodePath) {
       return this.state.expandedNodes.has(nodePath);
     }
 
     // Event Handlers
 
     onKeyDown(event) {
-      if (["ArrowUp", "ArrowDown", "ArrowLeft", "ArrowRight"].includes(
-        event.key)) {
-        event.preventDefault();
+      if (!["ArrowUp", "ArrowDown", "ArrowLeft", "ArrowRight"].includes(event.key)) {
+        return;
       }
-    }
 
-    onKeyUp(event) {
       let row = this.getSelectedRow(this.rows);
       if (!row) {
         return;
       }
 
       let index = this.rows.indexOf(row);
       switch (event.key) {
         case "ArrowRight":
@@ -260,18 +256,16 @@ define(function (require, exports, modul
           }
           break;
         case "ArrowUp":
           let previousRow = this.rows[index - 1];
           if (previousRow) {
             this.selectRow(previousRow);
           }
           break;
-        default:
-          return;
       }
 
       event.preventDefault();
     }
 
     onClickRow(nodePath, event) {
       event.stopPropagation();
       let cell = event.target.closest("td");
@@ -466,17 +460,16 @@ define(function (require, exports, modul
       });
 
       return (
         dom.table({
           className: classNames.join(" "),
           role: "tree",
           tabIndex: 0,
           onKeyDown: this.onKeyDown,
-          onKeyUp: this.onKeyUp,
           "aria-label": this.props.label || "",
           "aria-activedescendant": this.state.selected,
           cellPadding: 0,
           cellSpacing: 0},
           TreeHeader(props),
           dom.tbody({
             role: "presentation"
           }, rows)