Skip to content

Commit

Permalink
Event API: various bug fixes (#15485)
Browse files Browse the repository at this point in the history
  • Loading branch information
@trueadm
trueadm authored Apr 24, 2019
1 parent fb28e90 commit 3f058de
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 63 deletions.
11 changes: 9 additions & 2 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,13 @@ export function handleEventTarget(
rootContainerInstance: Container,
internalInstanceHandle: Object,
): boolean {
if (
__DEV__ &&
type === REACT_EVENT_TARGET_TOUCH_HIT &&
(props.left || props.right || props.top || props.bottom)
) {
return true;
}
return false;
}

Expand All @@ -992,9 +999,9 @@ export function commitEventTarget(
'value of "relative".',
);
warning(
computedStyles.getPropertyValue('zIndex') !== '',
computedStyles.getPropertyValue('z-index') !== '',
'<TouchHitTarget> inserts an empty <div> with "z-index" of "-1". ' +
'This requires its parent DOM node to have a "z-index" great than "-1",' +
'This requires its parent DOM node to have a "z-index" greater than "-1",' +
'but the parent DOM node was found to no "z-index" value set.' +
' Try using a "z-index" value of "0" or greater.',
);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-events/src/Focus.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ const FocusResponder = {
onEvent(
event: ReactResponderEvent,
context: ReactResponderContext,
props: Object,
props: FocusProps,
state: FocusState,
): void {
const {type, target} = event;
Expand Down
25 changes: 15 additions & 10 deletions packages/react-events/src/Press.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ const targetEventTypes = [
{name: 'keydown', passive: false},
{name: 'keypress', passive: false},
{name: 'contextmenu', passive: false},
'pointerdown',
// We need to preventDefault on pointerdown for mouse/pen events
// that are in hit target area but not the element area.
{name: 'pointerdown', passive: false},
'pointercancel',
];
const rootEventTypes = ['keyup', 'pointerup', 'pointermove', 'scroll'];
Expand Down Expand Up @@ -447,15 +449,18 @@ const PressResponder = {
}
}

// Ignore emulated mouse events and mouse pressing on touch hit target
// area
if (type === 'mousedown') {
if (
state.ignoreEmulatedMouseEvents ||
isEventPositionWithinTouchHitTarget(event, context)
) {
return;
}
// Ignore emulated mouse events
if (type === 'mousedown' && state.ignoreEmulatedMouseEvents) {
return;
}
// Ignore mouse/pen pressing on touch hit target area
if (
(pointerType === 'mouse' || pointerType === 'pen') &&
isEventPositionWithinTouchHitTarget(event, context)
) {
// We need to prevent the native event to block the focus
nativeEvent.preventDefault();
return;
}

// Ignore any device buttons except left-mouse and touch/pen contact
Expand Down
54 changes: 33 additions & 21 deletions packages/react-events/src/__tests__/TouchHitTarget-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ describe('TouchHitTarget', () => {

const Test = () => (
<EventComponent>
<div>
<div style={{position: 'relative', zIndex: 0}}>
{cond ? null : (
<TouchHitTarget top={10} left={10} right={10} bottom={10} />
)}
Expand All @@ -330,22 +330,24 @@ describe('TouchHitTarget', () => {
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'<div style="position: relative; z-index: 0;"><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'left: -10px; right: -10px; top: -10px;"></div></div>',
);

cond = true;
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe('<div></div>');
expect(container.innerHTML).toBe(
'<div style="position: relative; z-index: 0;"></div>',
);
});

it('should render a conditional TouchHitTarget correctly (true -> false)', () => {
let cond = true;

const Test = () => (
<EventComponent>
<div>
<div style={{position: 'relative', zIndex: 0}}>
{cond ? null : (
<TouchHitTarget top={10} left={10} right={10} bottom={10} />
)}
Expand All @@ -356,13 +358,15 @@ describe('TouchHitTarget', () => {
const container = document.createElement('div');
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe('<div></div>');
expect(container.innerHTML).toBe(
'<div style="position: relative; z-index: 0;"></div>',
);

cond = false;
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'<div style="position: relative; z-index: 0;"><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'left: -10px; right: -10px; top: -10px;"></div></div>',
);
});
Expand All @@ -372,7 +376,7 @@ describe('TouchHitTarget', () => {

const Test = () => (
<EventComponent>
<div>
<div style={{position: 'relative', zIndex: 0}}>
{cond ? (
<TouchHitTarget />
) : (
Expand All @@ -386,22 +390,24 @@ describe('TouchHitTarget', () => {
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'<div style="position: relative; z-index: 0;"><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'left: -10px; right: -10px; top: -10px;"></div></div>',
);

cond = true;
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe('<div></div>');
expect(container.innerHTML).toBe(
'<div style="position: relative; z-index: 0;"></div>',
);
});

it('should render a conditional TouchHitTarget hit slop correctly (true -> false)', () => {
let cond = true;

const Test = () => (
<EventComponent>
<div>
<div style={{position: 'relative', zIndex: 0}}>
<span>Random span 1</span>
{cond ? (
<TouchHitTarget />
Expand All @@ -417,14 +423,15 @@ describe('TouchHitTarget', () => {
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><span>Random span 1</span><span>Random span 2</span></div>',
'<div style="position: relative; z-index: 0;"><span>Random span 1</span><span>Random span 2</span></div>',
);

cond = false;
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><span>Random span 1</span><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'<div style="position: relative; z-index: 0;"><span>Random span 1</span>' +
'<div style="position: absolute; z-index: -1; bottom: -10px; ' +
'left: -10px; right: -10px; top: -10px;"></div><span>Random span 2</span></div>',
);
});
Expand All @@ -434,7 +441,7 @@ describe('TouchHitTarget', () => {

const Test = () => (
<EventComponent>
<div>
<div style={{position: 'relative', zIndex: 0}}>
<span>Random span 1</span>
{cond ? (
<TouchHitTarget top={10} left={null} right={10} bottom={10} />
Expand All @@ -455,15 +462,17 @@ describe('TouchHitTarget', () => {
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><span>Random span 1</span><div style="position: absolute; z-index: -1; bottom: 0px; ' +
'<div style="position: relative; z-index: 0;"><span>Random span 1</span>' +
'<div style="position: absolute; z-index: -1; bottom: 0px; ' +
'left: -20px; right: 0px; top: 0px;"></div><span>Random span 2</span></div>',
);

cond = true;
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><span>Random span 1</span><div style="position: absolute; z-index: -1; bottom: 0px; ' +
'<div style="position: relative; z-index: 0;"><span>Random span 1</span>' +
'<div style="position: absolute; z-index: -1; bottom: 0px; ' +
'left: -20px; right: 0px; top: 0px;"></div><span>Random span 2</span></div>',
);
});
Expand All @@ -473,7 +482,7 @@ describe('TouchHitTarget', () => {

const Test = () => (
<EventComponent>
<div>
<div style={{position: 'relative', zIndex: 0}}>
<span>Random span 1</span>
{cond ? (
<TouchHitTarget top={10} left={null} right={10} bottom={10} />
Expand All @@ -494,30 +503,33 @@ describe('TouchHitTarget', () => {
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><span>Random span 1</span><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'<div style="position: relative; z-index: 0;"><span>Random span 1</span>' +
'<div style="position: absolute; z-index: -1; bottom: -10px; ' +
'left: 0px; right: -10px; top: -10px;"></div><span>Random span 2</span></div>',
);

cond = false;
ReactDOM.render(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><span>Random span 1</span><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'<div style="position: relative; z-index: 0;"><span>Random span 1</span>' +
'<div style="position: absolute; z-index: -1; bottom: -10px; ' +
'left: 0px; right: -10px; top: -10px;"></div><span>Random span 2</span></div>',
);
});

it('should hydrate TouchHitTarget hit slop elements correcty and patch them', () => {
const Test = () => (
<EventComponent>
<div>
<div style={{position: 'relative', zIndex: 0}}>
<TouchHitTarget top={10} left={10} right={10} bottom={10} />
</div>
</EventComponent>
);

const container = document.createElement('div');
container.innerHTML = '<div></div>';
container.innerHTML =
'<div style="position: relative; z-index: 0"></div>';
expect(() => {
ReactDOM.hydrate(<Test />, container);
expect(Scheduler).toFlushWithoutYielding();
Expand All @@ -527,7 +539,7 @@ describe('TouchHitTarget', () => {
);
expect(Scheduler).toFlushWithoutYielding();
expect(container.innerHTML).toBe(
'<div><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'<div style="position: relative; z-index: 0"><div style="position: absolute; z-index: -1; bottom: -10px; ' +
'left: -10px; right: -10px; top: -10px;"></div></div>',
);
});
Expand Down
64 changes: 35 additions & 29 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,9 +595,41 @@ function commitLifeCycles(
}
case SuspenseComponent:
case IncompleteClassComponent:
case EventTarget:
case EventComponent:
break;
return;
case EventTarget: {
if (enableEventAPI) {
const type = finishedWork.type.type;
const props = finishedWork.memoizedProps;
const instance = finishedWork.stateNode;
let parentInstance = null;

let node = finishedWork.return;
// Traverse up the fiber tree until we find the parent host node.
while (node !== null) {
if (node.tag === HostComponent) {
parentInstance = node.stateNode;
break;
} else if (node.tag === HostRoot) {
parentInstance = node.stateNode.containerInfo;
break;
}
node = node.return;
}
invariant(
parentInstance !== null,
'This should have a parent host component initialized. This error is likely ' +
'caused by a bug in React. Please file an issue.',
);
commitEventTarget(type, props, instance, parentInstance);
}
return;
}
case EventComponent: {
if (enableEventAPI) {
mountEventComponent(finishedWork.stateNode);
}
return;
}
default: {
invariant(
false,
Expand Down Expand Up @@ -1218,31 +1250,6 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {
return;
}
case EventTarget: {
if (enableEventAPI) {
const type = finishedWork.type.type;
const props = finishedWork.memoizedProps;
const instance = finishedWork.stateNode;
let parentInstance = null;

let node = finishedWork.return;
// Traverse up the fiber tree until we find the parent host node.
while (node !== null) {
if (node.tag === HostComponent) {
parentInstance = node.stateNode;
break;
} else if (node.tag === HostRoot) {
parentInstance = node.stateNode.containerInfo;
break;
}
node = node.return;
}
invariant(
parentInstance !== null,
'This should have a parent host component initialized. This error is likely ' +
'caused by a bug in React. Please file an issue.',
);
commitEventTarget(type, props, instance, parentInstance);
}
return;
}
case HostRoot: {
Expand All @@ -1259,7 +1266,6 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {
return;
}
case EventComponent: {
mountEventComponent(finishedWork.stateNode);
return;
}
default: {
Expand Down

0 comments on commit 3f058de

Please sign in to comment.