Skip to content

Experimental Event API: various bug fixes with HitTargets #15485

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 24, 2019
Merged

Experimental Event API: various bug fixes with HitTargets #15485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 9 additions & 2 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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