[Flare] Tweaks to Flare system design and API #16264
Conversation
|
ReactDOM: size: 🔺+0.2%, gzip: 🔺+0.3% Details of bundled changes.Comparing: b5af4fe...d3e1b46 react-dom
react-art
react-native-renderer
react-events
react-test-renderer
react-reconciler
Generated by 🚫 dangerJS |
trueadm
force-pushed
the
further-flare-design-tweaks
branch
2 times, most recently
from
e8866b0 to
8ede067
Compare
WIP Fix test WIP fix
trueadm
force-pushed
the
further-flare-design-tweaks
branch
from
8ede067 to
55ee752
Compare
|
Thanks this looks good! The implementation appears simpler too. I do have a few questions / comments.
The implementation is still dispatching named events. Why do we need to do this? What is the payload
We've discussed several ownership models but this wasn't one of them. I'm not sure refs are a practical way to negotiate ownership as the same ref would have to be passed around a lot. I don't think the term "propagation" is relevant in the context of ownership over a pointer / gesture. We should chat about this idea at work.
👍 But let's do this as a separate PR. |
I didn't want to change the event object shapes as part of this PR.
This isn't to do with the ownership model. It's just a way of allowing responders of the same type to not propagate. Without this our internal Pressability design will no longer work.
I wanted to avoid converting the FocusScope module and tests over. I guess I can do that though. |
That makes perfect sense. But beyond this PR that's something I'd like to better understand. Something we can chat about in person.
I think this is still part of the system that determines which responders should be active, like the
No need to do that. We can merge a PR that removes FocusScope before this one gets applied to master. |
Agreed. I thought this might be an area that you'd like to spend some time working on actually, but you'd want the design changes in before you did that.
Agreed that it doesn't make much sense now. I'll remove that logic and the code for it. As long as we don't sync, this should be fine anyway. I've also put up a PR for the removal of |
|
I've restored the old logic for how responder propagation occurs. |
4 participants
This PR makes some slight adjustments to the exist Flare event system design. After internal testing and from feedback, it was found that passing responders as a React Element to host components, then having detached listeners led to unnecessary boilerplate and some issues around composability. To address these concerns the follow changes have been made:
useListener->useResponderThere was confusion as to why
useListenerwas actually using an Event Responder, but was named "useListener". Now the hook isuseResponder, which takes an Event Responder and returns and Event Listener.respondersprop ->listenerspropNow that the
useResponderhook returns a discrete event listener object, this now gets passed to host components rather than supplying a responder as a React Element on the host component.To demonstrate these changes, here's a before and after:
This design change allows us to explore composing the sequence of listeners, allowing for more complex interactions. It also makes the bindings between listeners and their responders more explicit and less magical.