Skip to content

Commit

Permalink
React events: fix nested Hover components error (#15428)
Browse files Browse the repository at this point in the history
* Add failing test for nested Hover
* Fix error caused by nested Hover event components
  • Loading branch information
necolas authored Apr 17, 2019
1 parent c73ab39 commit 1ae409d
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 20 deletions.
1 change: 1 addition & 0 deletions packages/react-events/src/Focus.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type FocusProps = {
onBlur: (e: FocusEvent) => void,
onFocus: (e: FocusEvent) => void,
onFocusChange: boolean => void,
stopPropagation: boolean,
};

type FocusState = {
Expand Down
40 changes: 24 additions & 16 deletions packages/react-events/src/Hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ type HoverProps = {
onHoverEnd: (e: HoverEvent) => void,
onHoverMove: (e: HoverEvent) => void,
onHoverStart: (e: HoverEvent) => void,
preventDefault: boolean,
stopPropagation: boolean,
};

type HoverState = {
Expand Down Expand Up @@ -178,6 +180,11 @@ function dispatchHoverEndEvents(
if (props.onHoverChange) {
dispatchHoverChangeEvent(context, props, state);
}

state.isInHitSlop = false;
state.hoverTarget = null;
state.skipMouseAfterPointer = false;
state.isTouched = false;
};

if (state.isActiveHovered) {
Expand Down Expand Up @@ -231,7 +238,8 @@ const HoverResponder = {
props: HoverProps,
state: HoverState,
): boolean {
const {type, phase, target, nativeEvent} = event;
const {type, phase, target} = event;
const nativeEvent: any = event.nativeEvent;

// Hover doesn't handle capture target events at this point
if (phase === CAPTURE_PHASE) {
Expand All @@ -247,11 +255,18 @@ const HoverResponder = {
}
break;
}
case 'touchcancel':
case 'touchend': {
if (state.isTouched) {
state.isTouched = false;
}
break;
}

case 'pointerover':
case 'mouseover': {
if (!state.isHovered && !state.isTouched) {
if ((nativeEvent: any).pointerType === 'touch') {
if (nativeEvent.pointerType === 'touch') {
state.isTouched = true;
return false;
}
Expand All @@ -261,8 +276,8 @@ const HoverResponder = {
if (
context.isPositionWithinTouchHitTarget(
target.ownerDocument,
(nativeEvent: any).x,
(nativeEvent: any).y,
nativeEvent.x,
nativeEvent.y,
)
) {
state.isInHitSlop = true;
Expand All @@ -278,10 +293,6 @@ const HoverResponder = {
if (state.isHovered && !state.isTouched) {
dispatchHoverEndEvents(event, context, props, state);
}
state.isInHitSlop = false;
state.hoverTarget = null;
state.isTouched = false;
state.skipMouseAfterPointer = false;
break;
}

Expand All @@ -296,8 +307,8 @@ const HoverResponder = {
if (
!context.isPositionWithinTouchHitTarget(
target.ownerDocument,
(nativeEvent: any).x,
(nativeEvent: any).y,
nativeEvent.x,
nativeEvent.y,
)
) {
dispatchHoverStartEvents(event, context, props, state);
Expand All @@ -307,18 +318,15 @@ const HoverResponder = {
if (
context.isPositionWithinTouchHitTarget(
target.ownerDocument,
(nativeEvent: any).x,
(nativeEvent: any).y,
nativeEvent.x,
nativeEvent.y,
)
) {
dispatchHoverEndEvents(event, context, props, state);
state.isInHitSlop = true;
} else {
if (props.onHoverMove) {
const syntheticEvent = createHoverEvent(
'hovermove',
event.target,
);
const syntheticEvent = createHoverEvent('hovermove', target);
context.dispatchEvent(syntheticEvent, props.onHoverMove, {
discrete: false,
});
Expand Down
2 changes: 1 addition & 1 deletion packages/react-events/src/__tests__/Focus-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ describe('Focus event responder', () => {
});

describe('nested Focus components', () => {
it('does not propagate events by default', () => {
it('do not propagate events by default', () => {
const events = [];
const innerRef = React.createRef();
const outerRef = React.createRef();
Expand Down
68 changes: 65 additions & 3 deletions packages/react-events/src/__tests__/Hover-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ let ReactFeatureFlags;
let ReactDOM;
let Hover;

const createPointerEvent = type => {
const event = document.createEvent('Event');
event.initEvent(type, true, true);
const createPointerEvent = (type, data) => {
const event = document.createEvent('CustomEvent');
event.initCustomEvent(type, true, true);
if (data != null) {
Object.entries(data).forEach(([key, value]) => {
event[key] = value;
});
}
return event;
};

Expand Down Expand Up @@ -361,6 +366,63 @@ describe('Hover event responder', () => {
});
});

describe('nested Hover components', () => {
it('do not propagate events by default', () => {
const events = [];
const innerRef = React.createRef();
const outerRef = React.createRef();
const createEventHandler = msg => () => {
events.push(msg);
};

const element = (
<Hover
onHoverStart={createEventHandler('outer: onHoverStart')}
onHoverEnd={createEventHandler('outer: onHoverEnd')}
onHoverChange={createEventHandler('outer: onHoverChange')}>
<div ref={outerRef}>
<Hover
onHoverStart={createEventHandler('inner: onHoverStart')}
onHoverEnd={createEventHandler('inner: onHoverEnd')}
onHoverChange={createEventHandler('inner: onHoverChange')}>
<div ref={innerRef} />
</Hover>
</div>
</Hover>
);

ReactDOM.render(element, container);

outerRef.current.dispatchEvent(createPointerEvent('pointerover'));
outerRef.current.dispatchEvent(
createPointerEvent('pointerout', {relatedTarget: innerRef.current}),
);
innerRef.current.dispatchEvent(createPointerEvent('pointerover'));
innerRef.current.dispatchEvent(
createPointerEvent('pointerout', {relatedTarget: outerRef.current}),
);
outerRef.current.dispatchEvent(
createPointerEvent('pointerover', {relatedTarget: innerRef.current}),
);
outerRef.current.dispatchEvent(createPointerEvent('pointerout'));
// TODO: correct result should include commented events
expect(events).toEqual([
'outer: onHoverStart',
'outer: onHoverChange',
// 'outer: onHoverEnd',
// 'outer: onHoverChange',
'inner: onHoverStart',
'inner: onHoverChange',
'inner: onHoverEnd',
'inner: onHoverChange',
// 'outer: onHoverStart',
// 'outer: onHoverChange',
'outer: onHoverEnd',
'outer: onHoverChange',
]);
});
});

it('expect displayName to show up for event component', () => {
expect(Hover.displayName).toBe('Hover');
});
Expand Down

0 comments on commit 1ae409d

Please sign in to comment.