search

radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.39k stars 768 forks source link

Select component crashes when translated on Chrome #2578

Open joshsny opened 9 months ago

joshsny commented 9 months ago

Bug report

Current Behavior

With a translation extension enabled, or any extension that modifies text on the page, changing the value of a Select component throws the following exception:

DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

This means the user is unable to change the value.

Expected behaviour

The user can change the value of the Select component.

Reproducible example

  1. Set your active language to be something other than English in Chrome
  2. Visit Select component in docs: https://www.radix-ui.com/themes/docs/components/select
  3. Try to change the value of the Select

Suggested solution

Select functionality should depend on the value only and be independent of the label

joshsny commented 9 months ago

Sorry to tag a few people, just want to bring this to your attention in case it got missed as it seems quite important. @benoitgrelard @jjenzz @andy-hook @peduarte @chancethedev @cappuc @Nazeh

This is currently affecting a lot of users of our site (~1% of users seem to use a translation extension on our site) but I imagine is an issue for almost any site using Radix Select (including the docs).

If anyone has ideas for a fix for this I am happy to contribute a PR.

AlexKMarshall commented 8 months ago

This is something we're hitting on our site too. As a temporary workaround we've moved over to HeadlessUI's combobox for this one use-case as it doesn't have the same problem.

We've encountered very similar issues in the past with our own components. When it's happened it's because we've had a an element with text and some other component inside it, which we're conditionally rendering. For instance a loading spinner on a button

<button>
  {children}
  {isLoading && <Spinner/>}
</button>

The extension has modified the text node (children) and when React tries to remove or add the other element (the spinner) its reconciliation breaks because the node has changed.

We worked around this by wrapping the text-containing part in a span, so it's separate from the conditionally rendered element. This doesn't seem possible to do as a consumer of <Select> (I tried). But may be is something that can be changed internally in the rendered markup.

benj-dobs commented 8 months ago

We've managed to work around this on our site by wrapping the label in a span:

<Item>
  <ItemText>
    <span>{label}</span>
  </ItemText>
</Item>

But this might not work for all use cases

rashdeva commented 8 months ago

Same what I did just covered my SelectItem with spans

❌ BEFORE:

  <SelectItem value="main">Main Pool</SelectItem>

✅ AFTER: 

  <SelectItem value="main">
    <span>Main Pool</span>
  </SelectItem>

I think it's neccessary to add for all items extra span for text nodes during Google Translate bug.

benoitgrelard commented 6 months ago

It doesn't look like something we could fix, or has anybody has a more canonical example of the issue here without an extention, ie. simulating what the extension is doing which would let us investigate?

If not, I'm inclined to close the issue.

Let me know!

joshsny commented 3 months ago

@benoitgrelard An extension is actually not required to repro this issue, just using the native translations built into Chrome and following the reproduction steps will trigger the issue.

This means that any users who use a website not in their default locale will not experience a functioning select component.

Here is a video reproducing the error: Screen Cast 2024-05-21 at 1 07 59 PM

benj-dobs commented 3 months ago

This error also occurs in Safari, using the browser's built-in translation feature.

Firefox's translation feature works differently and does not cause these errors.

(In the end our "solution" was to add translate="no" to the <html> element to prevent Safari and Chrome from translating the page at all. We were encountering these issues too frequently, not only with Radix UI but throughout our React app.)

bobinette commented 2 months ago

We also just hit this lately. I created this sandbox @benoitgrelard hoping it helps with investigating and fixing.

On our end, we realised thanks to comments here that the issue comes from the placeholder not being wrapped in a span. We re-implemented the Select.Value to wrap the placholder in a span when it is only a string. We might have to do the same for children, following https://github.com/radix-ui/primitives/issues/2578#issuecomment-1890801041