Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React events: fix press end event dispatching #15500

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 77 additions & 20 deletions packages/react-events/src/Press.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,16 @@ type PressState = {
isPressWithinResponderRegion: boolean,
longPressTimeout: null | Symbol,
pointerType: PointerType,
pressTarget: null | Element | Document,
pressTarget: null | Element,
pressEndTimeout: null | Symbol,
pressStartTimeout: null | Symbol,
responderRegion: null | $ReadOnly<{|
responderRegionOnActivation: null | $ReadOnly<{|
bottom: number,
left: number,
right: number,
top: number,
|}>,
responderRegionOnDeactivation: null | $ReadOnly<{|
bottom: number,
left: number,
right: number,
Expand Down Expand Up @@ -312,7 +318,7 @@ function calculateDelayMS(delay: ?number, min = 0, fallback = 0) {
}

// TODO: account for touch hit slop
function calculateResponderRegion(target, props) {
function calculateResponderRegion(target: Element, props: PressProps) {
const pressRetentionOffset = {
...DEFAULT_PRESS_RETENTION_OFFSET,
...props.pressRetentionOffset,
Expand Down Expand Up @@ -352,15 +358,33 @@ function isPressWithinResponderRegion(
nativeEvent: $PropertyType<ReactResponderEvent, 'nativeEvent'>,
state: PressState,
): boolean {
const {responderRegion} = state;
const {responderRegionOnActivation, responderRegionOnDeactivation} = state;
const event = (nativeEvent: any);
let left, top, right, bottom;

if (responderRegionOnActivation != null) {
left = responderRegionOnActivation.left;
top = responderRegionOnActivation.top;
right = responderRegionOnActivation.right;
bottom = responderRegionOnActivation.bottom;

if (responderRegionOnDeactivation != null) {
left = Math.min(left, responderRegionOnDeactivation.left);
top = Math.min(top, responderRegionOnDeactivation.top);
right = Math.max(right, responderRegionOnDeactivation.right);
bottom = Math.max(bottom, responderRegionOnDeactivation.bottom);
}
}

return (
responderRegion != null &&
(event.pageX >= responderRegion.left &&
event.pageX <= responderRegion.right &&
event.pageY >= responderRegion.top &&
event.pageY <= responderRegion.bottom)
left != null &&
right != null &&
top != null &&
bottom != null &&
(event.pageX >= left &&
event.pageX <= right &&
event.pageY >= top &&
event.pageY <= bottom)
);
}

Expand Down Expand Up @@ -408,7 +432,8 @@ const PressResponder = {
pressEndTimeout: null,
pressStartTimeout: null,
pressTarget: null,
responderRegion: null,
responderRegionOnActivation: null,
responderRegionOnDeactivation: null,
ignoreEmulatedMouseEvents: false,
};
},
Expand Down Expand Up @@ -469,7 +494,11 @@ const PressResponder = {
}

state.pointerType = pointerType;
state.pressTarget = target;
state.pressTarget = getEventCurrentTarget(event, context);
state.responderRegionOnActivation = calculateResponderRegion(
state.pressTarget,
props,
);
state.isPressWithinResponderRegion = true;
dispatchPressStartEvents(context, props, state);
context.addRootEventTypes(rootEventTypes);
Expand Down Expand Up @@ -519,26 +548,34 @@ const PressResponder = {
case 'touchmove': {
if (state.isPressed) {
// Ignore emulated events (pointermove will dispatch touch and mouse events)
// Ignore pointermove events during a keyboard press
// Ignore pointermove events during a keyboard press.
if (state.pointerType !== pointerType) {
return;
}

if (state.responderRegion == null) {
state.responderRegion = calculateResponderRegion(
getEventCurrentTarget(event, context),
// Calculate the responder region we use for deactivation, as the
// element dimensions may have changed since activation.
if (
state.pressTarget !== null &&
state.responderRegionOnDeactivation == null
) {
state.responderRegionOnDeactivation = calculateResponderRegion(
state.pressTarget,
props,
);
}
if (isPressWithinResponderRegion(nativeEvent, state)) {
state.isPressWithinResponderRegion = true;
state.isPressWithinResponderRegion = isPressWithinResponderRegion(
nativeEvent,
state,
);

if (state.isPressWithinResponderRegion) {
if (props.onPressMove) {
dispatchEvent(context, state, 'pressmove', props.onPressMove, {
discrete: false,
});
}
} else {
state.isPressWithinResponderRegion = false;
dispatchPressEndEvents(context, props, state);
}
}
Expand All @@ -551,18 +588,38 @@ const PressResponder = {
case 'mouseup':
case 'touchend': {
if (state.isPressed) {
// Ignore unrelated keyboard events
// Ignore unrelated keyboard events and verify press is within
// responder region for non-keyboard events.
if (pointerType === 'keyboard') {
if (!isValidKeyPress(nativeEvent.key)) {
return;
}
// If the event target isn't within the press target, check if we're still
// within the responder region. The region may have changed if the
// element's layout was modified after activation.
} else if (
state.pressTarget != null &&
!context.isTargetWithinElement(target, state.pressTarget)
) {
// Calculate the responder region we use for deactivation if not
// already done during move event.
if (state.responderRegionOnDeactivation == null) {
state.responderRegionOnDeactivation = calculateResponderRegion(
state.pressTarget,
props,
);
}
state.isPressWithinResponderRegion = isPressWithinResponderRegion(
nativeEvent,
state,
);
}

const wasLongPressed = state.isLongPressed;
dispatchPressEndEvents(context, props, state);

if (state.pressTarget !== null && props.onPress) {
if (context.isTargetWithinElement(target, state.pressTarget)) {
if (state.isPressWithinResponderRegion) {
if (
!(
wasLongPressed &&
Expand Down
Loading