primitives
primitives copied to clipboard
`data-state` composition issue when composing multiple primitives with `asChild`
Stemmed from this discussion https://github.com/radix-ui/primitives/discussions/560#discussioncomment-588638
When I did <Dialog.Trigger as={Tooltip.Trigger}>
the data-state
attribute was maintained from Dialog.Trigger
instead of being overridden with the Tooltip.Trigger
one. I'm not sure if this is a misunderstanding of as
prop on my part (they're a confusing beast) but thought it worth looking into as a DX improvement if possible.
Redundant now that we have asChild
instead.
In fact, the same applies with asChild
... Looks like we need to update Slot
to ensure that the data-state
from the child takes precedence but Slot
already overrides with child props so not sure what's going on here: https://codesandbox.io/s/eager-wood-dv0rl?file=/src/App.js
Ideally we would figure a way to have the state from both parts on the trigger still. There have been other discussions about this.
A workaround is to control the component(s): https://github.com/radix-ui/primitives/discussions/560#discussioncomment-588638
Hey guys,
I dive into it and found what's going on
when you have this structure
const DialogTrigger = (props) => {
const Comp = props.asChild ? Slot: 'button'
return <Comp {...props} />
}
const TooltipTrigger = (props) => {
const Comp = props.asChild ? Slot: 'button'
return <Comp {...props} />
}
<DialogTrigger asChild>
<TooltipTrigger />
</DialogTrigger>
in the mergeProps
function the slotProps
will be the props passed to the <Comp />
in DialogTrigger
and the childProps
will be the props passed to the TooltipTrigger
and not the <Comp />
underlying it and because hence childProps
will be an empty object.
To get the <Comp />
props from TooltipTrigger
it is possible to do children.type(children.props).props
but I don't know if this is a good idea :upside_down_face:. With this, the child props will take precedence.
PS.: The issue occurs with all props. The events are not composed, for example. Only if you do:
onClick={event => {
props.onClick(event)
}}
https://codesandbox.io/s/slot-x16b11?file=/src/App.tsx
Oh we know what the composition issue is, it's just that there is no obvious solution on how to best compose data-state
whilst still maintaining good DX. Given that it only happens in specific cases, it hasn't been a huge priority but I agree it would be good to get down to it at some point.
Essentially, at the moment, there will always be one data-state
that wins in the composition of 2 primitive parts that happen to have a data-state
.
PS.: The issue occurs with all props. The events are not composed, for example. Only if you do: onClick={event => { props.onClick(event) }}
I don't understand what you're saying here?
Hmm, I think I misunderstood the Slot
behavior...
Just to be sure, given this example
<DialogTrigger asChild>
<TooltipTrigger />
</DialogTrigger>
is the expected behavior that the props will merge between those of the rendered DialogTrigger
element and the TooltipTrigger
props (not the rendered TooltipTrigger
element)?
Another good example is using something like a Toggle
as the trigger for a Tooltip
<Tooltip.Root>
<Tooltip.Trigger asChild>
<Toggle.Root pressed />
</Tooltip.Trigger>
</Tooltip.Root>
In this case, the data-state
of the Toggle
can be on|off
. But the data-state
of Trigger
can be open|closed
. Currently the data-state
of the Trigger
overrides that of the Toggle
which means data-state
can no longer be used to style a pressed Toggle
.
We just ran into the same issue (with Tooltip and Tabs). We thought one possible solution to this could be that data-state
gets renamed to data-tabs-state
and data-tooltip-state
and so on, or that at least these scoped states are available in addition to data-state.
I'd be happy to open a PR if a decision is made.
Another good example is using something like a
Toggle
as the trigger for aTooltip
<Tooltip.Root> <Tooltip.Trigger asChild> <Toggle.Root pressed /> </Tooltip.Trigger> </Tooltip.Root>
In this case, the
data-state
of theToggle
can beon|off
. But thedata-state
ofTrigger
can beopen|closed
. Currently thedata-state
of theTrigger
overrides that of theToggle
which meansdata-state
can no longer be used to style a pressedToggle
.
I have this same problem with tooltips and toggles :'(
I think it will be better to append values to data-state
. For the example like below,
<Tooltip.Root>
<Tooltip.Trigger asChild>
<Toggle.Root pressed />
</Tooltip.Trigger>
</Tooltip.Root>
The data-state
of Toggle.Root
will be data-state="open on"
. In this case, we can still use the data-state
in the css using selectors, like [data-state~='open']
, [data-state~='on']
.
I have this same problem with tooltips and toggles :'(
Had this exact same issue. Solved a bunch of other issues related to composing tooltips/popovers/etc from this discussion, but then noticed my tabs weren't being styled when selected, because of data-state
.
To fix this, in my tabs component I pulled out the selected tab state to a useState
with value
and onValueChange
, then put my own "active" styling class on the button as appropriate. Wish I didn't have to do this.