[5.x] Added textProps prop #349

Peretz30 · 2020-03-13T13:36:34Z

Warning! Breaking change
Added textProps prop that replace existing textSelectable and allowFontScaling. Now, with this prop you are able to apply any text prop (selectable, textBreakStrategy, etc.) to text wrapper component.

Example usage:

                    <HTML
                        html={
                            newsItem.bodyHtml ||
                            '<div></div>'
                        }
                        imagesMaxWidth={maxWidth}
                        onLinkPress={onLinkPress}
                        tagsStyles={backendStyles.tagsStyles}
                        baseFontStyle={styles.text}
                        textProps={{ textBreakStrategy: "simple" }}
                    />

lingtran · 2020-03-24T20:12:24Z

Awesome, thanks for creating this pull request. I totally understand folks may be swamped at the moment. There have been a number of requests for this fix since Android Q made the text change. For our team we aren't able to simply add the textBreakStrategy prop directly. So we've created a fork with this fix. It would be great if this can be reviewed by a few of the maintainers and merged soon.

jjhampton · 2020-05-14T20:35:38Z

Please review and merge, this is a necessary fix w/ changes that Android Q introduced with text wrapping.

kuetsch · 2020-05-28T09:43:19Z

This would really be helpful! Would love to see the next release incorporate this fix.
Btw. I think there is one more location where the fix needs to be applied: Its the textwrapper-Function in HTMLRenderers.js

kamalpandey · 2020-06-03T07:37:10Z

Waiting for this one to get review and merged.

jsamr

Thank you so much for your participation, this is a great idea. I've added a few suggestions!
Because this feature would break current API, it will require a 5.0 release. But that sounds reasonable. We have a lot of pending PR that ought to be merged!

README.md

src/HTML.js

…der-html

Peretz30 · 2020-07-08T22:51:40Z

@jsamr thanks for your suggestions. Please, take a look at PR now. Looks like there are still some formatting problems. If it's critical. I can resolve them some later.

jsamr

See my comments! Sorry about the formatting issues: it would be much better if we enforced a formatting style with a linter and add some guidance for PRs. Will certainly do in the future though.

jsamr · 2020-07-09T12:33:03Z

src/HTML.js


            const renderersProps = {};
            if (Wrapper === Text) {
-                renderersProps.allowFontScaling = allowFontScaling;
-                renderersProps.selectable = textSelectable;
+                Object.keys(textWrapperProps).map(propName => { renderersProps[propName] = textWrapperProps[propName] });


I would rather go with:

let renderersProps = {}; if (Wrapper === Text) { renderersProps = textWrapperProps }

which I find easier to read

jsamr · 2020-07-09T12:40:08Z

src/HTML.js

-                        tagName,
-                        nodeIndex
-                    };
+            .filter((parsedNode) => parsedNode !== false && parsedNode !== undefined) // remove useless nodes


Because it is unnecessary formatting, you can reset this hunk to its master state.
To do so, you can do:

git checkout --patch src/HTML.js master

It will prompt you to select the hunks you want to restore to master's state, and those you want to keep ;-)

jsamr · 2020-07-09T12:40:52Z

src/HTML.js

-                    if (TextOnlyPropTypes.indexOf(styleKey) !== -1) {
-                        textChildrenInheritedStyles[styleKey] = wrapperStyles[styleKey];
-                        delete wrapperStyles[styleKey];
+                    let textChildrenInheritedStyles = {};


jsamr · 2020-07-09T12:41:01Z

src/HTML.js

-                        textChildrenInheritedStyles[styleKey] = wrapperStyles[styleKey];
-                        delete wrapperStyles[styleKey];
+                    let textChildrenInheritedStyles = {};
+                    Object.keys(wrapperStyles).forEach((styleKey) => {


jsamr · 2020-07-09T12:41:24Z

src/HTML.js

-                        delete wrapperStyles[styleKey];
+                    let textChildrenInheritedStyles = {};
+                    Object.keys(wrapperStyles).forEach((styleKey) => {
+                        // Extract text-only styles


jsamr · 2020-07-09T12:41:37Z

src/HTML.js

-                                ...textChildrenInheritedStyles,
-                                ...cssStringToObject(child.attribs.style)
-                            });
+                        if (wrapper === 'Text') {


jsamr · 2020-07-09T12:41:48Z

src/HTML.js

-            }
-            return parsedNode;
-        });
+                return parsedNode;


This reverts commit d2c5175.

This reverts commit ea0488f.

…tive-render-html" This reverts commit 5e23fb9, reversing changes made to 28d57f2.

Peretz30 · 2020-07-28T18:08:42Z

hey @jsamr ! Sorry for late response, I was very busy last week. I cleaned up code formatting. Check this out, please.

jsamr

After you offered this PR the first time, we published guidelines for PRs in which we would require:

Implementing tests on the new feature.
Changing the typescript definitions to reflect API changes.

In addition, I will ask you to rebase on master as you will have to resolve some conflicts. After which you can run some tests. There is notably a test about textSelectable that could fail with your changes, but should be an easy fix.

If you have any question or looking for advise on any of those steps, you're welcome to ping me in our Discord #contributing channel

jsamr · 2020-08-05T09:16:28Z

README.md

@@ -90,8 +90,7 @@ Prop | Description | Type | Required/Default
 `emSize` | The default value in pixels for `1em` | `number` | `14`
 `ptSize` | The default value in pixels for `1pt` | `number` | `1.3`
 `baseFontStyle` | The default style applied to `<Text>` components | `object` | `{ fontSize: 14 }`
-`allowFontScaling` | Specifies whether fonts should scale to respect Text Size accessibility settings | `boolean` | `true`
-`textSelectable` | Allow all texts to be selected | `boolean` | `false`
+`textWrapperProps` | Object with props, that would be applyed to text wrapper. For example `{allowFontScaling: 'false'}` | `object` | `{}`


jsamr · 2020-08-05T09:33:12Z

src/HTML.js

@@ -504,7 +500,7 @@ export default class HTML extends PureComponent {
                remoteErrorView(this.props, this.state) :
                (
                    <View style={{ flex: 1, alignItems: 'center' }}>
-                        <Text allowFontScaling={allowFontScaling} style={{ fontStyle: 'italic', fontSize: 16 }}>Could not load { this.props.uri }</Text>
+                        <Text { ...textWrapperProps } style={{ fontStyle: 'italic', fontSize: 16 }}>Could not load { this.props.uri }</Text>


Added textProps prop that would be applyed to text wrapper

058a829

Peretz30 mentioned this pull request Mar 13, 2020

Allow sending textBreakStrategy to <Text> elements #305

Closed

kamalpandey mentioned this pull request Jun 3, 2020

Last pragraph sentence of some html text gets cuts off i.e not shown #360

Closed

jsamr requested changes Jul 5, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

src/HTML.js Outdated Show resolved Hide resolved

jsamr added the is:waiting for feedback Waiting for OP input. label Jul 5, 2020

jsamr mentioned this pull request Jul 5, 2020

Fix textSelectable not working for tagName textwrapper #268

Closed

jsamr linked an issue Jul 5, 2020 that may be closed by this pull request

Last pragraph sentence of some html text gets cuts off i.e not shown #360

Closed

jsamr mentioned this pull request Jul 5, 2020

How to limit content number line? #243

Closed

Peretz30 added 2 commits July 9, 2020 01:46

Added textWrapperProps prop

28d57f2

Merge branch 'master' of https://github.com/Peretz30/react-native-ren…

5e23fb9

…der-html

Peretz30 requested a review from jsamr July 8, 2020 22:54

fix code formatting

d2c5175

jsamr requested changes Jul 9, 2020

View reviewed changes

jsamr added the pr:breaking Pull Request introducing a breaking change. label Jul 9, 2020

Peretz30 added 4 commits July 15, 2020 01:53

some refactoring

ea0488f

Revert "fix code formatting"

a7abfd8

This reverts commit d2c5175.

Revert "some refactoring"

685f1e2

This reverts commit ea0488f.

Revert "Merge branch 'master' of https://github.com/Peretz30/react-na…

dbf100b

…tive-render-html" This reverts commit 5e23fb9, reversing changes made to 28d57f2.

jsamr linked an issue Jul 19, 2020 that may be closed by this pull request

Add support for maxFontSizeMultiplier #241

Closed

fix format

8863b13

Peretz30 requested a review from jsamr July 28, 2020 18:09

jsamr requested changes Aug 5, 2020

View reviewed changes

jsamr changed the title ~~Added textProps prop~~ [5.x] Added textProps prop Nov 27, 2020

jsamr closed this in 7264cbe Dec 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[5.x] Added textProps prop #349

[5.x] Added textProps prop #349

Peretz30 commented Mar 13, 2020

lingtran commented Mar 24, 2020

jjhampton commented May 14, 2020

kuetsch commented May 28, 2020

kamalpandey commented Jun 3, 2020

jsamr left a comment

Peretz30 commented Jul 8, 2020 •

edited

Loading

jsamr left a comment

jsamr Jul 9, 2020 •

edited

Loading

jsamr Jul 9, 2020

jsamr Jul 9, 2020

jsamr Jul 9, 2020

jsamr Jul 9, 2020

jsamr Jul 9, 2020

jsamr Jul 9, 2020

Peretz30 commented Jul 28, 2020

jsamr left a comment •

edited

Loading

jsamr Aug 5, 2020

jsamr Aug 5, 2020

[5.x] Added textProps prop #349

[5.x] Added textProps prop #349

Conversation

Peretz30 commented Mar 13, 2020

lingtran commented Mar 24, 2020

jjhampton commented May 14, 2020

kuetsch commented May 28, 2020

kamalpandey commented Jun 3, 2020

jsamr left a comment

Choose a reason for hiding this comment

Peretz30 commented Jul 8, 2020 • edited Loading

jsamr left a comment

Choose a reason for hiding this comment

jsamr Jul 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Peretz30 commented Jul 28, 2020

jsamr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Peretz30 commented Jul 8, 2020 •

edited

Loading

jsamr Jul 9, 2020 •

edited

Loading

jsamr left a comment •

edited

Loading