Benjamin-Dobell / react-native-markdown-view

MarkdownView for React Native
MIT License
190 stars 77 forks source link

Add support for replacing all styles #23

Open skovhus opened 6 years ago

skovhus commented 6 years ago

This adds a boolean replaceAllDefaultStyles indicating if all default styles should be overridden by the given styles property.

Useful if you want complete control of the styling.

skovhus commented 6 years ago

@Benjamin-Dobell anything you would like me to change here?

Benjamin-Dobell commented 6 years ago

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 replaceAllDefaultRules property.

skovhus commented 6 years ago

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.

skovhus commented 6 years ago

As it is a power feature I would argue that validation isn’t super important...

Benjamin-Dobell commented 6 years ago

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 DefaultRules from MarkdownView.js.

Then this can be implemented trivially in a wrapper component that simply maps each of the DefaultRules to a copy of itself with the exception of render; which would be overridden with a function that calls the original render method with styles from the wrapper component's props, rather than the provided styles parameter. Then pass these rules to a child MarkdownView and forward through this.props.children.

skovhus commented 6 years ago

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.

Benjamin-Dobell commented 6 years ago

I think it would be enable override all existing styles + Validate that the given styling object contains all tags.

Agreed.