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

[TypeScript] [ButtonGroup] disabled shows as non-valid property in classes #18272

Closed
2 tasks done
favna opened this issue Nov 8, 2019 · 2 comments · Fixed by #18274
Closed
2 tasks done

[TypeScript] [ButtonGroup] disabled shows as non-valid property in classes #18272

favna opened this issue Nov 8, 2019 · 2 comments · Fixed by #18274
Labels
bug 🐛 Something doesn't work component: ButtonGroup The React component. good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@favna
Copy link
Contributor

favna commented Nov 8, 2019

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

In the [ButtonGroup API] it is listed that CSS overrides classes supports a property disabled that is applied when disabled=true. However passing this style with TypeScript causes typing errors:

No overload matches this call.
  Overload 1 of 2, '(props: { component: ElementType<any>; } & { color?: "primary" | "secondary" | "default" | "inherit" | undefined; disabled?: boolean | undefined; disableFocusRipple?: boolean | undefined; disableRipple?: boolean | undefined; fullWidth?: boolean | undefined; size?: "small" | ... 2 more ... | undefined; variant?: "text" | ... 2 more ... | undefined; } & CommonProps<...> & Pick<...>): Element', gave the following error.
    Type '{ disabled: string; }' is not assignable to type 'Partial<Record<ButtonGroupClassKey, string>>'.
      Object literal may only specify known properties, and 'disabled' does not exist in type 'Partial<Record<ButtonGroupClassKey, string>>'.
  Overload 2 of 2, '(props: DefaultComponentProps<ButtonGroupTypeMap<{}, "div">>): Element', gave the following error.
    Type '{ disabled: string; }' is not assignable to type 'Partial<Record<ButtonGroupClassKey, string>>'.
      Object literal may only specify known properties, and 'disabled' does not exist in type 'Partial<Record<ButtonGroupClassKey, string>>'.ts(2769)
withStyles.d.ts(107, 3): The expected type comes from property 'classes' which is declared here on type 'IntrinsicAttributes & { component: ElementType<any>; } & { color?: "primary" | "secondary" | "default" | "inherit" | undefined; disabled?: boolean | undefined; disableFocusRipple?: boolean | undefined; disableRipple?: boolean | undefined; fullWidth?: boolean | undefined; size?: "small" | ... 2 more ... | undefined; ...'
withStyles.d.ts(107, 3): The expected type comes from property 'classes' which is declared here on type 'IntrinsicAttributes & { color?: "primary" | "secondary" | "default" | "inherit" | undefined; disabled?: boolean | undefined; disableFocusRipple?: boolean | undefined; disableRipple?: boolean | undefined; fullWidth?: boolean | undefined; size?: "small" | ... 2 more ... | undefined; variant?: "text" | ... 2 more ... |...'

image

Expected Behavior 🤔

It should be possible to pass the disabled property and add CSS override classes when the buttongroup is disabled.

Temporary workaround

There is a workaround for the issue, however it is by no means perfect. The following screenshot shows what can be done to achieve the same effect:

image (1)

Steps to Reproduce 🕹

Steps:

  1. Open this codesandbox: https://codesandbox.io/s/material-demo-qgxnh?fontsize=14
  2. Go to demo.tsx
  3. Wait for TypeScript errors to be rendered in the editor. You will also observe that despite the styling the box-shadow is not removed
  4. Disable line 69 and enable line 71. This shows the work-around does work.

Context 🔦

In our company style we want disabled buttons to not have a box-shadow. We currently need to achieve this using a workaround as seen in the codesandbox as the "disabled" class property is broken.

Your Environment 🌎

Tech Version
Material-UI v4.6.0
React v16.11.0
Browser Chrome latest
TypeScript v3.6.3
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: ButtonGroup The React component. good first issue Great for first contributions. Enable to learn the contribution process. labels Nov 8, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 8, 2019

@favna Thanks for the report, the problem could be fixed with a change like this:

diff --git a/packages/material-ui/src/ButtonGroup/ButtonGroup.d.ts b/packages/material-ui/src/ButtonGroup/ButtonGroup.d.ts
index fa0899d9e..745c31d43 100644
--- a/packages/material-ui/src/ButtonGroup/ButtonGroup.d.ts
+++ b/packages/material-ui/src/ButtonGroup/ButtonGroup.d.ts
@@ -31,7 +31,8 @@ export type ButtonGroupClassKey =
   | 'groupedOutlinedSecondary'
   | 'groupedContained'
   | 'groupedContainedPrimary'
-  | 'groupedContainedSecondary';
+  | 'groupedContainedSecondary'
+  | 'disabled';

 export type ButtonGroupProps<
   D extends React.ElementType = ButtonGroupTypeMap['defaultComponent'],

Do you want to work on the issue?

Alternatively, a good workaround is to use the .Mui-disabled class name.

@ssliman
Copy link
Contributor

ssliman commented Nov 8, 2019

@oliviertassinari I can fix this

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: ButtonGroup 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.

3 participants