Skip to content

[docs][autocomplete] Fix duplicate autocomplete id#42086

Merged
oliviertassinari merged 4 commits intomui:nextfrom
oliviertassinari:fix-duplicate-id
May 3, 2024
Merged

[docs][autocomplete] Fix duplicate autocomplete id#42086
oliviertassinari merged 4 commits intomui:nextfrom
oliviertassinari:fix-duplicate-id

Conversation

@oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented May 1, 2024

We can't have duplicated id on a page. I noticed this in #42085. Part of the value of solving these errors, the more we have, the harder its to quickly spot regressions

SCR-20240501-pstw

https://validator.w3.org/nu/?doc=https%3A%2F%2Fdeploy-preview-39603--material-ui.netlify.app%2Fmaterial-ui%2Freact-autocomplete%2F

A regression from #34066

After: https://deploy-preview-42086--material-ui.netlify.app/material-ui/react-autocomplete/

@oliviertassinari oliviertassinari added type: bug It doesn't behave as expected. docs Improvements or additions to the documentation. scope: autocomplete Changes related to the autocomplete. This includes ComboBox. labels May 1, 2024
@mui-bot
Copy link

mui-bot commented May 1, 2024

Netlify deploy preview

https://deploy-preview-42086--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against cf6704e

@zannager zannager requested a review from michaldudak May 1, 2024 16:38
@oliviertassinari oliviertassinari added the type: regression A bug, but worse, it used to behave as expected. label May 1, 2024
@michaldudak
Copy link
Member

Why do we even need the explicit id? It doesn't add anything of value to the demo.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 2, 2024

@michaldudak The id was needed: #18283 (comment) (a side though: seeing this reinforces the idea that we should comment as much as possible, such a nice payoff 4 years after).

But since useId() was introduced in React 18: https://react.dev/blog/2022/03/29/react-v18#useid, the problem got solved by React.

It's a bit borderline to remove in the docs:

SCR-20240502-ozhb

https://tools-public.mui.com/prod/pages/npmVersion?package=react-dom

but I guess, why not? I have pushed a new commit.

@michaldudak
Copy link
Member

I understand the id is needed to wire the input to the label, but even if we use React 17 and automatically set the id only on the client, it's not really a problem. There won't be any layout shifts and I think it's safe to assume users with assistive technologies will need this in the brief moment before hydration.

@oliviertassinari
Copy link
Member Author

There won't be any layout shifts and I think it's safe to assume users with assistive technologies will need this in the brief moment before hydration.

Fair enough.

@oliviertassinari oliviertassinari merged commit 75e8f4a into mui:next May 3, 2024
@oliviertassinari oliviertassinari deleted the fix-duplicate-id branch May 3, 2024 10:57
@oliviertassinari oliviertassinari removed the type: bug It doesn't behave as expected. label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to the documentation. scope: autocomplete Changes related to the autocomplete. This includes ComboBox. type: regression A bug, but worse, it used to behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants