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

[Button] Multiple variant styles being applied #11932

Closed
2 tasks done
murtyjones opened this issue Jun 21, 2018 · 7 comments
Closed
2 tasks done

[Button] Multiple variant styles being applied #11932

murtyjones opened this issue Jun 21, 2018 · 7 comments
Assignees
Labels
component: button This is the name of the generic UI component, not the React module! discussion

Comments

@murtyjones
Copy link

murtyjones commented Jun 21, 2018

  • This is a v1.x issue (v0.x is no longer maintained).
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

When a variant is chosen, e.g. outlined, other variants such as flat should not be applied as classes.

Current Behavior

The button element has multiple classes applied to it despite the variant applied.
E.g. for an outlined button i'm seeing flat, flatSecondary, etc:

<button tabindex="0" class="MuiButtonBase-root-123 MuiButton-root-101 MuiButton-textSecondary-104 MuiButton-flat-105 MuiButton-flatSecondary-107 MuiButton-outlined-108" type="button">
...
</button>

Steps to Reproduce (for bugs)

Open up this sandbox example, then right-click and inspect the button element. You'll see the outlined variant in the classes, but also the flat variant.
https://codesandbox.io/s/8103lzm9q9

Context

I noticed this when the outlined variant of the MUI button had some of the flat styling properties I applied at the theme level.

Your Environment

Tech Version
Material-UI v1.0.0
React 16.4.0
browser Chrome
@oliviertassinari oliviertassinari self-assigned this Jun 21, 2018
@mbrookes
Copy link
Member

@murtyjones An outlined button is a flat button with a border, so this is an efficient reuse of styles, rather than duplicating them.

We could:

  • Duplicate the styles
  • Rename the flat style to reflect its dual purpose (breaking change)
  • Leave it as is (I haven't gone hunting, but I'm pretty sure we have other components that share styles between variants).
  • Something else?

(cc @leMaik )

@mbrookes mbrookes added component: button This is the name of the generic UI component, not the React module! discussion labels Jun 21, 2018
@mbrookes mbrookes changed the title Multiple button variants being applied [Buton] Multiple variant styles being applied Jun 21, 2018
@oliviertassinari
Copy link
Member

I'm pretty sure we have other components that share styles between variants

@mbrookes Not I'm aware of. I think that we should aim for a simple class name application logic. Trading CSS duplication for variant isolation is a tradeoff Bootstrap has been taking. It can help in simplifying the class name combination logic.

@oliviertassinari oliviertassinari removed their assignment Jun 21, 2018
@mbrookes
Copy link
Member

That's a heck of a lot of styles to duplicate in the button - for example all the contained (raised) styles are used for the FAB.

@oliviertassinari
Copy link
Member

@mbrookes I'm not saying we should duplicate. A tradeoff needs to be done. We can always keep the current one.

@oliviertassinari oliviertassinari self-assigned this Jun 21, 2018
@oliviertassinari
Copy link
Member

Alright, I'm keeping the current tradeoff and trying to simplify the rendering logic.

@murtyjones murtyjones changed the title [Buton] Multiple variant styles being applied [Button] Multiple variant styles being applied Jun 22, 2018
@murtyjones
Copy link
Author

murtyjones commented Jun 22, 2018 via email

@oliviertassinari
Copy link
Member

I have tried to simplify the classname logic in the linked pull request. You should be able to use the outlined classe key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: button This is the name of the generic UI component, not the React module! discussion
Projects
None yet
Development

No branches or pull requests

3 participants