Skip to content

Comments

Improved bookmark thumbnail sizing in email cards#26502

Open
minimaluminium wants to merge 1 commit intomainfrom
codex/mobile-email-bookmark-DES-1275
Open

Improved bookmark thumbnail sizing in email cards#26502
minimaluminium wants to merge 1 commit intomainfrom
codex/mobile-email-bookmark-DES-1275

Conversation

@minimaluminium
Copy link
Member

ref https://linear.app/ghost/issue/DES-1275/mobile-newsletter-font-size-inconsistent-between-designs

Some bookmark cards exceeded Gmail mobile viewport width, which made Gmail auto-scale the whole email.
Thumbnail sizing now uses proportional width (33%) with a desktop cap (180px) to preserve desktop layout.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 19, 2026

Walkthrough

Updates the .kg-bookmark-thumbnail CSS rule in an email template styles file. Replaces the fixed min-width: 140px property with flex-based sizing using flex: 0 1 33%, width: 33%, and min-width: 33%, while retaining the max-width: 180px constraint. This modifies how the thumbnail element resizes within flex layouts by shifting from a fixed minimum width to percentage-based dimensions. The modification involves 3 added lines and 1 removed line.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: improving bookmark thumbnail sizing in email cards, which directly aligns with the CSS modifications that shift from fixed to flexible sizing.
Description check ✅ Passed The description is relevant to the changeset, providing context about the mobile viewport issue and explaining the proportional sizing solution (33% width with 180px cap) that matches the CSS changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/mobile-email-bookmark-DES-1275

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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs`:
- Around line 1051-1054: The .kg-bookmark-container CSS sets min-width: 33%
which overrides the intended max-width: 180px on desktop; remove the conflicting
declaration (delete min-width: 33%) in the .kg-bookmark-container rule so
flex-basis/width: 33% and max-width: 180px can work together to cap the element
as intended while preserving proportional shrinking in narrow/non-flex clients.

Comment on lines +1051 to 1054
flex: 0 1 33%;
width: 33%;
min-width: 33%;
max-width: 180px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

min-width: 33% defeats the max-width: 180px desktop cap

The .kg-bookmark-container fills the email content area (≈ 558–600 px on desktop). At those widths, 33% resolves to roughly 184–198 px, which is larger than max-width: 180px. The CSS specification mandates that min-width beats max-width when it resolves larger, so max-width: 180px is effectively a no-op and the PR's stated "desktop cap of 180px" never applies.

min-width: 33% is also redundant: flex-basis: 33% (via the flex shorthand) and width: 33% already establish the proportional size, and the existing max-width: 180px is sufficient to cap it on wide viewports once the conflicting constraint is removed.

🛠️ Proposed fix — remove the conflicting `min-width: 33%`
 .kg-bookmark-thumbnail {
     flex: 0 1 33%;
     width: 33%;
-    min-width: 33%;
     max-width: 180px;
     background-repeat: no-repeat;
     background-size: cover;
     background-position: center;
     border-radius: 0 2px 2px 0;
 }

With this change:

  • Desktop (≥ 600 px container): flex-basis: 33% ≈ 198 px, clamped by max-width: 180px180 px (cap works as intended).
  • Narrow viewports where the media query is stripped (e.g. Gmail mobile webview): element shrinks proportionally via flex-shrink: 1 — no fixed minimum enforced, preventing overflow and auto-scaling.
  • Non-flex clients (e.g. Outlook): width: 33% applies; max-width: 180px still caps it.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
flex: 0 1 33%;
width: 33%;
min-width: 33%;
max-width: 180px;
flex: 0 1 33%;
width: 33%;
max-width: 180px;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs`
around lines 1051 - 1054, The .kg-bookmark-container CSS sets min-width: 33%
which overrides the intended max-width: 180px on desktop; remove the conflicting
declaration (delete min-width: 33%) in the .kg-bookmark-container rule so
flex-basis/width: 33% and max-width: 180px can work together to cap the element
as intended while preserving proportional shrinking in narrow/non-flex clients.

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