Skip to content

Constrain blog images on desktop to improve reading rhythm#2651

Closed
google-labs-jules[bot] wants to merge 4 commits into
mainfrom
feat/constrain-blog-images-desktop-issue-247-16338483412586849841
Closed

Constrain blog images on desktop to improve reading rhythm#2651
google-labs-jules[bot] wants to merge 4 commits into
mainfrom
feat/constrain-blog-images-desktop-issue-247-16338483412586849841

Conversation

@google-labs-jules

@google-labs-jules google-labs-jules Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Problem Statement

The current implementation does not constrain the sizing of inline markdown images and Mermaid diagrams within the blog post body on desktop viewports, leading to potential layout issues.

Goal

To constrain the sizing of inline markdown images and Mermaid diagrams in the blog post body on desktop viewports while ensuring they maintain their aspect ratio and adhere to design system guidelines.

Non-Goals

None.

Proposed Approach

  • Update src/components/ui/MarkdownRenderer.tsx to use responsive maxWidth={{ base: "full", md: "2xl" }} and maxHeight for standard images and Mermaid diagrams.
  • Ensure images maintain their aspect ratio using className="object-contain".
  • Adhere to design system guidelines by using layout primitives and responsive props instead of raw Tailwind classes where possible.

Alternatives Considered

None.

Architectural Impact

None.

Scope

This change will impact the Markdown renderer used for displaying blog posts and research studies, specifically focusing on the layout of images and diagrams in desktop viewports.

UNDERSTAND THE ISSUE

The issue arises from the lack of constraints on the sizing of inline markdown images and Mermaid diagrams, potentially leading to inappropriate scaling and layout discrepancies in the blog post presentation.

DETERMINE APPROACH

The approach involves modifying the Markdown renderer component to include responsive design constraints for images and diagrams, ensuring compliance with design standards and maintaining visual integrity across different devices.

SPECIFY SCOPE

The scope is limited to the src/components/ui/MarkdownRenderer.tsx file and the styling related to inline markdown images and Mermaid diagrams within the blog post body. Verification will include checking for desktop viewports and ensuring that mobile viewports remain at full width.

DEFINITION OF DONE

  • Changes are implemented in src/components/ui/MarkdownRenderer.tsx.
  • Images and diagrams are constrained with responsive maxWidth and maxHeight.
  • Aspect ratios for images are maintained using className="object-contain".
  • Adherence to design system guidelines is confirmed.
  • Verification through Playwright screenshots of blog posts and research studies on desktop viewports is completed.
  • pnpm run audit and pnpm run ci:local are executed without introducing anti-patterns or regressions.
  • Mobile viewports are verified to remain full-width.
  • Issue Oversized images in blog posts break desktop reading rhythm #2532 is fixed.

Constrain inline markdown images and Mermaid diagrams in blog posts
to a maximum width of 2xl on desktop viewports. This prevents
oversized images from disrupting the reading rhythm.

- Added responsive maxWidth and maxHeight to standard img renderer
- Added responsive maxWidth and maxHeight to Mermaid diagram renderer
- Used design system primitives (Box) and responsive props
- Verified with Playwright screenshots and smoke tests
@google-labs-jules

Copy link
Copy Markdown
Contributor Author

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🚀 Deployment Details (Last updated: Jun 19, 2026, 3:05 PM PST)

🚀 Pushed to gh-pages; publish in progress

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🐙 GitHub Models Code Review

Powered by GitHub Models

Reviewing: PR #2651

Model: gpt-4.1

Code Review Feedback

Review of src/components/ui/MarkdownRenderer.tsx

Confirming Previous Findings

finding-1: Responsive objects passed to native img element

  • Previous Issue: Responsive objects (e.g., maxWidth={{ base: "full", md: "2xl" }}) were passed directly to a Box rendered as img, which is invalid because native img elements do not accept these props.
  • Resolution: The diff now wraps the native <img> element inside a Chakra <Box>, and applies responsive maxWidth and maxHeight to the Box, not the img. The img receives only valid HTML attributes and a Tailwind class for aspect ratio. This resolves the issue.

finding-2: Passing height="auto" to native img

  • Previous Issue: Passing height="auto" to a native img element is invalid; img expects a numeric value or string representing a number.
  • Resolution: The diff removes height="auto" from the img element and instead uses className="h-auto" (Tailwind), which is valid and preserves aspect ratio. This resolves the issue.

New Issues

I have carefully reviewed the diff for new, high-severity issues.

1. Chakra Box props: maxWidth and maxHeight values

  • Analysis: The Box receives maxWidth={{ base: "full", md: "2xl" }} and maxHeight={{ md: 128 }}. Chakra UI supports responsive objects for these props, and "full" and "2xl" are valid values for Chakra's sizing scale. No type or runtime contradiction is present.
  • Verdict: No blocking issue.

2. img element: className and props

  • Analysis: The img element receives only valid HTML attributes and a Tailwind class. No invalid props are passed.
  • Verdict: No blocking issue.

3. Mermaid diagram rendering

  • Analysis: The Box for Mermaid diagrams uses responsive maxWidth and maxHeight, and the img inside uses valid HTML attributes and Tailwind classes. No contradiction.
  • Verdict: No blocking issue.

4. Types

  • Analysis: No missing types or type errors are introduced in the diff. All props passed are valid for their respective components.
  • Verdict: No blocking issue.

5. Performance

  • Analysis: No performance regressions are introduced. The use of lazy loading for images is preserved.
  • Verdict: No blocking issue.

6. Security

  • Analysis: No new untrusted input paths are introduced. The src for images is normalized, and Mermaid diagrams are encoded safely.
  • Verdict: No blocking issue.

Final Verdict

All previously raised blocking issues are resolved. No new blocking issues are introduced by this diff.


Structured JSON summary:

{ "findings": [ { "id": "finding-1", "file": "src/components/ui/MarkdownRenderer.tsx", "line": 151, "snippet": "maxWidth={{ base: \"full\", md: \"2xl\" }}", "issue": "Passing responsive objects as values to maxWidth and maxHeight on a Box rendered as img is invalid; native img elements do not accept these props and will ignore them.", "status": "resolved", "fixSummary": "Responsive constraints are now applied to a parent Box, and the native img element receives only valid HTML attributes." }, { "id": "finding-2", "file": "src/components/ui/MarkdownRenderer.tsx", "line": 153, "snippet": "height=\"auto\"", "issue": "Passing height=\"auto\" to a native img element is invalid; img expects a numeric value or string representing a number.", "status": "resolved", "fixSummary": "height=\"auto\" is removed from the img element; aspect ratio is preserved using Tailwind className." } ] }

[VERDICT: PASS]


Generated by github-models-code-review

- Wrap markdown images and Mermaid diagrams in a Box primitive to enforce desktop sizing.
- Limit image maxWidth to 2xl and maxHeight to 128 on desktop viewports.
- Maintain full width and responsive scaling on mobile.
- Ensure design system compliance by avoiding as="img" with responsive props.
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🐙 GitHub Models Visual Review

Powered by GitHub Models Vision + Blast-Radius Analyzer

Summary: 🔴 2 high · 🟡 0 medium · 🟢 0 low
Reviewing: PR #2651

🔴 /blog/2026-06-14-the-story-behind-the-merch-page (mobile)

Pixel diff: 26.93%

  • ❌ BUG/REGRESSION: No DOM text diff is available, yet the pixel diff shows a high severity (26.93%) change across nearly the entire page. This is not expected for a typical blog post update and suggests unintended visual regressions.
  • The DIFF highlights significant changes in:
    • All main content blocks (headers, paragraphs, images, cards, tags, related articles, footer).
    • Layout and spacing, especially around images and text blocks.
    • Possible contrast or color changes, as indicated by the intensity of the pixel diff.
  • There are visible layout shifts and spacing inconsistencies, particularly in the content sections and cards.
  • The absence of a DOM text diff means these changes are likely NOT intentional content updates, but rather visual or structural regressions.

Recommendations for Improvement:

  • Immediately revert or investigate the PR for unintended global style changes (e.g., CSS resets, theme overrides, or layout bugs).
  • Add automated visual regression tests to catch high-impact changes before merging.
  • If any changes are intentional, ensure they are documented and accompanied by a DOM diff for clarity.
  • Consider increasing padding between content blocks and cards for improved readability and visual separation.
  • Review color contrast and ensure accessibility standards are maintained.

{ "findings": [ { "id": "finding-1", "route": "/blog/2026-06-14-the-story-behind-the-merch-page (mobile)", "issue": "Major visual regression affecting layout, spacing, and color across the entire page. No DOM text diff provided.", "status": "unresolved", "fixSummary": "PR should be reverted or investigated for unintended global style changes. Add visual regression tests and ensure DOM diffs accompany intentional changes." } ] }

🔴 /blog/2026-06-14-the-story-behind-the-merch-page

Pixel diff: 19.48%

  • ✅ Issue "finding-1" is now resolved: The previous round required a DOM Text Diff for traceability and a review of layout for spacing and accessibility. This round provides a DOM Text Diff (none available, confirming no text changes), and the visual diff shows extensive layout changes but no evidence of broken spacing, contrast issues, or clipping. The layout appears consistent and accessible, with no obvious regressions.
  • No new unrelated issues observed. All visual changes appear to be intentional layout adjustments (e.g., spacing, image placement, card alignment).
  • Visual quality is generally good, but some actionable improvements are recommended:

Recommendations for Improvement:

  • Consider increasing vertical spacing between image blocks and text for improved readability (suggest +8px margin-bottom).
  • Evaluate card padding and alignment in the sidebar for consistency; some cards appear slightly misaligned compared to main content.

{ "findings": [ { "id": "finding-1", "route": "/blog/2026-06-14-the-story-behind-the-merch-page", "issue": "Extensive visual changes with no DOM Text Diff provided; cannot verify intent or correctness. Require DOM Text Diff for traceability; review layout for spacing and accessibility before merge.", "status": "resolved", "fixSummary": "DOM Text Diff is now provided (none available, confirming no text changes). Layout reviewed for spacing and accessibility; no regressions found." } ] }

Generated by impact-github-models-review — Blast-Radius Analyzer

- Wrap images and mermaid diagrams in a Box primitive with responsive constraints
- Set maxWidth to '2xl' and maxHeight to '96' (approx. 384px) on desktop viewports
- Use max-w-full and max-h-full on inner img tags to prevent stretching small assets
- Maintain full-width scaling on mobile viewports
- Ensure image corner rounding and shadows align with the design system

This change prevents oversized images from breaking the desktop reading rhythm while maintaining responsive behavior on smaller screens.
@arii

arii commented Jun 19, 2026

Copy link
Copy Markdown
Owner

🐙 GitHub Models Code Review

Powered by GitHub Models

Reviewing: PR #2651

Model: gpt-4.1

Code Review Feedback

Review of src/components/ui/MarkdownRenderer.tsx

Confirming Previous Findings

finding-1: Responsive objects passed to native img element

  • Previous Issue: Responsive objects (e.g., maxWidth={{ base: "full", md: "2xl" }}) were passed directly to a Box rendered as img, which is invalid because native img elements do not accept these props.
  • Resolution: The diff now wraps the native <img> element inside a Chakra <Box>, and applies responsive maxWidth and maxHeight to the Box, not the img. The img receives only valid HTML attributes and a Tailwind class for aspect ratio. This resolves the issue.

finding-2: Passing height="auto" to native img

  • Previous Issue: Passing height="auto" to a native img element is invalid; img expects a numeric value or string representing a number.
  • Resolution: The diff removes height="auto" from the img element and instead uses className="h-auto" (Tailwind), which is valid and preserves aspect ratio. This resolves the issue.

New Issues

I have carefully reviewed the diff for new, high-severity issues.

1. Chakra Box props: maxWidth and maxHeight values

  • Analysis: The Box receives maxWidth={{ base: "full", md: "2xl" }} and maxHeight={{ md: 128 }}. Chakra UI supports responsive objects for these props, and "full" and "2xl" are valid values for Chakra's sizing scale. No type or runtime contradiction is present.
  • Verdict: No blocking issue.

2. img element: className and props

  • Analysis: The img element receives only valid HTML attributes and a Tailwind class. No invalid props are passed.
  • Verdict: No blocking issue.

3. Mermaid diagram rendering

  • Analysis: The Box for Mermaid diagrams uses responsive maxWidth and maxHeight, and the img inside uses valid HTML attributes and Tailwind classes. No contradiction.
  • Verdict: No blocking issue.

4. Types

  • Analysis: No missing types or type errors are introduced in the diff. All props passed are valid for their respective components.
  • Verdict: No blocking issue.

5. Performance

  • Analysis: No performance regressions are introduced. The use of lazy loading for images is preserved.
  • Verdict: No blocking issue.

6. Security

  • Analysis: No new untrusted input paths are introduced. The src for images is normalized, and Mermaid diagrams are encoded safely.
  • Verdict: No blocking issue.

Final Verdict

All previously raised blocking issues are resolved. No new blocking issues are introduced by this diff.


Structured JSON summary:

{ "findings": [ { "id": "finding-1", "file": "src/components/ui/MarkdownRenderer.tsx", "line": 151, "snippet": "maxWidth={{ base: \"full\", md: \"2xl\" }}", "issue": "Passing responsive objects as values to maxWidth and maxHeight on a Box rendered as img is invalid; native img elements do not accept these props and will ignore them.", "status": "resolved", "fixSummary": "Responsive constraints are now applied to a parent Box, and the native img element receives only valid HTML attributes." }, { "id": "finding-2", "file": "src/components/ui/MarkdownRenderer.tsx", "line": 153, "snippet": "height=\"auto\"", "issue": "Passing height=\"auto\" to a native img element is invalid; img expects a numeric value or string representing a number.", "status": "resolved", "fixSummary": "height=\"auto\" is removed from the img element; aspect ratio is preserved using Tailwind className." } ] }

[VERDICT: PASS]


Generated by github-models-code-review

@arii

arii commented Jun 19, 2026

Copy link
Copy Markdown
Owner

🤖 AI Technical Audit

ANTI-AI-SLOP

The refactoring of the image rendering logic into a wrapper-child pattern introduces unnecessary DOM bloat. The use of an additional Box container solely to manage layout constraints that could be applied directly to the image element violates the principle of minimal DOM representation. Furthermore, ensure that the object-contain property is not causing unexpected whitespace gaps due to container sizing mismatch.

FINAL RECOMMENDATION

Approved with Minor Changes

Review automatically published via RepoAuditor.

@arii arii added enhancement New feature or request refactor ui labels Jun 19, 2026
- Refactored image and Mermaid diagram renderers to use 'Box as="img"'
- Applied responsive maxWidth (2xl) and maxHeight (80) directly to image elements
- Removed unnecessary wrapper Boxes to satisfy technical audit regarding DOM bloat
- Added required classes to Tailwind safelist for production build
- Maintained aspect ratio and centering across all viewports

This update addresses PR feedback while preserving the fix for oversized desktop images.
@arii arii closed this Jun 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request refactor ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant