Skip to content

fix(android): keep media slot heights when Yoga snaps the view wider - #4

Merged
juliusmarminge merged 3 commits into
pingdotgg:fork/t3-mobilefrom
voida6:fix/android-media-slot-pixel-snap
Oct 15, 2026
Merged

juliusmarminge merged 3 commits into
pingdotgg:fork/t3-mobilefrom
voida6:fix/android-media-slot-pixel-snap

Conversation

@voida6

@voida6 voida6 commented Oct 15, 2026 •

Copy link
Copy Markdown

What/Why?

On Android, React-rendered media slots (T3 Code's captioned markdown images) overlap the content after them on some densities. Reported in pingdotgg/t3code#19914.

The cause is a width mismatch between measurement and layout:

  • Yoga measures the document at its unrounded width, then rounds the measured node outward to whole pixels. The laid-out view can therefore be up to 2px wider than the width the measure pass saw.
  • React measures each slot at the laid-out width.
  • MediaSlot.heightForWidth only accepted a measured height within 0.5dp of the requested width.
  • At 2.8125x the measure pass saw 970.3125px (345.0dp) and the view was 972px (345.6dp). Every measured height was rejected, so the document was sized from the 16:9 fallback while MediaSlotView drew the real heights.

The fix:

  • heightForWidth takes a width tolerance.
  • MediaSlotView.heightPx passes 0.5dp plus 2px. It is the one place all three Android measure paths go through: the root measure pass, segment heights, and the view's onMeasure.
  • A real width change (rotation, resize) still invalidates the measured height until React measures again.

Takeover changes (by @juliusmarminge, on top of @voida6's fix):

  • iOS: ENRMMediaSlotHeight had the identical 0.5pt check against the same pixel-snapped width. It now allows 1.5pt, which is 0.5pt plus 2px at 2x, and covers 3x as well.
  • Test: the original test passed its own tolerance to heightForWidth, so it still passed with the fix removed. It is replaced by MediaSlotViewTest, which calls MediaSlotView.heightPx at 2.8125x with the logged widths. It fails without the tolerance and passes with it.

Upstream: none. Media slots exist only in this fork. Upstream closed those PRs in favour of designing media rendering themselves (software-mansion#54), so this goes away when the fork moves to their version.

Testing

  • :react-native-enriched-markdown:testDebugUnitTest (all suites) and ktlintCheck pass.
  • MediaSlotViewTest fails with the MediaSlotView tolerance removed and passes with it.
  • A read-only review traced Yoga's rounding (under 2px, and it does not compound across nesting) and the JS-side width gate (mediaOverrideForFrame compares two widths taken after layout, so it was never the rejecting gate).
  • iOS: one constant changed. clang-format is clean. Not built separately.

Originally reported and fixed by @voida6 in this PR.

Yoga measures the markdown view at its unrounded width, then snaps the
measured node outward to whole pixels (floor left, ceil right), so the
laid-out view can be up to 2px wider than the width the measure pass saw.
React measures each media slot at that laid-out width. The measure pass
rejected those overrides whenever the gap reached 0.5dp (for example
970.3px measured vs 972px laid out at 2.8125x) and sized the document from
the 16:9 fallback, while the view drew the real heights. Captioned images
then overlapped the following content and the message footer.

Allow the 2px snap on top of the existing 0.5dp tolerance.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1f9e29b0-7e53-457e-93b0-d2975faeaa0d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one to land it in the fork and ship it in T3 Code. I'll keep you credited and post what changed here.

juliusmarminge and others added 2 commits October 15, 2026 12:16
…al measure path

Replaces the parameter-only test with a MediaSlotView.heightPx test at
2.8125x that fails without the tolerance, and applies the same 2px
allowance to ENRMMediaSlotHeight, which had the identical 0.5pt check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants