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
Determine event target in Shadow DOM correctly #12163
Changes from 2 commits
c85de86
d02aa87
e2d5e68
12cb00f
230bc69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import FixtureSet from '../../FixtureSet'; | ||
import TestCase from '../../TestCase'; | ||
|
||
const SUPPORTS_SHADOW_DOM = 'attachShadow' in HTMLElement.prototype; | ||
|
||
const React = window.React; | ||
const ReactDOM = window.ReactDOM; | ||
|
||
class SelectFixture extends React.Component { | ||
render() { | ||
if (!SUPPORTS_SHADOW_DOM) { | ||
return ( | ||
<div>Browser does not support Shadow DOM, no tests to execute.</div> | ||
); | ||
} | ||
|
||
return ( | ||
<FixtureSet title="Shadow DOM" description=""> | ||
<TestCase title="Event listeners in shadow-dom" relatedIssues="4963"> | ||
<TestCase.Steps> | ||
<li>Click on the orange box</li> | ||
</TestCase.Steps> | ||
<TestCase.ExpectedResult> | ||
The box should turn green | ||
</TestCase.ExpectedResult> | ||
<Shadow> | ||
<Box /> | ||
</Shadow> | ||
</TestCase> | ||
</FixtureSet> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should probably also include a |
||
); | ||
} | ||
} | ||
|
||
class Shadow extends React.Component { | ||
componentDidMount() { | ||
this.ref.attachShadow({mode: 'open'}); | ||
const el = document.createElement('div'); | ||
this.ref.shadowRoot.appendChild(el); | ||
ReactDOM.render(this.props.children, el); | ||
} | ||
|
||
render() { | ||
return <div ref={ref => (this.ref = ref)} />; | ||
} | ||
} | ||
|
||
class Box extends React.Component { | ||
state = {active: false}; | ||
|
||
render() { | ||
const style = { | ||
height: 100, | ||
background: this.state.active ? 'green' : 'orange', | ||
color: 'white', | ||
marginBottom: 20, | ||
}; | ||
return ( | ||
<div | ||
onClick={() => this.setState({active: !this.state.active})} | ||
style={style} | ||
/> | ||
); | ||
} | ||
} | ||
|
||
export default SelectFixture; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
/** | ||
* Copyright (c) 2016-present, Facebook, Inc. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
* | ||
* @emails react-core | ||
*/ | ||
|
||
'use strict'; | ||
|
||
let React; | ||
let ReactDOM; | ||
|
||
describe('getEventTarget', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this test suite tests much? I suppose it doesn't hurt but it doesn't really cover this bug, or the svg case. I'm not sure if it makes sense to add. |
||
let container; | ||
|
||
beforeEach(() => { | ||
React = require('react'); | ||
ReactDOM = require('react-dom'); | ||
|
||
// The container has to be attached for events to fire. | ||
container = document.createElement('div'); | ||
document.body.appendChild(container); | ||
}); | ||
|
||
afterEach(() => { | ||
document.body.removeChild(container); | ||
container = null; | ||
}); | ||
|
||
describe('event on HTMLElement', () => { | ||
it('has expected target', () => { | ||
let actual; | ||
|
||
class Comp extends React.Component { | ||
render() { | ||
return ( | ||
<div onClick={e => (actual = e.target)}> | ||
<span /> | ||
</div> | ||
); | ||
} | ||
} | ||
|
||
ReactDOM.render(<Comp />, container); | ||
|
||
const div = container.firstChild; | ||
const span = div.firstChild; | ||
|
||
div.dispatchEvent( | ||
new MouseEvent('click', { | ||
bubbles: true, | ||
cancelable: true, | ||
}), | ||
); | ||
|
||
expect(actual).toBe(div); | ||
|
||
span.dispatchEvent( | ||
new MouseEvent('click', { | ||
bubbles: true, | ||
cancelable: true, | ||
}), | ||
); | ||
|
||
expect(actual).toBe(span); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,12 @@ import {TEXT_NODE} from '../shared/HTMLNodeType'; | |
function getEventTarget(nativeEvent) { | ||
let target = nativeEvent.target || window; | ||
|
||
// If composed / inside open shadow-dom use first item of composed path #9242 | ||
if (nativeEvent.composed) { | ||
const path = nativeEvent.composedPath(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd add an additional safety check here that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
target = path[0]; | ||
} | ||
|
||
// Normalize SVG <use> element events #4963 | ||
if (target.correspondingUseElement) { | ||
target = target.correspondingUseElement; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this test case fail without this PR? My understanding is the event would still fire, but the
target
would be wrong, the TestCase doesn't assert anything about the target, only that the event was seen.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test case fails without the other changes in this PR