Commit 753ff7f3 authored by Sigurd Schneider's avatar Sigurd Schneider Committed by Commit Bot

[turbolizer] Rework keydown event handling

This CL simplifies the keydown handling code and fixes
several issues:

 - Input to the search box was not reliably working, because
   the SVG keydown handler was attached to the window and its
   repeat-key detection was supressing key events.
 - Selecting the input of a node via keys 1-9 did not select the
   input, but always enabled the corresponding input node.
   1-9 now select the input node, and CTRL+1 through CTRL+9 can
   be used to toggle the input edge.

Bug: v8:7327
Notry: true
Change-Id: Ifedc8b703f6552e101ad00fee2f3c50f29b325b5
Reviewed-on: https://chromium-review.googlesource.com/c/1397666
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58579}
parent 94f81471
...@@ -62,7 +62,7 @@ export class CodeView extends View { ...@@ -62,7 +62,7 @@ export class CodeView extends View {
for (const location of locations) { for (const location of locations) {
const translated = sourceResolver.translateToSourceId(view.source.sourceId, location); const translated = sourceResolver.translateToSourceId(view.source.sourceId, location);
if (!translated) continue; if (!translated) continue;
view.selection.select(translated, selected); view.selection.select([translated], selected);
} }
view.updateSelection(firstSelect); view.updateSelection(firstSelect);
}, },
......
...@@ -25,7 +25,6 @@ interface GraphState { ...@@ -25,7 +25,6 @@ interface GraphState {
mouseDownNode: any; mouseDownNode: any;
justDragged: boolean, justDragged: boolean,
justScaleTransGraph: boolean, justScaleTransGraph: boolean,
lastKeyDown: number,
hideDead: boolean hideDead: boolean
} }
...@@ -59,12 +58,20 @@ export class GraphView extends View implements PhaseView { ...@@ -59,12 +58,20 @@ export class GraphView extends View implements PhaseView {
this.broker = broker; this.broker = broker;
this.showPhaseByName = showPhaseByName; this.showPhaseByName = showPhaseByName;
this.divElement = d3.select(this.divNode); this.divElement = d3.select(this.divNode);
const svg = this.divElement.append("svg").attr('version', '1.1') const svg = this.divElement.append("svg")
.attr('version', '2.0')
.attr("width", "100%") .attr("width", "100%")
.attr("height", "100%"); .attr("height", "100%")
svg.on("click", function (d) { svg.on("click", function (d) {
view.selectionHandler.clear(); view.selectionHandler.clear();
}); });
// Listen for key events. Note that the focus handler seems
// to be important even if it does nothing.
svg
.attr("focusable", false)
.on("focus", (e) => { })
.on("keydown", (e) => { view.svgKeyDown(); })
view.svg = svg; view.svg = svg;
this.state = { this.state = {
...@@ -72,7 +79,6 @@ export class GraphView extends View implements PhaseView { ...@@ -72,7 +79,6 @@ export class GraphView extends View implements PhaseView {
mouseDownNode: null, mouseDownNode: null,
justDragged: false, justDragged: false,
justScaleTransGraph: false, justScaleTransGraph: false,
lastKeyDown: -1,
showTypes: false, showTypes: false,
hideDead: false hideDead: false
}; };
...@@ -83,7 +89,7 @@ export class GraphView extends View implements PhaseView { ...@@ -83,7 +89,7 @@ export class GraphView extends View implements PhaseView {
broker.broadcastClear(this); broker.broadcastClear(this);
view.updateGraphVisibility(); view.updateGraphVisibility();
}, },
select: function (nodes: Iterable<GNode>, selected: boolean) { select: function (nodes: Array<GNode>, selected: boolean) {
let locations = []; let locations = [];
for (const node of nodes) { for (const node of nodes) {
if (node.nodeLabel.sourcePosition) { if (node.nodeLabel.sourcePosition) {
...@@ -148,14 +154,6 @@ export class GraphView extends View implements PhaseView { ...@@ -148,14 +154,6 @@ export class GraphView extends View implements PhaseView {
view.updateGraphVisibility(); view.updateGraphVisibility();
}); });
// listen for key events
d3.select(window).on("keydown", function (e) {
view.svgKeyDown.call(view);
}).on("keyup", function () {
view.svgKeyUp.call(view);
});
function zoomed() { function zoomed() {
if (d3.event.shiftKey) return false; if (d3.event.shiftKey) return false;
view.graphElement.attr("transform", d3.event.transform); view.graphElement.attr("transform", d3.event.transform);
...@@ -424,13 +422,13 @@ export class GraphView extends View implements PhaseView { ...@@ -424,13 +422,13 @@ export class GraphView extends View implements PhaseView {
reg.exec(n.nodeLabel.opcode) != null); reg.exec(n.nodeLabel.opcode) != null);
}; };
const selection = this.graph.nodes((n) => { const selection = [...this.graph.nodes((n) => {
if ((e.ctrlKey || n.visible) && filterFunction(n)) { if ((e.ctrlKey || n.visible) && filterFunction(n)) {
if (e.ctrlKey) n.visible = true; if (e.ctrlKey) n.visible = true;
return true; return true;
} }
return false; return false;
}); })];
this.selectionHandler.select(selection, true); this.selectionHandler.select(selection, true);
this.connectVisibleSelectedNodes(); this.connectVisibleSelectedNodes();
...@@ -444,23 +442,18 @@ export class GraphView extends View implements PhaseView { ...@@ -444,23 +442,18 @@ export class GraphView extends View implements PhaseView {
svgKeyDown() { svgKeyDown() {
const view = this; const view = this;
const state = this.state; const state = this.state;
let allowRepetition = true;
// Don't handle key press repetition
if (state.lastKeyDown !== -1) return;
const showSelectionFrontierNodes = (inEdges: boolean, filter: (e: Edge, i: number) => boolean, select: boolean) => { const showSelectionFrontierNodes = (inEdges: boolean, filter: (e: Edge, i: number) => boolean, doSelect: boolean) => {
const frontier = view.getNodeFrontier(state.selection, inEdges, filter); const frontier = view.getNodeFrontier(state.selection, inEdges, filter);
if (frontier != undefined && frontier.size) { if (frontier != undefined && frontier.size) {
if (select) { if (doSelect) {
if (!d3.event.shiftKey) { if (!d3.event.shiftKey) {
state.selection.clear(); state.selection.clear();
} }
state.selection.select(frontier, true); state.selection.select([...frontier], true);
} }
view.updateGraphVisibility(); view.updateGraphVisibility();
} }
allowRepetition = false;
} }
let eventHandled = true; // unless the below switch defaults let eventHandled = true; // unless the below switch defaults
...@@ -477,7 +470,7 @@ export class GraphView extends View implements PhaseView { ...@@ -477,7 +470,7 @@ export class GraphView extends View implements PhaseView {
// '1'-'9' // '1'-'9'
showSelectionFrontierNodes(true, showSelectionFrontierNodes(true,
(edge: Edge, index: number) => { return index == (d3.event.keyCode - 49); }, (edge: Edge, index: number) => { return index == (d3.event.keyCode - 49); },
false); !d3.event.ctrlKey);
break; break;
case 97: case 97:
case 98: case 98:
...@@ -491,7 +484,7 @@ export class GraphView extends View implements PhaseView { ...@@ -491,7 +484,7 @@ export class GraphView extends View implements PhaseView {
// 'numpad 1'-'numpad 9' // 'numpad 1'-'numpad 9'
showSelectionFrontierNodes(true, showSelectionFrontierNodes(true,
(edge, index) => { return index == (d3.event.keyCode - 97); }, (edge, index) => { return index == (d3.event.keyCode - 97); },
false); !d3.event.ctrlKey);
break; break;
case 67: case 67:
// 'c' // 'c'
...@@ -511,12 +504,15 @@ export class GraphView extends View implements PhaseView { ...@@ -511,12 +504,15 @@ export class GraphView extends View implements PhaseView {
break; break;
case 73: case 73:
// 'i' // 'i'
if (!d3.event.ctrlKey && !d3.event.shiftKey) {
showSelectionFrontierNodes(true, undefined, false); showSelectionFrontierNodes(true, undefined, false);
} else {
eventHandled = false;
}
break; break;
case 65: case 65:
// 'a' // 'a'
view.selectAllNodes(); view.selectAllNodes();
allowRepetition = false;
break; break;
case 38: case 38:
case 40: { case 40: {
...@@ -525,7 +521,7 @@ export class GraphView extends View implements PhaseView { ...@@ -525,7 +521,7 @@ export class GraphView extends View implements PhaseView {
} }
case 82: case 82:
// 'r' // 'r'
if (!d3.event.ctrlKey) { if (!d3.event.ctrlKey && !d3.event.shiftKey) {
this.layoutAction(this); this.layoutAction(this);
} else { } else {
eventHandled = false; eventHandled = false;
...@@ -535,10 +531,6 @@ export class GraphView extends View implements PhaseView { ...@@ -535,10 +531,6 @@ export class GraphView extends View implements PhaseView {
// 's' // 's'
view.selectOrigins(); view.selectOrigins();
break; break;
case 191:
// '/'
document.getElementById("search-input").focus();
break;
default: default:
eventHandled = false; eventHandled = false;
break; break;
...@@ -546,14 +538,7 @@ export class GraphView extends View implements PhaseView { ...@@ -546,14 +538,7 @@ export class GraphView extends View implements PhaseView {
if (eventHandled) { if (eventHandled) {
d3.event.preventDefault(); d3.event.preventDefault();
} }
if (!allowRepetition) {
state.lastKeyDown = d3.event.keyCode;
} }
}
svgKeyUp() {
this.state.lastKeyDown = -1
};
layoutGraph() { layoutGraph() {
console.time("layoutGraph"); console.time("layoutGraph");
......
...@@ -61,6 +61,11 @@ export class GraphMultiView extends View { ...@@ -61,6 +61,11 @@ export class GraphMultiView extends View {
if (!view.currentPhaseView) return; if (!view.currentPhaseView) return;
view.currentPhaseView.searchInputAction(searchInput, e) view.currentPhaseView.searchInputAction(searchInput, e)
}); });
view.divNode.addEventListener("keyup", (e: KeyboardEvent) => {
if (e.keyCode == 191) { // keyCode == '/'
searchInput.focus();
}
});
searchInput.setAttribute("value", window.sessionStorage.getItem("lastSearch") || ""); searchInput.setAttribute("value", window.sessionStorage.getItem("lastSearch") || "");
this.graph = new GraphView(this.divNode, selectionBroker, this.graph = new GraphView(this.divNode, selectionBroker,
(phaseName) => view.displayPhaseByName(phaseName)); (phaseName) => view.displayPhaseByName(phaseName));
......
...@@ -32,6 +32,7 @@ export class NodeLabel { ...@@ -32,6 +32,7 @@ export class NodeLabel {
this.live = live; this.live = live;
this.properties = properties; this.properties = properties;
this.sourcePosition = sourcePosition; this.sourcePosition = sourcePosition;
this.origin = origin;
this.opcode = opcode; this.opcode = opcode;
this.control = control; this.control = control;
this.opinfo = opinfo; this.opinfo = opinfo;
......
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import {isIterable} from "../src/util"
export class MySelection { export class MySelection {
selection: any; selection: any;
stringKey: (o: any) => string; stringKey: (o: any) => string;
...@@ -21,8 +19,7 @@ export class MySelection { ...@@ -21,8 +19,7 @@ export class MySelection {
this.selection = new Map(); this.selection = new Map();
} }
select(s, isSelected) { select(s: Iterable<any>, isSelected?: boolean) {
if (!isIterable(s)) { s = [s]; }
for (const i of s) { for (const i of s) {
if (!i) continue; if (!i) continue;
if (isSelected == undefined) { if (isSelected == undefined) {
...@@ -36,7 +33,7 @@ export class MySelection { ...@@ -36,7 +33,7 @@ export class MySelection {
} }
} }
isSelected(i): boolean { isSelected(i:any): boolean {
return this.selection.has(this.stringKey(i)); return this.selection.has(this.stringKey(i));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment