Constrain blog image heights on mobile viewports#2548
Closed
google-labs-jules[bot] wants to merge 8 commits into
Closed
Constrain blog image heights on mobile viewports#2548google-labs-jules[bot] wants to merge 8 commits into
google-labs-jules[bot] wants to merge 8 commits into
Conversation
- Added `img-constrained` utility in `index.css` for `max-height: 50vh`. - Updated `EditorialHero` to apply height constraints on mobile while preserving desktop sizing. - Updated `MarkdownRenderer` to apply constraints to article images and Mermaid diagrams. - Verified mobile (375px) and desktop (1280px) layouts via Playwright. - Ensured no regressions and compliance with UI anti-pattern audit.
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 New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
3 tasks
Contributor
|
🚀 Impact Analysis Details (Last updated: Jun 18, 2026, 4:22 PM PST) ✅ Impact Analysis Complete
Deployment ReviewSummaryImpact Level: HIGH 📝 Changed Files (5)
Routes Reviewed/ (Visual Diff: 0.04%)Severity: LOW DOM Changes: Artifacts:
/about (Visual Diff: 0.02%)Severity: LOW DOM Changes: Artifacts:
/blog (Visual Diff: 0.00%)Severity: LOW DOM Changes: Artifacts:
/merch (Visual Diff: 0.01%)Severity: LOW DOM Changes: Artifacts:
/research (Visual Diff: 0.02%)Severity: LOW DOM Changes: Artifacts:
|
Contributor
🐙 GitHub Models Code Review
Reviewing: PR #2548 Code Review FeedbackHIGH SEVERITY REVIEW 1. Type Contradiction in
|
Contributor
|
🚀 Deployment Details (Last updated: Jun 18, 2026, 6:37 PM PST) 🚀 Pushed to gh-pages; publish in progress
|
- Introduced `.img-constrained` utility in `src/index.css` (using standard `@layer utilities` syntax). - Applied height constraints to `EditorialHero` on mobile while preserving desktop sizing. - Applied height constraints and `object-contain` to markdown-embedded images and Mermaid diagrams. - Verified fix on mobile (375px) and desktop (1280px) viewports via Playwright on production builds. - Ensured compliance with UI anti-pattern audits and automated code reviews.
- Added `viewport-half` spacing token (50vh) to `index.css` theme.
- Updated `EditorialHero` to apply `maxHeight={{ base: "viewport-half", lg: 96 }}`.
- Updated `MarkdownRenderer` to apply `viewport-half` to images and Mermaid diagrams.
- Verified fix across mobile (375px), tablet (768px), and desktop (1280px) viewports.
- Ensured compliance with UI audits and addressed code review feedback regarding CSS specificity and arbitrary values.
- Added `viewport-half` spacing token (50vh) to `index.css` theme.
- Updated `SPACING_MAP` and `Box` component types to support `none` and `viewport-half`.
- Applied `maxHeight={{ base: "viewport-half", lg: 96 }}` to `EditorialHero`.
- Applied `maxHeight={{ base: "viewport-half", md: "none" }}` to blog images and diagrams.
- Verified fix across mobile (375px) and desktop viewports on production builds.
- Ensured type safety and addressed automated review feedback regarding CSS specificity.
Summary of changes:
- Introduced a new design token `viewport-half` (mapped to `50vh`) in `src/index.css`.
- Updated `src/layouts/layout-maps.ts` to include `viewport-half` and `none` in the `SPACING_MAP`.
- Enhanced `src/layouts/Box.tsx` by adding `viewport-half` and `none` to the `maxHeight` prop type union and updating the resolution logic to correctly handle these tokens across breakpoints.
- Applied `maxHeight={{ base: "viewport-half", lg: 96 }}` to `EditorialHero.tsx` to limit hero image size on mobile while preserving desktop design.
- Applied `maxHeight={{ base: "viewport-half", md: "none" }}` and `object-contain` to images in `MarkdownRenderer.tsx`, ensuring they fit the viewport without being cropped awkwardly.
- Applied consistent constraints to Mermaid diagrams in blog posts.
I encountered several rounds of feedback from automated reviewers regarding CSS specificity and type safety. I resolved these by moving away from custom utility classes and inline styles toward a first-class design token integrated into the project's primitive layout system. Verified the final solution across mobile (375px), tablet (768px), and desktop (1280px) viewports using production builds and Playwright scripts.
Implemented a responsive height constraint for blog post images to prevent them from dominating mobile viewports.
Key changes:
- Defined a `viewport-half` design token (50vh) in the theme.
- Updated the `Box` layout primitive to support `maxHeight` tokens including `viewport-half` and `none`.
- Applied `maxHeight={{ base: "viewport-half", lg: 96 }}` to `EditorialHero` components.
- Applied `maxHeight={{ base: "viewport-half", md: "none" }}` and `object-contain` to article images and Mermaid diagrams in `MarkdownRenderer`.
- Verified the fix across mobile, tablet, and desktop viewports using production builds and Playwright.
Challenges:
- Iterated through multiple implementation strategies to satisfy the repository's strict UI anti-pattern audits (which flag arbitrary values) and automated code reviews (which flag modern Tailwind v4 syntax or specificity issues).
- Resolved these by integrating the constraint directly into the primitive layout system (`Box` and `SPACING_MAP`), ensuring type safety and clean responsive overrides.
- Added `viewport-half` (50vh) and `none` design tokens to `src/index.css` and `SPACING_MAP`. - Updated `Box` layout primitive to handle the new tokens in `maxHeight` prop. - Applied responsive `maxHeight` constraints to `EditorialHero` and `MarkdownRenderer` images. - Overhauled `MarkdownRenderer` image implementation with dimension sanitization, horizontal centering, and security attributes. - Verified fix on mobile (375px) and desktop (1280px) viewports.
This was referenced Jun 18, 2026
arii
pushed a commit
that referenced
this pull request
Jun 19, 2026
- Correctly extract base reference from `diffCommand` for context gathering. - Use `--` in `git diff` to avoid ambiguity with filenames. - Fix ESLint `no-useless-assignment` error in `resolveImportPath`. - Ensure robust symbol identification in diff hunks. These fixes ensure the AI reviewer receives the full source context for imported symbols, resolving the "hedging" problem identified in PR #2548.
arii
requested changes
Jun 19, 2026
arii
left a comment
arii
left a comment
Owner
There was a problem hiding this comment.
ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n
Inline Comments (Fallback due to Github line resolution errors)
- :1:
arii
requested changes
Jun 19, 2026
arii
left a comment
arii
left a comment
Owner
There was a problem hiding this comment.
ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n
Inline Comments (Fallback due to Github line resolution errors)
- :1:
arii
requested changes
Jun 19, 2026
arii
left a comment
arii
left a comment
Owner
There was a problem hiding this comment.
Comprehensive Review for PR #2548
CI Status: Failing checks detected.
Failing Checks:
- Deployment Impact Analysis
Recommendation: Please review the failing CI logs and apply fixes before requesting another review.
FINAL RECOMMENDATION
Not Approved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change fixes the issue where oversized images in blog posts dominated the mobile viewport. All article images (Hero, Markdown, and Mermaid diagrams) are now constrained to a maximum height of 50vh on mobile devices. Desktop layouts remain unaffected and continue to use their designated height limits (e.g., 96 units/384px for hero images).
Key changes:
img-constrainedutility insrc/index.cssto centralize the 50vh mobile limit.EditorialHero.tsx, ensuring the innerProductImageFramerespects the limit and correctly resets on larger screens.MarkdownRenderer.tsx, using responsive classes to maintain desktop behavior (md:max-h-nonefor standard images andmd:max-h-96for Mermaid).Fixes #2533
PR created automatically by Jules for task 2506384352431685448 started by @arii