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

Created custom stylable decorator #2451

Merged
merged 1 commit into from
Dec 9, 2015
Merged

Created custom stylable decorator #2451

merged 1 commit into from
Dec 9, 2015

Conversation

rob0rt
Copy link
Contributor

@rob0rt rob0rt commented Dec 9, 2015

So after some investigation, I learned that the mixin library I found doesn't really work well with React, in the sense it doesn't do field merging for static properties or function-call chaining for react built-in methods. I tried this to investigate an alternative, where I created a custom decorator for StylePropable (called Stylable) which performs the same action as using a mixin on an es5 component. So now the parallel for using StylePropable mixin in the es5 syntax is using the Stylable decorator in the es6 syntax.

This was just an experiment and I'm open to feedback. The alternative, in the style of the mixin decorator, is just making a custom mixin decorator which is React-aware. I investigated this as well, and would likely be able to use a lot of the code from the React library which does the mixin merging (I might try making a library which does this).

@rob0rt rob0rt mentioned this pull request Dec 9, 2015
7 tasks
@rob0rt
Copy link
Contributor Author

rob0rt commented Dec 9, 2015

This is based off the second of three work arounds for mixins I discussed in #2437

@alitaheri alitaheri added this to the ES6 Classes milestone Dec 9, 2015
@alitaheri
Copy link
Member

Interesting work around! I like it 👍
@oliviertassinari review and merge ^^

@rob0rt
Copy link
Contributor Author

rob0rt commented Dec 9, 2015

In the future, when everything is moved, we could kill StylePropable and move all the logic into the decorator directly. This was just me not wanting to double logic.

...target.propTypes,
};

target.prototype.mergeStyles = StylePropable.mergeStyles;
Copy link
Member

Choose a reason for hiding this comment

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

Side note : this is an utility function. I think that we should move it out of the decorator at some point.

oliviertassinari added a commit that referenced this pull request Dec 9, 2015
Created custom stylable decorator
@oliviertassinari oliviertassinari merged commit 70e2d02 into mui:es6-classes Dec 9, 2015
@alitaheri
Copy link
Member

Thanks 👍 🎉

@zannager zannager added the package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. label Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants