RX.FastText for plain text?

[antonkh]

In UWP there are two elements for rendering text: RichTextBlock and TextBlock. The latter is faster, but supports only plain text. Hence there is a proposal to implement this in RNW: microsoft/react-native-windows#1256

The original idea was to choose RichTextBlock or TextBlock on the fly, based on the text content: if text has no markup, RNW creates a TextBlock. However it was pointed out that this will cause problems with Narrator: if a TextBlock is swapped with a RichTextBlock while Narrator had a focus on it, the focus will be lost. This is why it was decided to make this feature opt-in.

We've been discussing two options:

1. A new flag like <RX.Text fast=true>abc</RX.Text> would tell RNW to dynamically switch between RichTextBlock and TextBlock.
2. A new element <RX.FastText value="abc"/> would render only plain text and would always create a XAML TextBlock.

Any thoughts?

[fbartho]

Why not call it RX.PlainText and on each platform it'll be implemented appropriately?

[antonkh]

I agree that PlainText is a better name.

[erictraut]

I'm against exposing this additional complexity within ReactXP. I'd also prefer not to expose it in React Native. I think we should try hard to hide this from callers.

Can you provide more details about what you mean by "no markup"? Do you mean "no style information"?

[antonkh]

That's correct: "no markup" means plain text, i.e. a RN.Text with only one child element which is string.

[erictraut]

It would be very unusual for a plain-text element would be swapped for one that has markup or vice versa. In all of the ReactXP code we've written in Skype, I doubt if this case has ever come up.

I don't think this justifies exposing additional complexity within RX or RN. We can document the issue with Navigator, and app code can easily avoid this unusual edge case if they care about this focus behavior.

[antonkh]

I think such a case exists: the last message preview in the conversation icon element. If Narrator is focusing on this text element and someone sends another message with styles, TextBlock will be swapped with RichTextBlock.

If RX exposes neither RX.PlainText nor the additional property in RX.Text, app code will still have a way to avoid the Narrator issue: by wrapping the RX.Text in question into a RX.View so Narrator could keep focus on that view. It will also need to use collapsabe=false as otherwise RN will remove the layout-only view.

[erictraut]

If I understand you correctly, TextBlock is used only for JSX text nodes that have no tags. Let's consider the following example.

<Text style={myStyle}>Raw Text</Text>

This JSX would be translated to into code that constructs two nodes — a TextBlock with a value of "Raw Text" and a RichTextBlock that contains the first node as a child. Do I have that correct?

If so, then I don't see the problem because the focus would be on the outer RichTextBlock.

[antonkh]

I think that's correct but I would confirm with @rozele

[rozele]

I'm not particularly attached to either approach. I'm happy to expose additional props on for Windows that either enables or disables the optimization (i.e., whitelist or blacklist), or exposing a different component altogether. I like the idea of using a prop to disable the optimization because it's trivial to keep your components cross-platform, and platform-specific props are already a pervasive thing in React Native. That being said, it's pretty trivial to implement both, and call the TextBlock component something like PlainTextWindows as platform-specific components are also pervasive in React Native.

I agree with @erictraut that creating different components for text makes end-user development more complex, especially for such a core component. Ideally we just hide the optimization. One approach might be to have a global toggle for whitelist vs. blacklist of the (defaulting to whitelist), so we can write up a performance doc that says "switch the TextOptimization toggle to true (i.e., blacklist mode) and your text rendering performance may improve (with a slight risk for breaking screen reader experiences)."

[erictraut]

If my understanding is correct, there's no need to expose anything or provide an optimization switch of any sort. Other RN implementations already differentiate between "RCTText" and "RCTRawText" nodes. Those are implementation details that are never exposed to RN users. It sounds like this is the same thing.

[antonkh]

I think it should be enough to mention in the docs this behavior and tell to wrap the text into a View if it changes:

• If contents of RX.Text may change, wrap it into a RX.View and annotate it with accessibility props so the Narrator could keep focus on it.
• If contents are immutable, no additional changes needed.

[antonkh]

@rozele What's the difference between RCTRawText and RCTFastText? My impression is that RCTRawText corresponds to "text node" in HTML and is only needed to wrap unstyled chunks of text inside other text elements. For instance

<View style={styles.container}>
    <Text>
        <Text style={styles.welcome}>
        Welcome to React Native!
        </Text>
        {'\n'}To get started, edit index.windows.js
    </Text>
    <Text style={styles.instructions}>
        Shake or press Shift+F10 for dev menu
    </Text>
</View>

turns into

[4,"RCTView",1,{"flex":1,"justifyContent":"center","alignItems":"center"}]
    [5,"RCTText",1,{"accessible":true,"allowFontScaling":true,"lineBreakMode":"tail"}]
        [6,"RCTVirtualText",1,{"fontSize":20,"fontWeight":"bold","textAlign":"center"}]
            [7,"RCTRawText",1,{"text":"Welcome to React Native!"}]
        [8,"RCTRawText",1,{"text":"\n"}]
        [9,"RCTRawText",1,{"text":"To get started, edit index.windows.js"}]
    [10,"RCTFastText",1,{"color":4281545523,"text":"Shake or press Shift+F10 for dev menu"}]

so

• a Text element with mixed styling, i.e. with nested Text elements and strings, becomes a RCTText
  • RCTRawText represents nested plain strings. I think React needs this special "text wrapper node" to simplify the interface: subnodes can only be other RCT elements (not just plain strings or numbers or anything else) and if we want to make a plain string a child node, we wrap it with a RCTRawText and make the string an attribute of that wrapper node. Later these RCTRawText nodes become inlines in XAML RichTextBlock.
  • RCTVirtualText corresponds to a React Text element placed inside another Text element. I suppose that RCTVirtualText also becomes an inline in RichTextBlock.
• a Text element with simple styling, i.e. with one style for the entire string, becomes a RCTFastText which is then mapped to XAML TextBlock.

@erictraut, regarding your earlier question about <Text style={myStyle}>Raw Text</Text>, I think I was wrong. This element will be rendered as a single RCTFastText. In XAML there will be only one TextBlock (it allows to set text styles). Then if we change contents of this Text element to something more complex, e.g. <Text style={myStyle}>Raw Text <Text style={innerStyle}>123</Text></Text>, RNW will have to replace TextBlock with RichTextBlock and Narrator will lose focus. However if the optimization was off, RNW would only update the inlines collection in the RichTextBlock.

With all that in mind, I think the global flag to turn on the optimization, is the best solution if we don't want to modify the RX.Text interface and once the optimization is turned on, the app needs to wrap all mutable text elements into view containers if the text elements are expected to switch between "rich" and "plain" modes. The view container will likely need some special attributes (@zholobov?) to tell Narrator to keep focus on this view and not on the child text element.

[zholobov]

@antonkh The view container would need to set importantForAccessibility to "yes" to disallow screen readers to get to the real text element which can be replaced dynamically.

We can also automagically do this for user even if the view wrapping textblock does not have the importantForAccessibility property set, that is when it has the default "auto" value. But since user will have to manually wrap the textblock anyway I do not think it brings any value at least matching the lost predictability.

[erictraut]

Do we have a clear sense for how much perf this optimization gains in real-world scenarios (as opposed to simple synthetic benchmarks)? If it's not significant, I'd still prefer not to expose any of this complexity. I will need to see a really compelling argument to justify exposing any of this in ReactXP's interface or documentation.

[antonkh]

In S4L the gain is visible: ~1.3x speedup in conversations switching. It's also interesting that it saves a lot of memory: the RichTextBlock version of S4L can easily go up to 1.5 GB, while the TextBlock version hardly gets to 0.5 GB.

[erictraut]

OK, those impacts are too significant to ignore. Sounds like we need to expose this in some manner. Let's try to keep it minimal such that 99.9% of ReactXP usage doesn't need to know about this complexity. It's very rare that a component both 1) dynamically switches between RX.Text with multiple children and a single raw text child and 2) cares about retaining the screen navigator focus across such a switch. We know of one such example in the Skype app, but we'd probably be hard pressed to find a second real-world example.

[antonkh]

I agree that enabling this optimization by default (with a note in the docs to wrap such mutable texts into views) should be fine. @rigdern?

[rigdern]

@antonkh Let's wait until @zholobov finishes his accessibility implementation. Then you can verify your proposed strategy for fixing the accessibility issue caused by the FastText optimization. At that point we'll have all of the information we need to decide on an API for controlling the FastText optimization.

[erictraut]

Sounds like this won't be needed. Closing for now. We can re-open if it turns out to be required.