Skip to content

chore(site): replace custom LoadingButton from the one in MUI #10351

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

Merged
merged 9 commits into from
Oct 20, 2023

Conversation

BrunoQuaresma
Copy link
Collaborator

No description provided.

@BrunoQuaresma BrunoQuaresma requested a review from a team October 19, 2023 16:45
@BrunoQuaresma BrunoQuaresma self-assigned this Oct 19, 2023
@BrunoQuaresma BrunoQuaresma requested review from aslilac and removed request for a team October 19, 2023 16:45
Copy link
Member

@Parkreiner Parkreiner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look like really good and straightforward changes.

I guess my only slight reservation is with this component still being part of the MUI Lab package, rather than MUI Core. From reading through the docs, it looks like Lab components have a few differences:

  • They aren't necessarily guaranteed to have as good TypeScript support (or possibly even any support at all, for some super-early versions of the components)
  • They reserve the right to introduce breaking changes more aggressively
  • They might not have as much testing in place
  • If something isn't being used enough, they can remove it from the library

I still think that switching over is still the right move – if there's a bug, we can file it with the MUI team and hopefully benefit anyone using the component, and us using it is the only way to help indicate that there's demand for the feature. I'm just wondering if there's a way to indicate that this component is slightly "riskier" than the others we're already using

@BrunoQuaresma
Copy link
Collaborator Author

I'm just wondering if there's a way to indicate that this component is slightly "riskier" than the others we're already using

I think this is okay since we have TS types of checks and tests running in place. Also, this would be a more visual issue and nothing that would block the user so I think it is fine as it is for now.

@BrunoQuaresma BrunoQuaresma merged commit ac32272 into main Oct 20, 2023
@BrunoQuaresma BrunoQuaresma deleted the bq/remove-load-button branch October 20, 2023 12:57
@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants