Skip to content

Commit

Permalink
fix keyMap behavior by using event.code; add shiftKeys (cruise-automa…
Browse files Browse the repository at this point in the history 
…tion#71)

cruise-automation#56 switched from [`e.keyCode`](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode) to [`e.key`](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key), which caused a problem where keys could get "stuck" down. For example, after `press A, press Shift, release A, release Shift` the CameraListener would think the A key was still pressed.

`e.key` is explicitly supposed to take into account modifier keys such as shift & option, because it represents the character that would be typed when pressing the key, so using it to track key-up/down state doesn't work well. In this PR I'm switching to [`e.code`](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code) instead, which is a more newly standardized property that is independent of keyboard layout (it always works as if the layout were QWERTY) and intended to be used when you care about the key's position rather than the character it represents.

This PR also ignores key repeats, removes an unused `onKeyDown` prop, and fixes a bug where the 1/10 magnitude adjustment for the shift key was applied twice, which made keyboard movement with the shift key even slower than intended.

Also adds a `shiftKeys={/*boolean*/}` to disable shift key modifiers.
  • Loading branch information
@jtbandes
jtbandes authored Mar 26, 2019
1 parent 567b4d2 commit 4c03928
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 54 deletions.
27 changes: 15 additions & 12 deletions docs/src/3.1.Worldview.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
| `onCameraStateChange` | `(CameraState) => void` | | callback whenever the camera state changes |
| `defaultCameraState` | `$Shape<CameraState>` | `DEFAULT_CAMERA_STATE` | pass in this prop to turn Worldview to an uncontrolled component. `onCameraStateChange` will be ignored |
| `keyMap` | `CameraKeyMap` | {} | override default Worldview hotkeys |
| `shiftKeys` | `boolean` | true | enable/disable shift modifier for keyboard controls |
| `backgroundColor` | `Vec4` | `[0,0,0,1]` | change the scene background color |
| `hitmapOnMouseMove` | `boolean` | | if enabled, the clicked objectId will be returned from the mouse event handler |
| `children` | `React.Node` | | regl components to render inside the Worldview |
Expand All @@ -27,18 +28,20 @@ See [MouseEvents](#/docs/api/mouse-events).

## Keyboard Controls

`keyMap` allows you to customize the keyboard controls for Worldview's camera. The `keyMap` is an optional object whose keys are [KeyboardEvent.key](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key) values, and whose values specify the action to perform when the corresponding key is pressed. To disable a key from the keyboard controls, simply provide a `null`/`false` value for that key. For example, the following would reverse all actions and disable tilting and zooming:
`keyMap` allows you to customize the keyboard controls for Worldview's camera. The `keyMap` is an optional object whose keys are [KeyboardEvent.code](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code) values, and whose values specify the action to perform when the corresponding key is pressed. To disable a key from the keyboard controls, simply provide a `null`/`false` value for that key. For example, the following would reverse all actions and disable tilting and zooming:

<Worldview keyMap={{
w: 'moveDown',
a: 'moveRight',
s: 'moveUp',
d: 'moveLeft',
e: 'rotateLeft',
q: 'rotateRight',

r: false,
f: null,
x: false,
z: null,
KeyW: 'moveDown',
KeyA: 'moveRight',
KeyS: 'moveUp',
KeyD: 'moveLeft',
KeyE: 'rotateLeft',
KeyQ: 'rotateRight',

KeyR: false,
KeyF: null,
KeyX: false,
KeyZ: null,
}} />

When the Shift key is pressed along with keys in the `keyMap`, the motion will be slowed. To disable this behavior, pass `shiftKeys={false}`.
2 changes: 1 addition & 1 deletion packages/regl-worldview/package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions packages/regl-worldview/src/Worldview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const DEFAULT_MOUSE_CLICK_RADIUS = 3;

export type BaseProps = {|
keyMap?: CameraKeyMap,
shiftKeys: boolean,
backgroundColor?: Vec4,
// rendering the hitmap on mouse move is expensive, so disable it by default
hitmapOnMouseMove?: boolean,
Expand Down Expand Up @@ -74,6 +75,7 @@ export class WorldviewBase extends React.Component<BaseProps, State> {

static defaultProps = {
backgroundColor: DEFAULT_BACKGROUND_COLOR,
shiftKeys: true,
style: {},
};

Expand Down Expand Up @@ -260,12 +262,12 @@ export class WorldviewBase extends React.Component<BaseProps, State> {
}

render() {
const { width, height, showDebug, keyMap, style } = this.props;
const { width, height, showDebug, keyMap, shiftKeys, style } = this.props;
const { worldviewContext } = this.state;

return (
<div style={{ position: "relative", overflow: "hidden", ...style }}>
<CameraListener cameraStore={worldviewContext.cameraStore} keyMap={keyMap}>
<CameraListener cameraStore={worldviewContext.cameraStore} keyMap={keyMap} shiftKeys={shiftKeys}>
<canvas
style={{ width, height, maxWidth: "100%", maxHeight: "100%" }}
width={width}
Expand Down
81 changes: 42 additions & 39 deletions packages/regl-worldview/src/camera/CameraListener.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,25 @@ const KEYBOARD_ZOOM_SPEED = 150;
const KEYBOARD_SPIN_SPEED = 1.5;

const DEFAULT_KEYMAP: CameraKeyMap = {
a: "moveLeft",
d: "moveRight",
e: "rotateRight",
f: "tiltUp",
q: "rotateLeft",
r: "tiltDown",
s: "moveDown",
w: "moveUp",
x: "zoomOut",
z: "zoomIn",
KeyA: "moveLeft",
KeyD: "moveRight",
KeyE: "rotateRight",
KeyF: "tiltUp",
KeyQ: "rotateLeft",
KeyR: "tiltDown",
KeyS: "moveDown",
KeyW: "moveUp",
KeyX: "zoomOut",
KeyZ: "zoomIn",
};

type KeyMotion = { x?: number, y?: number, zoom?: number, yaw?: number, tilt?: number };

type Props = {|
cameraStore: CameraStore,
keyMap?: CameraKeyMap,
shiftKeys: boolean,
children?: React.ChildrenArray<React.Element<any> | null>,
onKeyDown?: (KeyboardEvent) => void,
|};

// attaches mouse and keyboard listeners to allow for moving the camera on user input
Expand Down Expand Up @@ -112,6 +112,7 @@ export default class CameraListener extends React.Component<Props> {
}

_getMoveMagnitude() {
// avoid interference with drawing tools
if (this._ctrlKey) {
return { x: 0, y: 0 };
}
Expand All @@ -124,15 +125,13 @@ export default class CameraListener extends React.Component<Props> {
if (perspective) {
// in perspective mode its more like flying, so move by the magnitude
// we use the camera distance as a heuristic
const magnitude = this._getMagnitude(distance);

return { x: magnitude, y: magnitude };
return { x: distance, y: distance };
}
// in orthographic mode we know the exact viewable area
// which is a square so we can move exactly percentage within it
const { width, height } = this._rect;
const bounds = getOrthographicBounds(distance, width, height);
return { x: this._getMagnitude(bounds.width), y: this._getMagnitude(bounds.height) };
return { x: bounds.width, y: bounds.height };
}

_onWindowMouseMove = (e: MouseEvent) => {
Expand Down Expand Up @@ -174,7 +173,7 @@ export default class CameraListener extends React.Component<Props> {

if (this._isLeftMouseDown()) {
const { x, y } = this._getMoveMagnitude();
cameraMove([moveX * x, -moveY * y]);
cameraMove([this._getMagnitude(moveX * x), this._getMagnitude(-moveY * y)]);
}
};

Expand Down Expand Up @@ -214,12 +213,16 @@ export default class CameraListener extends React.Component<Props> {
}
}

getKeyMotion = (key: string): ?KeyMotion => {
_getKeyMotion = (code: string): ?KeyMotion => {
const moveSpeed = this._getMagnitude(KEYBOARD_MOVE_SPEED);
const zoomSpeed = this._getMagnitude(KEYBOARD_ZOOM_SPEED);
const spinSpeed = this._getMagnitude(KEYBOARD_SPIN_SPEED);
const { keyMap } = this.props;
const action: CameraAction | false = (keyMap && keyMap[key]) || DEFAULT_KEYMAP[key] || false;
const { keyMap, shiftKeys } = this.props;
const action: CameraAction | false = (keyMap && keyMap[code]) || DEFAULT_KEYMAP[code] || false;

if (this._shiftKey && !shiftKeys) {
return null;
}

switch (action) {
case "moveRight":
Expand Down Expand Up @@ -251,10 +254,10 @@ export default class CameraListener extends React.Component<Props> {
}
};

moveKeyboard(dt: number) {
_moveKeyboard(dt: number) {
const motion = { x: 0, y: 0, zoom: 0, yaw: 0, tilt: 0 };
this._keys.forEach((key) => {
const { x = 0, y = 0, zoom = 0, yaw = 0, tilt = 0 } = this.getKeyMotion(key) || {};
this._keys.forEach((code) => {
const { x = 0, y = 0, zoom = 0, yaw = 0, tilt = 0 } = this._getKeyMotion(code) || {};
motion.x += x;
motion.y += y;
motion.zoom += zoom;
Expand Down Expand Up @@ -288,7 +291,7 @@ export default class CameraListener extends React.Component<Props> {
return;
}
this._keyTimer = requestAnimationFrame((stamp) => {
this.moveKeyboard((lastStamp ? stamp - lastStamp : 0) / 1000);
this._moveKeyboard((lastStamp ? stamp - lastStamp : 0) / 1000);
this._keyTimer = undefined;

// Only start the timer if keys are still pressed.
Expand All @@ -309,45 +312,45 @@ export default class CameraListener extends React.Component<Props> {
this._keyTimer = undefined;
}

_onKeyDown = (e: KeyboardEvent) => {
const { onKeyDown, keyMap } = this.props;
_onKeyDown = (e: SyntheticKeyboardEvent<HTMLDivElement>) => {
const { keyMap } = this.props;
this._shiftKey = e.shiftKey;
this._metaKey = e.metaKey;
this._ctrlKey = e.ctrlKey;
const code = ((e.nativeEvent: any): KeyboardEvent).code;

// ignore repeated keydown events
if (e.repeat || this._keys.has(code)) {
e.stopPropagation();
e.preventDefault();
return;
}

if (e.altKey || e.ctrlKey || e.metaKey) {
if (onKeyDown) {
onKeyDown(e);
}
// we don't currently handle these modifiers
return;
}

// allow null, false, or empty keymappings which explicitly cancel Worldview from processing that key
if (keyMap && e.key in keyMap && !keyMap[e.key]) {
if (keyMap && code in keyMap && !keyMap[code]) {
return false;
}

// ignore repeated keydown events
if (this._keys.has(e.key)) {
e.stopPropagation();
e.preventDefault();
} else if (this.getKeyMotion(e.key)) {
this._keys.add(e.key);
// if we respond to this key, start the update timer
if (this._getKeyMotion(code)) {
this._keys.add(code);
this._startKeyTimer();
e.stopPropagation();
e.preventDefault();
} else if (onKeyDown) {
onKeyDown(e);
}
};

_onKeyUp = (e: KeyboardEvent) => {
_onKeyUp = (e: SyntheticKeyboardEvent<HTMLDivElement>) => {
this._shiftKey = e.shiftKey;
this._metaKey = e.metaKey;
this._ctrlKey = e.ctrlKey;

this._keys.delete(e.key);
this._keys.delete(((e.nativeEvent: any): KeyboardEvent).code);
};

_onWheel = (e: WheelEvent) => {
Expand Down

0 comments on commit 4c03928

Please sign in to comment.