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

Could we make classesStyles be applied after tagsStyles? #35

Closed
TheaLanherne opened this issue Oct 18, 2017 · 3 comments
Closed

Could we make classesStyles be applied after tagsStyles? #35

TheaLanherne opened this issue Oct 18, 2017 · 3 comments
Labels
pr:review needed This PR requires review from maintainers.

Comments

@TheaLanherne
Copy link

Just wanted to say thanks for the great library, we're really impressed with the performance of it vs using a WebView!

We've noticed that, at the moment, tagsStyles override classesStyles - e.g. with the following code, the last-paragraph will come out grey rather than teal, because the styling for 'p' tag is applied after the "last-paragraph" class styling:

// props
    tagsStyles: { p: { color: 'grey' } },
    classesStyles: { 'last-paragraph': { color: 'teal' } }

const html = `
    <p class="last-paragraph">Finally, this paragraph is style through the classesStyles prop</p>`;

We think it would make more sense to swap this round to be more similar to CSS Specificity, where class selectors take precedence over type selectors. What do you think?

I think it might be as simple as just swapping over lines 324 and 325 in HTML.js

Let me know if you approve of this change and I can submit a PR for it. I just wanted to check whether you agree with the idea of the change first!

@Exilz
Copy link
Contributor

Exilz commented Oct 18, 2017

Hi,
thanks for the feedback !

You're right. Actually, I realized this and recently made the change recently. I'm about to push it on the development branch.

It will land on the next version with some new features and bugfixes.

@TheaLanherne
Copy link
Author

Awesome, thank you!

Exilz added a commit that referenced this issue Oct 18, 2017
@Exilz Exilz mentioned this issue Oct 19, 2017
@Exilz
Copy link
Contributor

Exilz commented Oct 19, 2017

This landed in 3.5.0-rc.1. Since this release is quite important, testing would be appreciated !

@Exilz Exilz added the pr:review needed This PR requires review from maintainers. label Oct 26, 2017
@Exilz Exilz closed this as completed Oct 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:review needed This PR requires review from maintainers.
Projects
None yet
Development

No branches or pull requests

2 participants