Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kilobyte2007 opened this issue Oct 30, 2024 · 6 comments
Assignees
Labels
bug Something isn't working v2

Comments

@kilobyte2007
Copy link
Contributor

kilobyte2007 commented Oct 30, 2024

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

@kilobyte2007 kilobyte2007 added the bug Something isn't working label Oct 30, 2024
@zernonia
Copy link
Member

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

WDYT?

@kilobyte2007
Copy link
Contributor Author

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
Copy link
Member

zernonia commented Nov 1, 2024

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

@kilobyte2007
Copy link
Contributor Author

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
Copy link
Member

zernonia commented Nov 7, 2024

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
Copy link
Contributor Author

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

@zernonia zernonia added the v2 label Nov 16, 2024
@zernonia zernonia added this to v2 Nov 16, 2024
@zernonia zernonia moved this to Backlog in v2 Nov 16, 2024
@zernonia zernonia self-assigned this Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v2
Projects
Status: Backlog
Development

No branches or pull requests

2 participants