Skip to content

fix(mobile): PR images directly under a caption load like standalone images - #26862

Closed
apratxxm wants to merge 1 commit into
pingdotgg:mainfrom
apratxxm:fix/mobile-pr-image-after-caption
Closed

apratxxm wants to merge 1 commit into
pingdotgg:mainfrom
apratxxm:fix/mobile-pr-image-after-caption

Conversation

@apratxxm

Copy link
Copy Markdown

Problem

On mobile, an image on the line right after a caption (<sub>Before</sub> or plain text, with no blank line between them) shares the caption's paragraph. The native renderer offers its custom image slot, which loads private source-control media through the authenticated resolver at full size, only to an image that is the paragraph's only child. So the image falls back to the inline path with the raw URL. Private user-attachments URLs return 404 there, and the image disappears; it would also render as a small inline attachment. Fixes #26663.

Change

transformNativeMarkdown (the existing pass that separates tables from text at render time) now also gives an image that is alone on its source line its own paragraph. It does this by inserting a blank line, with container markers kept, before and/or after that line:

  • Alone on its line: decided from the neighbouring nodes, so a line with thousands of images stays linear.
  • Never applied: where the blank line would move content out of its blockquote or list item (lazy continuation lines, shallower quote markers, insufficient indent), would create an indented code block, or follows a backslash hard break.
  • Unchanged: images inside a line of text, links, and code.

It adds no extra parse, and it runs only when the text contains ![.

Scope and approval

This is a small, focused fix for an obvious bug, done the way the issue's own workaround suggests (a blank line). The same paragraph rule applies to every native Markdown surface, so images alone on their own line in thread messages are separated the same way. One visible side effect: a badge alone on its own line now renders as a block image. Badges on a shared line are unchanged. Routing inline images through resolveImageSource was considered and not chosen: the PR media resolver is an async hook, and inline images would still render small.

Verification

  • Focused test in apps/mobile/src/lib/markdownHtml.test.ts. It runs markdownForNativeRendering, then the renderer's own md4c parser (react-native-enriched-markdown/src/web/parseMarkdown), and asserts each Image is its paragraph's only child.
    • On main it fails on the issue's <sub> caption case.
    • With this change it passes, with rows for:
      • a caption above, a caption below, and captions between two images
      • a blockquote and a list item
      • CRLF, a lone CR, and a two-space hard break
      • an image in the middle of a sentence, which stays inline
    • Lazy quote/list lines and a backslash break come out byte-identical.
  • Neighbouring tests: the 6 mobile Markdown test files pass (155 tests).
  • vp run check:changed: passes.
  • Performance, local only: 8,000 images on one line render in 339 ms, versus 391 ms just to parse.
  • Not run: a native iOS or Android render. This was done on Windows, with no simulator.

Claude Opus 5.5 (claude-opus-5-5) via Claude Code; reviewed by GPT-6.1 Sol (Codex).

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 29, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 50dcb39

Macroscope's review found this PR approvable — Images on their own line now render as standalone mobile images even when adjacent to captions, using the existing image-loading path. The change is limited to presentation, with guards to preserve list and blockquote structure.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between f855f01 and 50dcb39.

Files selected for processing: 2
  • apps/mobile/src/lib/markdownDocument.ts
  • apps/mobile/src/lib/markdownHtml.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The native Markdown transformation now separates eligible standalone images from adjacent text when table breaking is enabled. It preserves existing line endings, avoids duplicate breaks, and skips images that do not meet the container and indentation checks. Tests cover placement and unchanged cases.

Changes: Native Markdown image handling

Layer / File(s) Summary
Detect and separate standalone images
apps/mobile/src/lib/markdownDocument.ts, apps/mobile/src/lib/markdownHtml.test.ts
The paragraph visitor inserts line breaks around eligible standalone images, and the final edit pass sorts edits by source position. Tests cover images around captions in paragraphs, quotes, and lists, as well as inline images and cases that remain unchanged.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: bugfix, size:M

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to 50dcb

Standalone images are separated from captions; inline images remain unchanged. No merge-blocking risk was identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Description check Passed The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the bug, implementation, scope decision, focused tests, broader checks, performance re…
Title check Passed The title is concise, specific, and accurately describes the mobile fix for images placed directly below captions.
Linked Issues check Passed [#26663] The change detects Markdown images in transformNativeMarkdown and inserts paragraph breaks only when an image occupies its source line. The focused test passes the caption-above-image case …
Out of Scope Changes check Passed The diff changes only apps/mobile/src/lib/markdownDocument.ts and its focused regression test. The implementation supports the linked image-rendering fix and preserves existing table and file-link p…
Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the bug, implementation, scope decision, focused tests, broader checks, performance result, and unrun native rendering checks.

Full details: Linked Issues check

Explanation

[#26663] The change detects Markdown images in transformNativeMarkdown and inserts paragraph breaks only when an image occupies its source line. The focused test passes the caption-above-image case through markdownForNativeRendering and parseMarkdown, and verifies a paragraph-only Image. This enables the existing standalone image path. PullRequestMarkdown supplies renderImage, and its MarkdownImage uses usePullRequestMediaUrl, which provides authenticated media loading for the standalone path. Inline images remain unchanged as documented. The test covers quotes, lists, newline forms, hard breaks, and inline images.

Full details: Out of Scope Changes check

Explanation

The diff changes only apps/mobile/src/lib/markdownDocument.ts and its focused regression test. The implementation supports the linked image-rendering fix and preserves existing table and file-link processing. The broader native Markdown behavior and the documented badge effect are direct consequences of the shared transform, not unrelated work.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Copy link
Copy Markdown
Member

Closing in favour of #24735, which makes the same change. Thanks for the work here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: PR description images after captions bypass authenticated media loading without a blank line

2 participants