-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
Add support for replacing all styles #23
base: master
Are you sure you want to change the base?
Conversation
@Benjamin-Dobell anything you would like me to change here? |
Hmm, I'm not 100% sold on this. I think this particular implementation makes it a bit too easy for a user to forget to specify styles. If functionality like this is to be added, then I think strong type checking on the styles must be included with it; such that apps very loudly break when a style is missing. This is particularly important if I were to add additional styles in future releases. The naming of the property also somewhat begs the question as to why there isn't an equivalent |
This is a power feature. It’s not useful for most people, but if you want complete control you need to be able to override the styles. Alternative to this would be to make it configurable whenever a style tag would be merged on not. Not 100% sure if that would also solve the issue right now. |
As it is a power feature I would argue that validation isn’t super important... |
I wouldn't consider what you're wanting to achieve a power feature. I understand why you want it, and I think such a feature (if it exists) should be easy to use. It only comes across as a power feature because the proposed implementation is dangerous. If you really want to be able to achieve this in your own project without worrying about style safety and what-not, then I think the best thing to do in the short-term would be to submit another pull request that simply exports Then this can be implemented trivially in a wrapper component that simply maps each of the |
By dangerous you mean that a the use might forget to style a specific tag? What would be the ideal solution you think? I think it would be enable override all existing styles + Validate that the given styling object contains all tags. |
Agreed. |
This adds a boolean
replaceAllDefaultStyles
indicating if all default styles should be overridden by the givenstyles
property.Useful if you want complete control of the styling.