Skip to content

OUT-3907 | Mobile Web App - Home App Banner Not Readjusting - #215

Merged
arpandhakal merged 1 commit into
mainfrom
OUT-3907
Jun 26, 2026
Merged

OUT-3907 | Mobile Web App - Home App Banner Not Readjusting#215
arpandhakal merged 1 commit into
mainfrom
OUT-3907

Conversation

@arpandhakal

Copy link
Copy Markdown
Collaborator

Summary

  • Banner container previously used responsive aspect ratios (3/1 mobile → 4/1 tablet → 5/1 desktop), causing wide banner images to be progressively cropped as the viewport narrowed
  • Unified to aspect-[5/1] across all breakpoints, matching the recommended banner spec (1200 × 250px = 4.8:1)
  • object-cover retained — portrait/non-standard images zoom to fill consistently at all sizes with no regression

Testing criteria

  • Upload a wide banner image (1200 × 250 or similar) and verify the full image is visible on mobile without left/right cropping
  • Verify desktop banner appearance is unchanged
  • Verify portrait/square images still fill the banner (zoom + crop, no letterboxing)
  • Verify drag-to-reposition still works

Impact analysis

  • Visual change only — no data model, API, or state changes
  • Mobile banner height reduced from ~130px to ~78px at 390px viewport width (trade-off accepted)
  • Desktop rendering unchanged (was already 5/1)

🤖 Generated with Claude Code

…ropping

Responsive breakpoint aspect ratios (3/1 mobile → 4/1 tablet → 5/1 desktop)
caused wide banner images to be progressively cropped as the viewport narrowed.
Unifying to 5/1 matches the recommended 1200×250 banner spec at all sizes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jun 26, 2026

Copy link
Copy Markdown

OUT-3907

@vercel

vercel Bot commented Jun 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
client-home-v3 Ready Ready Preview, Comment Jun 26, 2026 7:33am

Request Review

@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR unifies the banner's responsive aspect-ratio classes to a single aspect-[5/1] across all breakpoints, replacing the previous staggered 3/1 (mobile) → 4/1 (sm) → 5/1 (desktop) progression that caused wide banner images to be progressively cropped on smaller viewports.

  • The fix directly addresses the reported mobile cropping issue: wide banners (≈1200×250 recommended spec) now display without left/right clipping on any viewport.
  • As noted in the PR description, the trade-off is a shorter banner on mobile (~78px at 390px width vs. ~130px previously), and drag-to-reposition sensitivityY increases proportionally on mobile because 50 / rect.height grows as the container shrinks — but both effects are accepted trade-offs of the unified ratio.

Confidence Score: 5/5

Safe to merge — one-line CSS-only change with no logic, data, or API impact.

The change removes three Tailwind breakpoint modifiers and replaces them with one fixed class. No state, props, event handlers, or API calls are touched. The visual trade-off (shorter mobile banner) is well-documented in the PR and is the intended outcome.

No files require special attention.

Important Files Changed

Filename Overview
src/features/banner/components/Banner.tsx Unified banner aspect ratio to aspect-[5/1] across all breakpoints, removing the previous responsive 3/1 → 4/1 → 5/1 progression. Pure visual change; no logic or API differences.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Banner Component] --> B{Viewport width}
    B -->|Before PR| C["mobile: aspect-[3/1] ~130px tall\nsm 640px+: aspect-[4/1]\nlg 1024px+: aspect-[5/1]"]
    B -->|After PR| D["all breakpoints: aspect-[5/1]\nmobile: ~78px tall"]
    C --> E[object-cover fills container]
    D --> E
    E --> F{Image orientation}
    F -->|Wide 5:1 image| G[Full image visible, no crop]
    F -->|Portrait / square| H[Zoom + crop to fill]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Banner Component] --> B{Viewport width}
    B -->|Before PR| C["mobile: aspect-[3/1] ~130px tall\nsm 640px+: aspect-[4/1]\nlg 1024px+: aspect-[5/1]"]
    B -->|After PR| D["all breakpoints: aspect-[5/1]\nmobile: ~78px tall"]
    C --> E[object-cover fills container]
    D --> E
    E --> F{Image orientation}
    F -->|Wide 5:1 image| G[Full image visible, no crop]
    F -->|Portrait / square| H[Zoom + crop to fill]
Loading

Reviews (1): Last reviewed commit: "fix(OUT-3907): use uniform aspect-[5/1] ..." | Re-trigger Greptile

@arpandhakal arpandhakal self-assigned this Jun 26, 2026
@arpandhakal
arpandhakal requested a review from priosshrsth June 26, 2026 07:34
@arpandhakal
arpandhakal merged commit f046a4e into main Jun 26, 2026
8 checks passed
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