fix(web/mobile): render project images in file nested markdown previews and harden markdown images - #7857
fix(web/mobile): render project images in file nested markdown previews and harden markdown images#7857flamboh wants to merge 13 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
One finding on the web markdown image path: workspace images now render a signed asset URL as src, which changes what the existing copy-as-markdown serializer emits. Details inline.
Posted via Macroscope — UI Consistency
f664077 to
2393f5c
Compare
ad2e233 to
f3fab02
Compare
There was a problem hiding this comment.
One finding: the workspace-image loading placeholder now receives authored pixel dimensions but lacks the height/width caps the loaded image has, so oversized authored sizes can overflow the container and cause a large layout jump on load.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding on the new authored-dimension inline sizing in apps/web/src/components/ChatMarkdown.tsx: applying width and height as independent fixed pixel values lets the shared max-h/max-w caps clamp each axis separately, which breaks the authored aspect ratio for both-dimension images. Details inline.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding on the authored image sizing added to ChatMarkdown: the both-axes branch still lets max-h-[30rem] reshape the box for tall images.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One remaining copy-fidelity inconsistency in the image renderer; the authored-size and signed-URL/copy handling for workspace images now looks correct (the min(100%, 30rem, ratio) width cap keeps the placeholder and the loaded image on the same proportional bounds).
Posted via Macroscope — UI Consistency
178056e to
0f23290
Compare
There was a problem hiding this comment.
One finding on the new inline layout for workspace markdown images: the loading skeleton's geometry no longer matches the inline flow the same change introduces.
Posted via Macroscope — UI Consistency
923ad27 to
2727cdc
Compare
There was a problem hiding this comment.
Two layout findings on the new inline image handling in ChatMarkdown.tsx. Everything else (copy parity across all three image states, ratio-aware sizing for fully sized images, bounded placeholder width) looks consistent.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding on the authored image sizing helper in apps/web/src/components/ChatMarkdown.tsx. The both-axes case is now ratio-correct, but the single-axis case (the common README form) still loses the intrinsic ratio.
Posted via Macroscope — UI Consistency
ApprovabilityVerdict: Skipped Macroscope did not run approvability analysis for this PR. Macroscope could not determine whether this PR modifies its approvability configuration, so the PR was not approved automatically. A PR that may change the rules that govern approval is never approved automatically. |
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo Closing this PR after an automated pass over open pull requests. Workspace image rendering already shipped in #6433. |
|
Bilal is correct. At minimum, any markdown file not in the root directory is broken and will not display images that are linked to relatively. e.g. tests/README.md with an image relatively linked to by will show "image not available" |
The problem
cwd, but images in nested markdown files likedocs/README.mdwould fail.The solution
renderImagewiring for the file sidebar, resolving images based on the location of the documentThreadFeedarchitecture for chat markdown images is moved into a sharedmarkdownImages.tsxto allow file previews and chat to use the same code paths<p align="center">are centeredHardening
renders nothingicon.svg#logowould drop the#logopart and never re-append it, changing the client-side renderingUI Changes
Below is the relevant HTML snippet for the desktop screenshots, with the favicon at
public/icon-512.pngfor the root, andtests/public/icon-512.pngfor the nested examples:Web Before, nested readme
Fails to find images, is checking the wrong directory
Web Before, root dir readme
Ignores image height styling
Web After
Now renders at all and at the correct size.
Mobile UI changes
Here's the markdown for the below example:
iOS Before
iOS After
Scope Notes
<Image>cannot decode SVGs at all, so SVGs show image not found.The two above capability gaps are tracked in #7929
Review
Built with Fable 5 and GPT 5.6 Sol in T3 Code
Note
Render project images in
FileMarkdownPreviewand harden markdown imagesFileMarkdownPreviewrelative to the markdown document directory using signed asset URLs.rehypePreserveWindowsImageSrcandisWindowsDrivePathHrefto keep Windows drive path image sources through sanitization.authoredImageSizeStyleto apply authored width and height as inline CSS while respecting chat layout bounds.markdownImageSourceFragmentand copies the original markdown source on image selection..svgguard in markdownImages.tsx.Macroscope summarized f632e92.