search

unovue / radix-vue

Vue port of Radix UI Primitives. An open-source UI component library for building high-quality, accessible design systems and web apps.
https://radix-vue.com
MIT License
3.66k stars 226 forks source link

[Bug]: Conflicting data properties between Tooltip and ToggleGroupItem/Toggle #1407

Open kilobyte2007 opened 2 weeks ago

kilobyte2007 commented 2 weeks ago

Environment

Radix v1.9.8

Link to minimal reproduction

Will provide in case more context is needed.

Steps to reproduce

The Tooltip component has a [data-state] attribute that is being set on the trigger based on whether the tooltip is open and can be "closed" | "delayed-open" | "instant-open". The ToggleGroupItem component has a [data-state] attribute which can be "on" | "off" based on whether an item is pressed.

Same for the Toggle component.

Describe the bug

The issue is when I try using a tooltip on a toggle group item, the data-state attribute is not being set correctly.

Not sure what the best solution would be here, but having tooltips on toggle group items, especially the ones with only an icon is pretty important.

Is there an existing solution? Wrapping the item seems like a solution but not a very elegant one.

Expected behavior

No response

Context & Screenshots (if applicable)

No response

zernonia commented 2 weeks ago

Thanks for the issue @kilobyte2007 ! This is something we are aware of https://github.com/radix-ui/primitives/issues/602 In order to not incur breaking change, we can add another attribute for Tooltip component, eg [data-tooltip-state].

WDYT?

kilobyte2007 commented 2 weeks ago

Thanks a lot for the answer @zernonia! Yes, I think that would be reasonable. This way we can differentiate but still be able to use the data attributes for styling and other internal stuff.

But there will still be a small breaking change in case someone was relying on the data-state for the tooltip-status-based styling, no? I have no problem with that, but someone else might.

Also thanks for pointing to the issue, the workaround from the last comment there seems to be the most reasonable one until this is fixed.

zernonia commented 2 weeks ago

For now we can add additional data attribute [data-tooltip-state], this wouldn't incur breaking changes 😁

kilobyte2007 commented 1 week ago

But if we don't remove the data-state attribute from the Tooltip trigger, wouldn't it still overwrite the one from the ones I mentioned? Maybe I am missing something. That was my initial issue.

zernonia commented 1 week ago

You are right @kilobyte2007 ! Perhaps we can leave data-state attribute as is, and add new data-{component}-state attributes for all affected component in v2.

For the mean time you should be able to use the slotProps exposed by Tooltip/Toggle to style it 😁

kilobyte2007 commented 1 week ago

Thanks, that would be the best solution I think, yes!