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

[core] Change style API #6892

Merged
merged 1 commit into from May 19, 2017
Merged

[core] Change style API #6892

merged 1 commit into from May 19, 2017

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented May 17, 2017

Propose the following improvements:

  • allow all the class names to be extended with the classes property, so we stop using the xxxClassName pattern. That's pretty close to react-css-themr.
// Before
// How can I do that, should a submit a PR to expose a `roundedClassName`? 

// After
<Paper classes={{ rounded: 'my-rounded-implementation' }} />
  • remove the duplication between the theme and the styleManager as we can use
    this.context.styleManager.theme over this.context.theme.
  • I have migrated one component, the Paper with that new approach. If we are happy with that, I can convert the whole code base.
  • Update the docs
  • Automatically generate the classes docs
  • Migrate the code base
  • ref handling and instance method access

@muibot muibot added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI PR: Needs Review labels May 17, 2017
Copy link
Member

@rosskevin rosskevin left a comment

Choose a reason for hiding this comment

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

very nice!

@muibot muibot added PR: out-of-date The pull request has merge conflicts and can't be merged PR: Needs Review and removed PR: Needs Review PR: out-of-date The pull request has merge conflicts and can't be merged labels May 18, 2017
@oliviertassinari
Copy link
Member Author

@rosskevin Thanks for the feedback, I'm going one step further: auto docs generation 💅 .

Propose the following improvments:
- allow all the classnames to be extended with the classes property
- remove the duplication between the theme and the styleManager
@oliviertassinari oliviertassinari removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label May 18, 2017
@oliviertassinari oliviertassinari merged commit 8ab5299 into mui:next May 19, 2017
@oliviertassinari oliviertassinari deleted the next-classes branch May 19, 2017 07:06
rosskevin pushed a commit to alienfast/material-ui that referenced this pull request May 19, 2017
Propose the following improvments:
- allow all the classnames to be extended with the classes property
- remove the duplication between the theme and the styleManager
rosskevin pushed a commit to alienfast/material-ui that referenced this pull request May 19, 2017
Propose the following improvments:
- allow all the classnames to be extended with the classes property
- remove the duplication between the theme and the styleManager
@zannager zannager added the core Infrastructure work going on behind the scenes label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants