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

[lab][LoadingButton] Prevent React crash with Google Translate #42552

Closed

Conversation

aarongarciah
Copy link
Member

@aarongarciah aarongarciah commented Jun 6, 2024

Resolves #27853. Based on a previous attempt #35198.


Original PR description:

Use case summary:

Using the LoadingButton while have Google translation (chrome feature) active on your site.
Problem: Google modifies the button contents and React can't figure out its bindings anymore. Causes crash 💥

How to reproduce:

https://codesandbox.io/s/dry-water-eig2e7?file=/demo.tsx

Steps:

  • Open sandbox in external browser
  • Use google chrome's translate feature (see screenshot)
  • Click the loading button
  • Notice crash 💥
    image

Additional description:

I introduced two changes from the original PR:

  1. Use a <span> instead of a <div> to wrap children.
  2. Remove display: contents.

Regarding number 2, display: contents suffered from deep accessibility issues, the most important one causing elements with the property applied to be removed from the accessibility tree. Most of these issues have been solved over the years, but it's still unclear if it's 100% reliable, especially on Safari. I'm wary of using this property if we're not 100% sure about it'll have the expected behavior. Let me know your thoughts. Some literature about the topic:

@aarongarciah aarongarciah added package: lab Specific to @mui/lab component: LoadingButton The React component. labels Jun 6, 2024
@aarongarciah aarongarciah self-assigned this Jun 6, 2024
@aarongarciah aarongarciah changed the title [lab][LoadingButton] Loading button wrap contents [lab][LoadingButton] Prevent React crash with Google Translate Jun 6, 2024
@mui-bot
Copy link

mui-bot commented Jun 6, 2024

Netlify deploy preview

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 4856247

@aarongarciah aarongarciah added this to the Material UI: v6 milestone Jun 6, 2024
@aarongarciah aarongarciah force-pushed the loading-button-wrap-contents branch from 521f813 to 4856247 Compare June 6, 2024 16:06
@aarongarciah aarongarciah requested a review from DiegoAndai June 6, 2024 16:17
@aarongarciah aarongarciah marked this pull request as ready for review June 6, 2024 16:17
@aarongarciah aarongarciah removed this from the Material UI: v6 milestone Jun 6, 2024
@aarongarciah
Copy link
Member Author

aarongarciah commented Jun 6, 2024

Closing this to continue in the original PR: #35198

@aarongarciah aarongarciah removed the request for review from DiegoAndai June 6, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: LoadingButton The React component. package: lab Specific to @mui/lab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][LoadingButton] Crashes React when Chrome has Translated the Page
3 participants