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

[Checkbox][Radio] Icon font size override #21289

Closed
2 tasks done
samuelcolburn opened this issue Jun 2, 2020 · 3 comments · Fixed by #21362
Closed
2 tasks done

[Checkbox][Radio] Icon font size override #21289

samuelcolburn opened this issue Jun 2, 2020 · 3 comments · Fixed by #21362
Labels
bug 🐛 Something doesn't work component: checkbox This is the name of the generic UI component, not the React module! component: radio This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@samuelcolburn
Copy link

When using the icon and checkedIcon props for the Checkbox component, passing a custom fontSize doesn't change size of the icons.

This issue is also present in the Checkbox demos, where the Custom Size example is the same size as all the other examples.

  • 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 😯

Passing a fontSize prop to the icon or customIcon component does nothing.

<Checkbox
  icon={<CheckBoxOutlineBlankIcon fontSize="small" />} // this doesn't do anything
  checkedIcon={<CheckBoxIcon fontSize="large" />} // neither does this
/>

// other props like color work as expected
<Checkbox
  icon={<CheckBoxOutlineBlankIcon color="secondary" />}
/>

Expected Behavior 🤔

Passing a valid fontSize prop to icon or customIcon should increase the size of the Icon.

Steps to Reproduce 🕹

Steps:

  1. Create a Checkbox component
  2. Pass a custom component to icon or checkedIcon props
  3. Add a custom fontSize
  4. See nothing happen

Links

Sandbox with error reproduced: https://codesandbox.io/s/checkbox-issue-demo-ksr7k

Documentation section with the same error: https://material-ui.com/components/checkboxes/#checkbox-with-formcontrollabel

Context 🔦

I want to be able to customize the size of my Checkboxes. Note that other props like color are not overridden in the same manner.

A somewhat similar issue is currently open regarding Input size props:
#19796

Your Environment 🌎

Tech Version
Material-UI v4.10.1
React 16.13.1
Browser Chrome version 83.0.4103.61 (64 bit)
@samuelcolburn samuelcolburn added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 2, 2020
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: checkbox This is the name of the generic UI component, not the React module! component: radio This is the name of the generic UI component, not the React module! 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 Jun 3, 2020
@oliviertassinari
Copy link
Member

@samuelcolburn Thanks for reporting the issue. It comes from the usage of cloneElement that forces a value. It affects the Radio component too. What do you think of this fix?

diff --git a/packages/material-ui/src/Checkbox/Checkbox.js b/packages/material-ui/src/Checkbox/Checkbox.js
index 7916091ab..6d28ca15c 100644
--- a/packages/material-ui/src/Checkbox/Checkbox.js
+++ b/packages/material-ui/src/Checkbox/Checkbox.js
@@ -64,14 +64,17 @@ const Checkbox = React.forwardRef(function Checkbox(props, ref) {
     checkedIcon = defaultCheckedIcon,
     classes,
     color = 'secondary',
-    icon = defaultIcon,
+    icon: iconProp = defaultIcon,
     indeterminate = false,
-    indeterminateIcon = defaultIndeterminateIcon,
+    indeterminateIcon: indeterminateIconProp = defaultIndeterminateIcon,
     inputProps,
     size = 'medium',
     ...other
   } = props;

+  const icon = indeterminate ? indeterminateIconProp : iconProp;
+  const indeterminateIcon = indeterminate ? indeterminateIconProp : checkedIcon;
+
   return (
     <SwitchBase
       type="checkbox"
@@ -87,11 +90,11 @@ const Checkbox = React.forwardRef(function Checkbox(props, ref) {
         'data-indeterminate': indeterminate,
         ...inputProps,
       }}
-      icon={React.cloneElement(indeterminate ? indeterminateIcon : icon, {
-        fontSize: size === 'small' ? 'small' : 'default',
+      icon={React.cloneElement(icon, {
+        fontSize: icon.props.fontSize === undefined && size !== 'medium' ? size : icon.props.fontSize,
       })}
-      checkedIcon={React.cloneElement(indeterminate ? indeterminateIcon : checkedIcon, {
-        fontSize: size === 'small' ? 'small' : 'default',
+      checkedIcon={React.cloneElement(indeterminateIcon, {
+        fontSize: indeterminateIcon.props.fontSize === undefined && size !== 'medium' ? size : indeterminateIcon.props.fontSize
       })}
       ref={ref}
       {...other}
@@ -171,7 +174,7 @@ Checkbox.propTypes = {
    * The size of the checkbox.
    * `small` is equivalent to the dense checkbox styling.
    */
-  size: PropTypes.oneOf(['medium', 'small']),
+  size: PropTypes.oneOf(['large', 'medium', 'small']),
   /**
    * The value of the component. The DOM API casts this to a string.
    * The browser uses "on" as the default value.

It's a start. Do you want to work on a pull request? :)

@oliviertassinari oliviertassinari changed the title [Checkbox][docs] Passing fontSize to Checkbox Icon props does nothing [Checkbox][Radio] Icon font size override Jun 3, 2020
@kn1ves
Copy link
Contributor

kn1ves commented Jun 5, 2020

@oliviertassinari I wish to work on this as my first issue if it is not taken by someone else already, thanks.

@oliviertassinari
Copy link
Member

@kn1ves Awesome, it's all yours. Note that the above fix isn't meant to be the final solution, but a proof of concept, they will be a couple of rough edges to polish :).

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: checkbox This is the name of the generic UI component, not the React module! component: radio This is the name of the generic UI component, not the React module! 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