Bug 1529680 - [release 127] Fixing source selection with the keyboard (fixes #7874) (#7874) (#7992). r=dwalsh
authorYura Zenevich <yura.zenevich@gmail.com>
Mon, 25 Feb 2019 10:01:55 -0500
changeset 518968 065b2e8fef2bd8d77a18e41ddc6ca02402278f7a
parent 518967 6b5b546aac14484f94eecb8a776460357f5a9cea
child 518969 4f8f76780ecab209576d28700d0813622f96c2e2
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdwalsh
bugs1529680
milestone67.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 1529680 - [release 127] Fixing source selection with the keyboard (fixes #7874) (#7874) (#7992). r=dwalsh
devtools/client/debugger/new/src/components/PrimaryPanes/SourcesTree.js
devtools/client/debugger/new/src/components/PrimaryPanes/tests/SourcesTree.spec.js
devtools/client/debugger/new/src/components/PrimaryPanes/tests/__snapshots__/SourcesTree.spec.js.snap
--- a/devtools/client/debugger/new/src/components/PrimaryPanes/SourcesTree.js
+++ b/devtools/client/debugger/new/src/components/PrimaryPanes/SourcesTree.js
@@ -159,16 +159,20 @@ class SourcesTree extends Component<Prop
       this.props.selectSource(item.contents.id);
     }
   };
 
   onFocus = (item: TreeNode) => {
     this.props.focusItem({ thread: this.props.thread, item });
   };
 
+  onActivate = (item: TreeNode) => {
+    this.selectItem(item);
+  };
+
   // NOTE: we get the source from sources because item.contents is cached
   getSource(item: TreeNode): ?Source {
     const source = getSourceFromNode(item);
     if (source) {
       return this.props.sources[source.id];
     }
 
     return null;
@@ -189,23 +193,16 @@ class SourcesTree extends Component<Prop
   onExpand = (item: Item, expandedState: Set<string>) => {
     this.props.setExpandedState(this.props.thread, expandedState);
   };
 
   onCollapse = (item: Item, expandedState: Set<string>) => {
     this.props.setExpandedState(this.props.thread, expandedState);
   };
 
-  onKeyDown = (e: KeyboardEvent) => {
-    const { focused } = this.props;
-    if (e.keyCode === 13 && focused) {
-      this.selectItem(focused);
-    }
-  };
-
   isEmpty() {
     const { sourceTree } = this.state;
     return sourceTree.contents.length === 0;
   }
 
   renderEmptyElement(message) {
     return (
       <div key="empty" className="no-sources-message">
@@ -275,16 +272,17 @@ class SourcesTree extends Component<Prop
       getRoots: this.getRoots,
       highlightItems,
       itemHeight: 21,
       key: this.isEmpty() ? "empty" : "full",
       listItems,
       onCollapse: this.onCollapse,
       onExpand: this.onExpand,
       onFocus: this.onFocus,
+      onActivate: this.onActivate,
       renderItem: this.renderItem,
       preventBlur: true
     };
 
     return <ManagedTree {...treeProps} />;
   }
 
   renderPane(...children) {
@@ -331,17 +329,17 @@ class SourcesTree extends Component<Prop
     const { worker } = this.props;
 
     if (!features.windowlessWorkers && worker) {
       return null;
     }
 
     return this.renderPane(
       this.renderThreadHeader(),
-      <div key="tree" className="sources-list" onKeyDown={this.onKeyDown}>
+      <div key="tree" className="sources-list">
         {this.renderTree()}
       </div>
     );
   }
 }
 
 function getSourceForTree(
   state: AppState,
--- a/devtools/client/debugger/new/src/components/PrimaryPanes/tests/SourcesTree.spec.js
+++ b/devtools/client/debugger/new/src/components/PrimaryPanes/tests/SourcesTree.spec.js
@@ -172,36 +172,27 @@ describe("SourcesTree", () => {
           ...props,
           selectedSource: mockSource
         });
         expect(component).toMatchSnapshot();
       });
     });
   });
 
-  describe("focusItem", () => {
-    it("update the focused item", async () => {
+  describe("activateItem", () => {
+    it("select activated item", async () => {
+      const { instance, props } = render();
       const item = createMockItem();
-      const { component, props } = render({ focused: item });
-
-      await component
-        .find(".sources-list")
-        .simulate("keydown", { keyCode: 13 });
+      const spy = jest.spyOn(instance, "selectItem");
 
-      expect(props.selectSource).toHaveBeenCalledWith(item.contents.id);
-    });
-
-    it("allows focus on the (index)", async () => {
-      const item = createMockItem("https://davidwalsh.name/", "(index)");
-
-      const { component, props } = render({ focused: item });
-      await component
-        .find(".sources-list")
-        .simulate("keydown", { keyCode: 13 });
-      expect(props.selectSource).toHaveBeenCalledWith(item.contents.id);
+      instance.onActivate(item);
+      expect(spy).toHaveBeenCalledWith(item);
+      expect(props.selectSource).toHaveBeenCalledWith(
+        "server1.conn13.child1/39"
+      );
     });
   });
 
   describe("selectItem", () => {
     it("should select item with no children", async () => {
       const { instance, props } = render();
       instance.selectItem(createMockItem());
       expect(props.selectSource).toHaveBeenCalledWith(
@@ -209,24 +200,16 @@ describe("SourcesTree", () => {
       );
     });
 
     it("should not select item with children", async () => {
       const { props, instance } = render();
       instance.selectItem(createMockDirectory());
       expect(props.selectSource).not.toHaveBeenCalled();
     });
-
-    it("does not select if no item is focused on", async () => {
-      const { component, props } = render();
-      await component
-        .find(".sources-list")
-        .simulate("keydown", { keyCode: 13 });
-      expect(props.selectSource).not.toHaveBeenCalled();
-    });
   });
 
   describe("handles items", () => {
     it("getChildren from directory", async () => {
       const { component } = render();
       const item = createMockDirectory("http://mdn.com/views", "views", [
         "a",
         "b"
--- a/devtools/client/debugger/new/src/components/PrimaryPanes/tests/__snapshots__/SourcesTree.spec.js.snap
+++ b/devtools/client/debugger/new/src/components/PrimaryPanes/tests/__snapshots__/SourcesTree.spec.js.snap
@@ -3,17 +3,16 @@
 exports[`SourcesTree After changing expanded nodes Shows the tree with four.js, five.js and six.js expanded 1`] = `
 <div
   className="sources-pane"
   key="pane"
 >
   <div
     className="sources-list"
     key="tree"
-    onKeyDown={[Function]}
   >
     <ManagedTree
       autoExpandAll={false}
       autoExpandDepth={0}
       expanded={
         Array [
           "four.js",
           "five.js",
@@ -21,16 +20,17 @@ exports[`SourcesTree After changing expa
         ]
       }
       getChildren={[Function]}
       getParent={[Function]}
       getPath={[Function]}
       getRoots={[Function]}
       itemHeight={21}
       key="full"
+      onActivate={[Function]}
       onCollapse={[Function]}
       onExpand={[Function]}
       onFocus={[Function]}
       preventBlur={true}
       renderItem={[Function]}
     />
   </div>
 </div>
@@ -39,27 +39,27 @@ exports[`SourcesTree After changing expa
 exports[`SourcesTree Should show the tree with nothing expanded 1`] = `
 <div
   className="sources-pane"
   key="pane"
 >
   <div
     className="sources-list"
     key="tree"
-    onKeyDown={[Function]}
   >
     <ManagedTree
       autoExpandAll={false}
       autoExpandDepth={1}
       getChildren={[Function]}
       getParent={[Function]}
       getPath={[Function]}
       getRoots={[Function]}
       itemHeight={21}
       key="full"
+      onActivate={[Function]}
       onCollapse={[Function]}
       onExpand={[Function]}
       onFocus={[Function]}
       preventBlur={true}
       renderItem={[Function]}
     />
   </div>
 </div>
@@ -68,17 +68,16 @@ exports[`SourcesTree Should show the tre
 exports[`SourcesTree When loading initial source Shows the tree with one.js, two.js and three.js expanded 1`] = `
 <div
   className="sources-pane"
   key="pane"
 >
   <div
     className="sources-list"
     key="tree"
-    onKeyDown={[Function]}
   >
     <ManagedTree
       autoExpandAll={false}
       autoExpandDepth={0}
       expanded={
         Array [
           "one.js",
           "two.js",
@@ -86,16 +85,17 @@ exports[`SourcesTree When loading initia
         ]
       }
       getChildren={[Function]}
       getParent={[Function]}
       getPath={[Function]}
       getRoots={[Function]}
       itemHeight={21}
       key="full"
+      onActivate={[Function]}
       onCollapse={[Function]}
       onExpand={[Function]}
       onFocus={[Function]}
       preventBlur={true}
       renderItem={[Function]}
     />
   </div>
 </div>
@@ -104,17 +104,16 @@ exports[`SourcesTree When loading initia
 exports[`SourcesTree on receiving new props updates highlighted items updates highlightItems if selectedSource changes 1`] = `
 <div
   className="sources-pane"
   key="pane"
 >
   <div
     className="sources-list"
     key="tree"
-    onKeyDown={[Function]}
   >
     <ManagedTree
       autoExpandAll={false}
       autoExpandDepth={1}
       getChildren={[Function]}
       getParent={[Function]}
       getPath={[Function]}
       getRoots={[Function]}
@@ -246,16 +245,17 @@ exports[`SourcesTree on receiving new pr
             "name": "mdn.com",
             "path": "mdn.com",
             "type": "directory",
           },
         ]
       }
       itemHeight={21}
       key="full"
+      onActivate={[Function]}
       onCollapse={[Function]}
       onExpand={[Function]}
       onFocus={[Function]}
       preventBlur={true}
       renderItem={[Function]}
     />
   </div>
 </div>