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

[Joy] Make Icon fontSize adaptable to its parent #31268

Merged
merged 6 commits into from Mar 4, 2022

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Mar 2, 2022

This is an experimental feature called "Adaptive component".

It gives the parent component the ability to control the children without hurting customization experience.

The problem

In Material UI, it is using CSS selector to control the children which leads to bad customization experience. #28917

<Button size="small" startIcon={<Icon />}>...</Button>
// some icon looks great by default, but for some icon needs customization

// here is what I expect to do with sx
<Button size="small" startIcon={<Icon sx={{ fontSize: '18px' }} />}>...</Button>
//❗️it does not work, and then I have to open a devtool and found that it is using CSS selector

// Finally, ends up with this
<Button size="small" startIcon={<Icon />}
  sx={{[`& .${buttonClasses.startIcon} > *:nth-of-type(1)`]: {
    fontSize: "30px"
  }}}
>...</Button>
// 🥲 too much thing to do just to change the size of the icon

The solution

create CSS variable with a default value, so by default <Icon /> renders this:

<Icon /> // font-size: var(--Icon-fontSize, 1.5rem);

This allows any component to control this icon via --Icon-fontSize, for example a Button:

// Button size "sm" create a variable `--Icon-fontSize: 20px`
<Button size="sm" startIcon={<Icon />}>
// it looks great by default

Customization

When developer defines a fontSize prop to the instance, it creates the variable at the icon, so the value has the highest priority.

// `--Icon-fontSize: 1rem`
<Icon fontSize="md" />

Meaning, the variable from the parent has no effect 😲. I think this is the best experience so far.

A plus of using CSS variables is that developers do not need to use our theme (in js) to set the default props, they can use any CSS solution without any setup 😲

// any CSS solution, ex. global.css
:root {
  --Icon-fontSize: 1.25rem; // apply to all default <Icon />
}

@mui-bot
Copy link

mui-bot commented Mar 2, 2022

Details of bundle changes

@mui/joy: parsed: +0.44% , gzip: +0.43%

Generated by 🚫 dangerJS against 354b3a1

@siriwatknp siriwatknp marked this pull request as ready for review March 2, 2022 03:09
@siriwatknp siriwatknp added the package: joy-ui Specific to @mui/joy label Mar 2, 2022
@mnajdova
Copy link
Member

mnajdova commented Mar 2, 2022

What are the downsides of using this API? Some I could think of:

  • we cannot rely on default props for setting the font size and colors, as it would always override the CSS variable
  • we are leaking API abstraction trough the CSS variables, they basically become an API too (we can't avoid this anyway at this point)

What would be the recommended approach when using custom icons?

@siriwatknp
Copy link
Member Author

we cannot rely on default props for setting the font size and colors, as it would always override the CSS variable

You can (check my last commit). The reason I didn't add it before is that it touches @mui/material but now I think it is okay because Material UI consumers would not know about the instanceFontSize in the ownerState.

we are leaking API abstraction trough the CSS variables, they basically become an API too (we can't avoid this anyway at this point)

Partially right. At this point, we don't even need to document that we are using CSS variables because you can do this without the knowledge of CSS variables:

<Button size="sm" startIcon={<Icon />}> // works
<Button size="sm" startIcon={<Icon fontSize="sm" />}> // works
<Button size="sm" startIcon={<Icon sx={{ fontSize: '20px' }} />}> // works

What would be the recommended approach when using custom icons?

Let's use font icon for example:

<Button size="sm" startIcon={<i className="font-awesome ..." />}>
.font-awesome {
  font-size: var(--Icon-fontSize, 20px);
}

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Alright, let's try this and see how it will scale.

Important thing to think about for the API is how do we document the css variables available in the component. We should also make sure we use them consistently after the stable release, as they are basically a public API.

@siriwatknp siriwatknp merged commit 63fbc87 into mui:master Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: joy-ui Specific to @mui/joy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants