Skip to content

Commit

Permalink
[react-events] Rely on 'buttons' rather than 'button' (#16479)
Browse files Browse the repository at this point in the history
The semantics of 'button' on events differs between PointerEvent and
MouseEvent, whereas they are the same for 'buttons'. Furthermore, 'buttons'
allows developers to determine when multiple buttons are pressed as the same
time.

https://w3c.github.io/pointerevents/#the-button-property
  • Loading branch information
necolas authored Aug 21, 2019
1 parent c433fbb commit 2559111
Show file tree
Hide file tree
Showing 11 changed files with 205 additions and 123 deletions.
4 changes: 2 additions & 2 deletions packages/react-dom/src/events/DOMEventResponderSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ function createDOMResponderEvent(
passive: boolean,
passiveSupported: boolean,
): ReactDOMResponderEvent {
const {pointerType} = (nativeEvent: any);
const {buttons, pointerType} = (nativeEvent: any);
let eventPointerType = '';
let pointerId = null;

Expand All @@ -454,7 +454,7 @@ function createDOMResponderEvent(
pointerId = (nativeEvent: any).pointerId;
} else if (nativeEvent.key !== undefined) {
eventPointerType = 'keyboard';
} else if (nativeEvent.button !== undefined) {
} else if (buttons !== undefined) {
eventPointerType = 'mouse';
} else if ((nativeEvent: any).changedTouches !== undefined) {
eventPointerType = 'touch';
Expand Down
10 changes: 0 additions & 10 deletions packages/react-events/docs/Press.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,6 @@ type PressOffset = {

Disables all `Press` events.

### onContextMenu: (e: PressEvent) => void

Called when the context menu is shown. When a press is active, the context menu
will only be shown (and the press cancelled) if `preventDefault` is `false`.

### onPress: (e: PressEvent) => void

Called immediately after a press is released, unless the press is released
Expand Down Expand Up @@ -115,11 +110,6 @@ down) can be moved back within the bounds of the element to reactivate it.
Ensure you pass in a constant to reduce memory allocations. Default is `20` for
each offset.

### preventContextMenu: boolean = false

Prevents the native context menu from being shown, but `onContextMenu`
is still called.

### preventDefault: boolean = true

Whether to `preventDefault()` native events. Native behavior is prevented by
Expand Down
31 changes: 15 additions & 16 deletions packages/react-events/src/dom/ContextMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,19 @@ type ContextMenuState = {
};

type ContextMenuEvent = {|
target: Element | Document,
type: 'contextmenu',
pointerType: PointerType,
timeStamp: number,
clientX: null | number,
clientY: null | number,
pageX: null | number,
pageY: null | number,
x: null | number,
y: null | number,
altKey: boolean,
buttons: 0 | 1 | 2,
ctrlKey: boolean,
metaKey: boolean,
pageX: null | number,
pageY: null | number,
pointerType: PointerType,
shiftKey: boolean,
target: Element | Document,
timeStamp: number,
type: 'contextmenu',
x: null | number,
y: null | number,
|};

const hasPointerEvents =
Expand All @@ -60,13 +59,13 @@ function dispatchContextMenuEvent(

const gestureState = {
altKey: nativeEvent.altKey,
button: nativeEvent.button === 0 ? 'primary' : 'auxillary',
ctrlKey: nativeEvent.altKey,
metaKey: nativeEvent.altKey,
pageX: nativeEvent.altKey,
pageY: nativeEvent.altKey,
buttons: nativeEvent.buttons != null ? nativeEvent.buttons : 0,
ctrlKey: nativeEvent.ctrlKey,
metaKey: nativeEvent.metaKey,
pageX: nativeEvent.pageX,
pageY: nativeEvent.pageY,
pointerType,
shiftKey: nativeEvent.altKey,
shiftKey: nativeEvent.shiftKey,
target,
timeStamp,
type: 'contextmenu',
Expand Down
8 changes: 4 additions & 4 deletions packages/react-events/src/dom/Hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,16 @@ type HoverState = {
type HoverEventType = 'hoverstart' | 'hoverend' | 'hoverchange' | 'hovermove';

type HoverEvent = {|
pointerType: PointerType,
target: Element | Document,
type: HoverEventType,
timeStamp: number,
clientX: null | number,
clientY: null | number,
pageX: null | number,
pageY: null | number,
pointerType: PointerType,
screenX: null | number,
screenY: null | number,
target: Element | Document,
timeStamp: number,
type: HoverEventType,
x: null | number,
y: null | number,
|};
Expand Down
66 changes: 36 additions & 30 deletions packages/react-events/src/dom/Press.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,24 +75,24 @@ type PressEventType =
| 'presschange';

type PressEvent = {|
button: 'primary' | 'auxillary',
defaultPrevented: boolean,
target: Element | Document,
type: PressEventType,
pointerType: PointerType,
timeStamp: number,
altKey: boolean,
buttons: 0 | 1 | 4,
clientX: null | number,
clientY: null | number,
ctrlKey: boolean,
defaultPrevented: boolean,
metaKey: boolean,
pageX: null | number,
pageY: null | number,
pointerType: PointerType,
screenX: null | number,
screenY: null | number,
shiftKey: boolean,
target: Element | Document,
timeStamp: number,
type: PressEventType,
x: null | number,
y: null | number,
altKey: boolean,
ctrlKey: boolean,
metaKey: boolean,
shiftKey: boolean,
|};

const hasPointerEvents =
Expand Down Expand Up @@ -151,7 +151,7 @@ function createPressEvent(
defaultPrevented: boolean,
): PressEvent {
const timeStamp = context.getTimeStamp();
let button = 'primary';
let buttons = 1;
let clientX = null;
let clientY = null;
let pageX = null;
Expand All @@ -171,31 +171,36 @@ function createPressEvent(
let eventObject;
eventObject = (touchEvent: any) || (nativeEvent: any);
if (eventObject) {
({clientX, clientY, pageX, pageY, screenX, screenY} = eventObject);
}
if (nativeEvent.button === 1) {
button = 'auxillary';
({
buttons,
clientX,
clientY,
pageX,
pageY,
screenX,
screenY,
} = eventObject);
}
}
return {
button,
defaultPrevented,
target,
type,
pointerType,
timeStamp,
altKey,
buttons,
clientX,
clientY,
ctrlKey,
defaultPrevented,
metaKey,
pageX,
pageY,
pointerType,
screenX,
screenY,
shiftKey,
target,
timeStamp,
type,
x: clientX,
y: clientY,
altKey,
ctrlKey,
metaKey,
shiftKey,
};
}

Expand Down Expand Up @@ -581,11 +586,12 @@ const pressResponderImpl = {
state.activePointerId = touchEvent.identifier;
}
// Ignore any device buttons except primary/auxillary and touch/pen contact.
// Ignore any device buttons except primary/secondary and touch/pen contact.
// Additionally we ignore primary-button + ctrl-key with Macs as that
// acts like right-click and opens the contextmenu.
if (
nativeEvent.button > 1 ||
nativeEvent.buttons === 2 ||
nativeEvent.buttons > 4 ||
(isMac && isMouseEvent && nativeEvent.ctrlKey)
) {
return;
Expand Down Expand Up @@ -703,7 +709,7 @@ const pressResponderImpl = {
case 'mouseup':
case 'touchend': {
if (isPressed) {
const button = nativeEvent.button;
const buttons = nativeEvent.buttons;
let isKeyboardEvent = false;
let touchEvent;
if (type === 'pointerup' && activePointerId !== pointerId) {
Expand All @@ -722,7 +728,7 @@ const pressResponderImpl = {
}
isKeyboardEvent = true;
removeRootEventTypes(context, state);
} else if (button === 1) {
} else if (buttons === 4) {
// Remove the root events here as no 'click' event is dispatched when this 'button' is pressed.
removeRootEventTypes(context, state);
}
Expand Down Expand Up @@ -780,7 +786,7 @@ const pressResponderImpl = {
}
}

if (state.isPressWithinResponderRegion && button !== 1) {
if (state.isPressWithinResponderRegion && buttons !== 4) {
dispatchEvent(
event,
onPress,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@

'use strict';

import {createEventTarget, platform, setPointerEvent} from '../testing-library';
import {
buttonsType,
createEventTarget,
platform,
setPointerEvent,
} from '../testing-library';

let React;
let ReactFeatureFlags;
Expand Down Expand Up @@ -61,7 +66,11 @@ describe.each(table)('ContextMenu responder', hasPointerEvents => {
expect(preventDefault).toHaveBeenCalledTimes(1);
expect(onContextMenu).toHaveBeenCalledTimes(1);
expect(onContextMenu).toHaveBeenCalledWith(
expect.objectContaining({pointerType: 'mouse', type: 'contextmenu'}),
expect.objectContaining({
buttons: buttonsType.secondary,
pointerType: 'mouse',
type: 'contextmenu',
}),
);
});

Expand All @@ -80,7 +89,11 @@ describe.each(table)('ContextMenu responder', hasPointerEvents => {
expect(preventDefault).toHaveBeenCalledTimes(1);
expect(onContextMenu).toHaveBeenCalledTimes(1);
expect(onContextMenu).toHaveBeenCalledWith(
expect.objectContaining({pointerType: 'touch', type: 'contextmenu'}),
expect.objectContaining({
buttons: buttonsType.none,
pointerType: 'touch',
type: 'contextmenu',
}),
);
});

Expand Down Expand Up @@ -144,7 +157,11 @@ describe.each(table)('ContextMenu responder', hasPointerEvents => {
target.contextmenu({}, {modified: true});
expect(onContextMenu).toHaveBeenCalledTimes(1);
expect(onContextMenu).toHaveBeenCalledWith(
expect.objectContaining({pointerType: 'mouse', type: 'contextmenu'}),
expect.objectContaining({
buttons: buttonsType.primary,
pointerType: 'mouse',
type: 'contextmenu',
}),
);
});
});
Expand Down
Loading

0 comments on commit 2559111

Please sign in to comment.