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

feat: Wrap MUI button and apply styles, references #1318 #1360

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ajhollid
Copy link
Collaborator

This PR is a proposal for wrapping MUI components and applying styles as discussed and referenced in #1318. It wraps an MUI Button and applies our styles from the GlobalTheme.

Please try using the component next to a regular MUI Button, all styles appear to be applied correctly to me and the component appears identical to an MUI button styled by the GlobalTheme

I propose that we use styled components as this MUI has support out of the box and this is how MUI components are styled themselves.

This will also allow us to pretty much directly copy and paste styles out of the Global theme which should speed up the process.

I suggest we standardize on this approach going forward. That said I'm completely open to disucssion on the topic if anyone has strong opinions!

Once we've decided on the method of styling we'd like to use we can go forward and wrap all the necessary components.

Copy link
Contributor

@marcelluscaio marcelluscaio left a comment

Choose a reason for hiding this comment

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

That is exactly what I had in mind, @ajhollid. I just don't see the need for calling styled. We can simply do :

const Button = ... return (<MuiButton sx={all the styles}/>)

@ajhollid
Copy link
Collaborator Author

That is exactly what I had in mind, @ajhollid. I just don't see the need for calling styled. We can simply do :

const Button = ... return (<MuiButton sx={all the styles}/>)

Sure that works too, using styled was nice because I could just directly copy paste from the theme.

I'm ok with either approach, I'll leave the call with you!

If sx makes more sense can you make the changes and push to this branch so we have it as a reference for how to proceed?

Thanks!

@jennifer-gan
Copy link
Contributor

This PR is a proposal for wrapping MUI components and applying styles as discussed and referenced in #1318. It wraps an MUI Button and applies our styles from the GlobalTheme.

Please try using the component next to a regular MUI Button, all styles appear to be applied correctly to me and the component appears identical to an MUI button styled by the GlobalTheme

I propose that we use styled components as this MUI has support out of the box and this is how MUI components are styled themselves.

This will also allow us to pretty much directly copy and paste styles out of the Global theme which should speed up the process.

I suggest we standardize on this approach going forward. That said I'm completely open to disucssion on the topic if anyone has strong opinions!

Once we've decided on the method of styling we'd like to use we can go forward and wrap all the necessary components.

Makes sense as MUI is recommending to use styled to wrap component and its overriding props

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants