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

[LoadingButton] does not inherit custom variant override types #30715

Closed
2 tasks done
awdyson opened this issue Jan 21, 2022 · 3 comments · Fixed by #30682
Closed
2 tasks done

[LoadingButton] does not inherit custom variant override types #30715

awdyson opened this issue Jan 21, 2022 · 3 comments · Fixed by #30682
Labels
bug 🐛 Something doesn't work component: LoadingButton The React component. good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@awdyson
Copy link

awdyson commented Jan 21, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

LoadingButton properly renders the styles set in MuiButton.styleOverrides, however the console throws an Invalid Prop Type Value error. I'd guess that this is also a

Expected behavior 🤔

I don't think this used to throw an error? Regardless, this should be an acceptable usage no? Or maybe there needs to be a LoadingButtonPropsVariantOverrides interface?

Steps to reproduce 🕹

  1. Create a ButtonPropsVariantOverrides interface
declare module '@mui/material/Button' {
  interface ButtonPropsVariantOverrides {
    foobar: true;
  }
}
  1. Add some custom styling for these new variants
  2. Try using said variant
<LoadingButton variant="foobar">Some Button</LoadingButton>
  1. Check the console for error

Context 🔦

This is an issue when needing more than the supplied variants -- at least for components that inherit from an override-able source.

Your environment 🌎

`npx @mui/envinfo`
  System:
    OS: macOS 11.6.2
  Binaries:
    Node: 14.16.1 - ~/.local/share/nvm/v14.16.1/bin/node
    Yarn: 3.1.1 - ~/.local/share/nvm/v14.16.1/bin/yarn
    npm: 7.11.2 - ~/.local/share/nvm/v14.16.1/bin/npm
  Browsers:
    Chrome: 97.0.4692.71
@awdyson awdyson added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 21, 2022
@mnajdova
Copy link
Member

Is the issue related to the typings or the propTypes? I think we only need to update the propTypes and add test for it:

index 0a7616f608..48404ab2b5 100644
--- a/packages/mui-lab/src/LoadingButton/LoadingButton.js
+++ b/packages/mui-lab/src/LoadingButton/LoadingButton.js
@@ -266,7 +266,10 @@ LoadingButton.propTypes /* remove-proptypes */ = {
    * The variant to use.
    * @default 'text'
    */
-  variant: PropTypes.oneOf(['contained', 'outlined', 'text']),
+   variant: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([
+    PropTypes.oneOf(['contained', 'outlined', 'text']),
+    PropTypes.string,
+  ]),
 };

 export default LoadingButton;

Feel free to pick it up if you want.

@mnajdova mnajdova added bug 🐛 Something doesn't work component: LoadingButton The React component. good first issue Great for first contributions. Enable to learn the contribution process. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 24, 2022
@niishaaant
Copy link

Hi,
I just left a PR #30880 fixing the issue thanks to @mnajdova . This would be my first contribution so leaving a comment telling me if I did everything right or where I messed up would really help.

@CarlosAmaral
Copy link

This doesn't seem to work anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: LoadingButton The React component. good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants