Add event touch hit target hit slop logic to experimental event API #15261
Conversation
|
ReactDOM: size: 0.0%, gzip: 🔺+0.1% Details of bundled changes.Comparing: 4482fdd...15f37f3 react-dom
react-test-renderer
Generated by 🚫 dangerJS |
| // element inside it absolutely position around the target. | ||
| // TODO add a dev check for the computed style and warn if it isn't | ||
| // compatible. | ||
| element.style.position = 'relative'; |
There was a problem hiding this comment.
@necolas We need to come up with a good solution for this. We should mark this in the umbrella issue and track it as to how we can validate it and make sure stacking order etc is good.
| // We update the event target state node to be that of the element. | ||
| // We can then diff this entry to determine if we need to add the | ||
| // hit slop element, or change the dimensions of the hit slop. | ||
| const lastElement = internalInstanceHandle.stateNode; |
There was a problem hiding this comment.
We shouldn't drill into fibers at this level. We assume that host configs don't have access to fibers. These are supposed to be opaque types.
There was a problem hiding this comment.
I wasn't aware of this constraint. I can re-work it so that stateNode is an object ref instead. Then the object ref is passed to this function in host config; would that make more sense?
|
@sebmarkbage In regards to SSR, there is a task on here #15257 to do it as a separate point. In regards to |
| // element inside it absolutely position around the target. | ||
| // TODO add a dev check for the computed style and warn if it isn't | ||
| // compatible. | ||
| element.style.position = 'relative'; |
There was a problem hiding this comment.
This won't be resilient to the styles changing on the element. Updates to it might override this.
There was a problem hiding this comment.
Yes, this was a bit talking point with @necolas too. There's no cheap way to get around having to do a computed lookup to find what the position is. Do you have any ideas on how we might solve this problem?
| 'div', | ||
| emptyObject, | ||
| rootContainerInstance, | ||
| parentNamespace, |
There was a problem hiding this comment.
This won't work if the parentNamespace isn't HTML so you should throw if you don't intend to support other namespaces.
|
If the EventTarget's element gets a child added at the end, then React will think it's the last child so we use parentNode.appendChild(newNode) to insert it. That will put it at the end of the list but I think you want the hit slop to always be the last child in the children list. Can you add this test? |
|
@sebmarkbage As the hit slop is absolutely positioned, I'm not sure it matters. It might mess with CSS styles though. With your idea around |
|
I'm closing this and will open another PR with a different implementation. |
Note: this is for an experimental event API that we're testing out internally at Facebook.
This PR adds the necessary logic to mount and update the hit slop for touch hit targets. It does this by creating an element, absolutely positioning it inside a child and then appending it the target DOM element. This required moving some parts around to add the necessary fields required to properly ensure that we can create the element in the right namespace. There are also some TODOs that will be tackled in the coming weeks.