radix-ui / themes

Radix Themes is an open-source component library optimized for fast development, easy maintenance, and accessibility. Maintained by @workos.
https://radix-ui.com/themes
MIT License
4.81k stars 167 forks source link

Switch inside of a Tooltip is not rendered correctly #440

Open janmonschke opened 3 months ago

janmonschke commented 3 months ago

Description

When a Switch is rendered inside a Tooltip, the Switch's background color is not correct when it's unchecked.

Actual behaviour

Screenshot 2024-04-05 at 20 54 00 Screenshot 2024-04-05 at 20 54 10

Example code:

<Heading mb="2">Switch with a tooltip</Heading>
<Tooltip content="test tooltip content">
  <Switch defaultChecked={false} />
</Tooltip>
<Heading my="2">Switch without a tooltip</Heading>
<Switch defaultChecked={false} />

Codesandbox: https://codesandbox.io/p/sandbox/radix-themes-tooltip-switch-g7jgg5

Expected behaviour

The background color of the Switch inside the Tooltip matches the unchecked color of the standalone Switch.

Meta

henriquekio commented 3 months ago

Hey,

Observing the inspector looks like it occurs because the CSS of the switch uses the data-state (checked | unchecked) attr as a selector, but, the Tooltip component replaces the data-state also (closed | delayed-open | instant-open).

Every time that you set a tooltip your first child will receive this attr (data-state), and in this case, this causes a conflict of styles.

I don't know if it's possible to replace this class selector to not watch the data-state attribute, and if this change, can hurt the accessibility principles of radix.

But you can quickly fix this using an ancestor in the switch component like a span element. Using this way, the data-state of the tooltip will change states on the ancestor element (span), and not will interfere with the data-state of the switch.

Codesandbox: https://codesandbox.io/p/sandbox/radix-themes-tooltip-switch-forked-qmlwq7

janmonschke commented 3 months ago

Gotcha, the <span /> workaround is good enough for me at the moment. This should probably be mentioned in the docs though, wdyt?

vladmoroz commented 3 months ago

@benoitgrelard do you know of a way around this issue?