React-native-render-html: classesStyles properties don't override baseFontStyle properties

Created on 13 Feb 2018 · 9Comments · Source: meliorence/react-native-render-html

classesStyles properties override the corresponding baseFontStyle properties only if the tagsStyles have the same properties.

For example:

<HTML
    html={'Just text <p>Normal paragraph</p><p class="smallRedFont">Small red paragraph</p>'}
    baseFontStyle={{ fontSize: 20, color: 'green' }}
    tagsStyles={{ p: { lineHeight: 30 } }}
    classesStyles={{ smallRedFont: { fontSize: 10, color: 'red' } }}       
 />

fontSize and color properties of smallRedFont class won't override fontSize and color of baseFontStyle, because tagsStyles for p doesn't include these properties.

In this case, however,

<HTML
    html={'Just text <p>Normal paragraph</p><p class="smallRedFont">Small red paragraph</p>'}
    baseFontStyle={{ fontSize: 20, color: 'green' }}
    tagsStyles={{ p: { fontSize: 16, color: 'blue' } }}
    classesStyles={{ smallRedFont: { fontSize: 10, color: 'red' } }}       
 />

fontSize and color properties of smallRedFont class will override fontSize and color of baseFontStyle, because tagsStyles for p does have these properties.

bug waiting for feedback

Source

yamov

Most helpful comment

Hi, thanks for the very precise explanation and reproduction of this issue.

Could you try replacing the line 444 from HTML.js

from :

const textElementStyles = this.filterBaseFontStyles(element, classStyles, props);

const textElementStyles = parentTag && TEXT_TAGS.indexOf(parentTag) !== -1 ? {} : this.filterBaseFontStyles(element, classStyles, props);

Basically, what I've done here is not to apply baseFontStyle to text nodes nested inside other text nodes, since the parent should be the one receiving the style anyway.

I'm pretty concerned with regressions, and the demo isn't complete enough for me to check all use cases. I'd like you to tell me if it fixed it for you, and if you see any potential drawbacks.

Exilz on 13 Feb 2018

🎉1 👍1

All 9 comments

Hi, thanks for the very precise explanation and reproduction of this issue.

Could you try replacing the line 444 from HTML.js

from :

const textElementStyles = this.filterBaseFontStyles(element, classStyles, props);

const textElementStyles = parentTag && TEXT_TAGS.indexOf(parentTag) !== -1 ? {} : this.filterBaseFontStyles(element, classStyles, props);

Basically, what I've done here is not to apply baseFontStyle to text nodes nested inside other text nodes, since the parent should be the one receiving the style anyway.

I'm pretty concerned with regressions, and the demo isn't complete enough for me to check all use cases. I'd like you to tell me if it fixed it for you, and if you see any potential drawbacks.

Exilz on 13 Feb 2018

🎉1 👍1

Hey, @Exilz

Your solution doesn't seem to work properly :/.
It solves the case of classesStyles but breaks some other cases. Namely, it seems like baseFontStyle does not apply to tags (without classes) anymore.

yamov on 14 Feb 2018

@yamov @fvsch

Allright, so I took some more time to dig into this problem, which seems closely tied to #119 and it's way more complicated than what I originally thought.

I think the whole logic of text style inheritance needs to be rewritten, filterBaseFontStyle and those overridenFrom... variables just don't cut it.

Here's the logic I came up with that I need to try out :

DON'T apply any styling to the text nodes that aren't mapped as rawtext, only compute the final styles for each rawtexts. This just seems easier overall.
For each rawtext, recursively loop from the deepest text parent to the highest.
Create an empty object that will receive the final text styles for this rawtext
On each iteration, create a style object with the proper priority order : htmlAttribs > classesStyles.class > tagsStyles.tag > baseFontStyle
Only merge the keys that aren't yet applied to the final object. ie: if fontSize is already set in the first iteration, ignore the fontSize that we got from the 3rd iteration because of a class for instance.

Let's take this snippet as an example :

<p>
    Texte paragraphe
    <span class="red-p">red span </span>
    <span class="blue-p">
         blue<em> span</em>
    </span>
</p>

And those props :

            baseFontStyle: { fontSize: 20, color: 'green' },
            classesStyles: {
                'red-p': { fontSize: 10, color: 'red' },
                'blue-p': { fontSize: 10, color: 'blue' }
            }

If I'm not mistaken, the above algorithm should map this HTML to the following native components :

                <Text paragraph>
                    <Text raw style={{ fontSize: 20, color: 'green' }}>Texte paragraphe</Text>
                    <Text span>
                        <Text raw style={{ fontSize: 10, color: 'red' }}>red span</Text>
                    </Text>
                    <Text span>
                        <Text raw style={{ fontSize: 10, color: 'blue' }}>blue</Text>
                        <Text em>
                            <Text raw style={{ fontSize: 10, color: 'blue', fontStyle: 'italic' }}>span</Text>
                        </Text>
                    </Text>
                </Text>

I'll try to implement this and keep you posted.

Exilz on 23 Mar 2018

Holy moly, I think I'm getting there. I'll create a separate branch so you can take a look at it and test it yourselves. This won't be part of the next release since this is quite a major rewrite.

Exilz on 23 Mar 2018

👍1

A first version of these changes is available for testing in the development branch. I'm looking forward to your feedback.

Exilz on 23 Mar 2018

Thanks, I’ll take a look on monday.

fvsch on 23 Mar 2018

@Exilz I had a related problem using version 3.9.1. fontSize would not apply, when I was using classesStyles. Here is a test to reproduce it:

The test branch works:
"react-native-render-html": "https://github.com/archriss/react-native-render-html#2fc1f8af99aeb76df1e5816a1d629b27058606ad",

```
const alterNode = (node) => {
if (node.name === 'p' && !node.parent) {
node.attribs.class = 'blue-p'
}
}

return (
baseFontStyle={{ fontSize: 20, color: 'green' }}
alterNode={alterNode}
classesStyles={{
'blue-p': { fontSize: 100, color: 'blue' }
}}
html={<p>Testing</p>}
/>
)

CarstenHoyer on 10 Apr 2018

@CarstenHoyer glad to hear the development branch fixed this for you. I have been testing these changes on our applications, and it looks like it fixed a lot of wrong behaviors.

However, please note this broke some renderers that shouldn't have been working before but were using bugs to their advantage.

I'll be releasing this soon, along with some small new features.

More reviews are always welcome :)

Exilz on 10 Apr 2018

I've tested this as well and the dev branch works nicely for me.