fixed (iOS) setState is not working properly for text inline image #41236

[ehsemloh]

Summary:

Closes #41236

setState is not working properly for text inline image

Fixed demo (please see the animation as in rendering pass rather than re-mounting pass)

Screen.Recording.2023-11-02.at.11.15.47.mov

How it works

Background

Inline views are not included in the Yoga node tree, rather, they are retained as attachments of NSAttributedString and are managed by the respective text fragment (RCTTextShadowView) that includes them (Code snippet 1).

<div layout="width: 393; height: 852; top: 0; left: 0;" style="" >
  <div layout="width: 393; height: 852; top: 0; left: 0;" style="flex: 1; " >
    <div layout="width: 393; height: 852; top: 0; left: 0;" style="flex: 1; " >
      <div layout="width: 393; height: 241; top: 0; left: 0;" style="padding-top: 59px; " >
        <div layout="width: 393; height: 50; top: 59; left: 0;" style="width: 100%; height: 50px; " >
          <div layout="width: 393; height: 17.3333; top: 0; left: 0;" style="" has-custom-measure="true"></div>
        </div>
        <div layout="width: 393; height: 50; top: 109; left: 0;" style="width: 100%; height: 50px; " >
          <div layout="width: 393; height: 17.3333; top: 0; left: 0;" style="" has-custom-measure="true"></div>
        </div>
        /* Text node that does not contain inline view that is supposed to be there */
        <div layout="width: 393; height: 74.3333; top: 167; left: 0;" style="margin-top: 8px; " has-custom-measure="true"></div>
      </div>
    </div>
  </div>
</div>

Code snippet 1, output of YGNodePrint() in normal layout flow

The layout of such node is handled ad-hoc (inline layout) inside RCTTextShadowView (Code snippet 2)

/* Inline node is calculated on its own */
<div layout="width: 48; height: 48; top: 0; left: 0;" style="overflow: hidden; width: 48px; height: 48px; min-width: 0px; min-height: 0px; " ></div>

Code snippet 2, output of YGNodePrint() in inline layout flow

Problem description

The issue happens when the sizes given by setState() are smaller than those in the last round setState(). Since the min-width and min-height are already populated (Code snippet 3) with greater values, the new layout pass gives rather a noop.

/* min sizes are greater than them in the new style */
<div layout="width: 48; height: 48; top: 0; left: 0;" style="overflow: hidden; width: 32px; height: 32px; min-width: 48px; min-height: 48px; " ></div>

Code snippet 3, output of YGNodePrint() in inline layout (issue) flow

Fix description

This biased min-width and min-height are given using the current frame size (i.e., sizes set in the last round setState()) in the inline layout (in RCTTextShadowView § Background), whilst the same parameters are given as CGSizeZero _minimumSize in normal layout (§ Background).

The change of this PR is to unify this behavior of normal layout by using CGSizeZero _minimumSize as the input also for inline layout.

Changelog:

[IOS] [FIXED] - setState is not working properly for text inline image

Test Plan:

• Using rn-tester for basic verification
• Complete plan: https://docs.google.com/spreadsheets/d/1QLuqNvqX0dM4K68ygRoHDR3S0wcK5umptmjoR7KtkaY/edit?usp=sharing

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	17,661,590	+4,420
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	21,039,173	+4,449
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 53a2742
Branch: main

[lunaleaps]

Let me try testing this out as well

[lunaleaps]

@ehsemloh can you also provide more detail on how this fixes the setState issue?

[cipolleschi]

/rebase - this comment will rebase this PR on top of main.

[cipolleschi]

...and it failed to push the rebase. :/

@ehsemloh could you please rebase and push? I'd like to see that the tests on Ruby 3.2.0 are passing.
We had a failure in the infra that I fixed yesterday evening.

[ehsemloh]

@cipolleschi Thank you so much help out! Let me do that.

[ehsemloh]

@cipolleschi done.

[ehsemloh]

Hi @NickGerleman , thanks for being involved! I believe this PR is ready for review.

cc: @lunaleaps @cipolleschi

[NickGerleman]

Thank you for adding the detailed description!

[facebook-github-bot]

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[github-actions]

This pull request was successfully merged by @ehsemloh in 36ef646.

^{When will my fix make it into a release? | Upcoming Releases}