Add a Span layer to AttributedString

[cubuspl42]

Summary:

Add a Span layer to AttributedString

Changelog:

[INTERNAL] [CHANGED] - TODO

Test Plan:

TODO

[cubuspl42]

@NickGerleman @cortinico @mdvacca

We can now start the fun part of the project.

This is a draft, but it's enough to start talking about the high-level direction we're going. I haven't actually touched the JavaScript layer yet.

Read just the top commit.

I'm suggesting a model where the new features (fill-line-gap, inline borders, ...) are supported in the following scheme:

Let's call the red node a "root node", the blue nodes "span nodes" and the more deeply nested nodes "inner nodes".

Let's call the new properties like fillLineGap, the inline padding, and inline border "span properties".

<Text style={{ color: 'red', fontSize: 20, fillLineGap: true }}>
  <Text style={{ color: 'blue', fontSize: 18, fillLineGap: true }}>
    Level 2
    <Text style={{ color: 'green', fontSize: 16 }}>
      Level 3
      <Text style={{ color: 'orange', fontSize: 14 }}>
        Level 4
      </Text>
    </Text>
  </Text>
  <Text style={{ color: 'blue', fontSize: 12, fillLineGap: false }}>
    Level 2 (line gap not filled!)
    <Text style={{ color: 'green', fontSize: 16, fillLineGap: true }}>
      Level 3 (fillLineGap ignored!)
      <Text style={{ color: 'orange', fontSize: 14 }}>
        Level 4
      </Text>
    </Text>
  </Text>
</Text>

For the root node, I suggest:

• Either treating span properties as the defaults for span nodes
• ...or ignoring them (I don't have a strong opinion here)

For the span nodes, I suggest:

• Converting the span properties to the SpanAttributes
  • ...and later to AttributedString::Span
  • ...and in the end, of course, actually render those styles

For the inner nodes, I suggest:

• Ignoring the span properties

I suggest ignoring span properties for <Paragraph> and Android text input.

This is the least invasive model I could think of. It's not the only possible one. We could think of a new hierarchy of components like <SpannedText>.

<SpannedText color="red" fontSize={20}>
  Level 1
  <TextSpan color="blue" fontSize={18}>
    Level 2
    <Text color="green" fontSize={16}>
      Level 3
      <Text color="orange" fontSize={14}>
        Level 4
      </Text>
    </Text>
  </TextSpan>
</SpannedText>

Of course, I'm not saying to drop backward compatibility with the "old-style" nesting, just to require the new style when someone wants fancy borders and such.

Pros:

• Easier to reason about
• better Flow/TypeScript type safety

Cons:

• Less forward compatible in case we'd like to support rendering nested span styles
• I don't know if you are at all open to such big API changes.

Please share your thought!

[cubuspl42]

@NickGerleman @mdvacca I would be extremely grateful for quick thoughts on the API sketch above ♥️

[cubuspl42]

@NickGerleman @mdvacca

The more I think about the second approach (SpannedText / TextSpan / Text), the more I like it. On second thought, it's as forward-compatible as the first approach, as we can always later allow nesting TextSpan inside TextSpan. It also feels right to separate actual "containers" (TextSpan), which have a very significant position in the tree, from "wrappers" (Text) which affect character styles and can be nested in various configurations, still achieving the same final effect.

Damn, we could even extract a separate kind of component for this purpose, StyledText, which would make it clearly separated from Text (which is more like Android's TextView).

I know these are pretty drastic changes, but if a new API was to be added, we could at least ensure it's designed properly.

Let me know if it is realistic in any way to add a new core component family like this.

It would be great to hear at least a few words from you about these ideas, as implementing them just to hear "absolutely no way" after things are done would be an enormous waste of time on my side.

[cubuspl42]

@cortinico Would you find some time to quickly look at this draft PR and say if it's going in the right direction?

It's a sketch of an implementation of this idea

[cortinico]

@cortinico Would you find some time to quickly look at this draft PR and say if it's going in the right direction?

It's a 2K LOC change, sadly not the easiest thing to review :/

[cubuspl42]

@cortinico Would you find some time to quickly look at this draft PR and say if it's going in the right direction?

It's a 2K LOC change, sadly not the easiest thing to review :/

It's built on top of the other PR. Would you just peek at the top commit in the single commit view?

[cubuspl42]

Actually, I'm not using the base commits yet, I can force-push this with only the top commit

[cubuspl42]

@cortinico Done. This is a single commit which aims to build the ReactCore foundations of the new structure which can represent the features I proposed. This post I mentioned before has a description (with pictures) of what I aim for.

[cubuspl42]

Essentially, this is a new model, where a rich text is not annotated with a sequence of adjacent ranges (the fragments), as it was before, but with a sequence of spans (containers), which hold a sequence of adjacent ranges (the fragments). These spans will be holding the styles of different nature; i.e. those that span around the text (like borders), instead of just providing cascading per-character styles.

There are consequences. I suggest not supporting spans-within-spans initially. It's an open question how TextInput would handle it. It's very surprising that TextInput on Android is a rich text editor, and to be honest this is considered an anti-feature in the project I work on. I would require guidance with how to integrate "spans" with TextInput. I would preferrably do not support them at all there, i.e. somehow split the logic that TextInput operates on a single root span (or directly on a sequence of fragments), not supporting actual span styling.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	17,770,976	+65,515
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	21,162,234	+65,522
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: a3f238a
Branch: main

[cubuspl42]

I'm closing this, as too much code changed in the meantime. Right now, I'm in the process of submitting smaller PRs. Eventually we'll get back to this subject.