Skip to content

Commit c1cba56

Browse files
Don’t add onclick listener to React root
Fixes facebook#13777 As part of facebook#11927 we introduced a regression by adding onclick handler to the React root as well. This causes the whole React tree to flash when tapped on iOS devices (for reasons I outlined in facebook#12989 (comment)). To fix this, we should only apply onclick listeners to portal roots. I verified that this fix indeed worked by checkout out our DOM fixtures and added regression tests as well. Strangely, I had to make changes to the DOM fixtures to see the behavior in the first place. This seems to be caused by our normal sites being bigger than the viewport: ![](http://cl.ly/3f18f8b85e91/Screen%20Recording%202018-10-05%20at%2001.32%20AM.gif) An alternative fix would be to add a third parameter to `appendChildToContainer` based on the tag of the parent fiber. Although I think relying on the `_reactRootContainer` property that we set on the element is less intrusive.
1 parent d836010 commit c1cba56

File tree

2 files changed

+38
-1
lines changed

2 files changed

+38
-1
lines changed

packages/react-dom/src/__tests__/ReactDOMComponent-test.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2659,4 +2659,41 @@ describe('ReactDOMComponent', () => {
26592659
document.body.removeChild(container);
26602660
}
26612661
});
2662+
2663+
describe('iOS Tap Highlight', () => {
2664+
it('adds onclick handler to elements with onClick prop', () => {
2665+
const container = document.createElement('div');
2666+
2667+
const elementRef = React.createRef();
2668+
function Component() {
2669+
return <div ref={elementRef} onClick={() => {}} />;
2670+
}
2671+
2672+
ReactDOM.render(<Component />, container);
2673+
expect(typeof elementRef.current.onclick === 'function').toBeTruthy();
2674+
});
2675+
2676+
it('adds onclick handler to a portal root', () => {
2677+
const container = document.createElement('div');
2678+
2679+
function Component() {
2680+
return <div onClick={() => {}} />;
2681+
}
2682+
2683+
ReactDOM.render(<Component />, container);
2684+
expect(typeof container.onclick === 'function').toBeFalsy();
2685+
});
2686+
2687+
it('does not add onclick handler to react root', () => {
2688+
const container = document.createElement('div');
2689+
const portalContainer = document.createElement('div');
2690+
2691+
function Component() {
2692+
return ReactDOM.createPortal(<div onClick={() => {}} />, portalContainer);
2693+
}
2694+
2695+
ReactDOM.render(<Component />, container);
2696+
expect(typeof portalContainer.onclick === 'function').toBeTruthy();
2697+
});
2698+
});
26622699
});

packages/react-dom/src/client/ReactDOMHostConfig.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ export function appendChildToContainer(
360360
// event exists. So we wouldn't see it and dispatch it.
361361
// This is why we ensure that containers have inline onclick defined.
362362
// https://github.com/facebook/react/issues/11918
363-
if (parentNode.onclick === null) {
363+
if (!container._reactRootContainer && parentNode.onclick === null) {
364364
// TODO: This cast may not be sound for SVG, MathML or custom elements.
365365
trapClickOnNonInteractiveElement(((parentNode: any): HTMLElement));
366366
}

0 commit comments

Comments
 (0)