Properly merge incoming props #1265
Conversation
concise `incomingProps` This is going to make a bit more sense in the next commits of this branch, hold on!
This will allow us to merge the props with a bit more control. Instead of overriding every prop from the user' props with our props, we can now merge event listeners.
This will essentially do the exact same thing we were doing before:
```js
let props = { ...propsTheyControl, ...propsWeControl }
```
But instead of overriding everything, we will merge the event listener
related props like `onClick`, `onKeyDown`, ...
- Rename `propsWeControl` to `ourProps` - Rename `propsTheyControl` to `theirProps`
|
This pull request is being automatically deployed with Vercel (learn more). headlessui-vue – ./packages/playground-vue🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-vue/C8vEahosMWBASDZiU5fQSUBxp82x headlessui-react – ./packages/playground-react🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-react/5Gz8dnfmgoKw87Budwxzfsi1NAbV |
|
Would this also solve #1122? Radix solves that by checking if
|
|
@BrianHung yep that would "fix" that as well. The reason I say "fix", is because you can add an Whether it's a good idea or not to prevent the default logic depends on the context, but this should work: // Example on the main button
function Example() {
return (
<Menu>
{({ open }) => (
<>
<Menu.Button
onClick={(event) => {
if (open) {
console.log('Closing? How about no...')
event.preventDefault()
} else {
console.log('Time to open, all good!')
}
}}
>
Button
</Menu.Button>
...
</>
)}
</Menu>
)
}// Example on an individual item
function Example() {
return (
<Menu>
<Menu.Button>Toggle</Menu.Button>
<Menu.Items>
<Menu.Item as="button" onClick={(event) => event.preventDefault()}>
This stays open...
</Menu.Item>
</Menu.Items>
</Menu>
)
} |
This PR will allow you to pass an
onClickto a component even if the component already has anonClick. This will also be called before our code. This means that you can stop the default behaviour viaevent.preventDefault()if you need to. This is often not recommended, but there are probably use cases for this behaviour.Ignoring the
event.preventDefault()from above, it can still be useful to add your own event listeners in case you need to know about those events.This is only a React fix because in Vue you can already have multiple event listeners. That said, we did similar renames in the Vue code for consistency.
Fixes: #846
Fixes: #1256