Bug 1415728 - Change JSON Viewer's selected row on keydown instead of on keyup. draft
authorOriol Brufau <oriol-bugzilla@hotmail.com>
Thu, 09 Nov 2017 01:16:36 +0100
changeset 695627 85c21448545e20ddb5b45009ea9d1afc2319d417
parent 694565 40df5dd35fdb7ce3652fe4448ac8961c075c928e
child 695718 15e20061039554c50b646c45a8b0af7cf5057067
child 695796 e144f8ef9b4abb421f908a2d4f3db680fe639e3f
child 695898 8a851b7bf2f23dac865987a7cf951919ea7e590e
child 696360 ae77be2bc066563b5468ca1acb7d3f8bf0110365
push id88484
push userbmo:oriol-bugzilla@hotmail.com
push dateThu, 09 Nov 2017 15:06:17 +0000
bugs1415728
milestone58.0a1
Bug 1415728 - Change JSON Viewer's selected row on keydown instead of on keyup. 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)