Skip to content

fix: removed height that causes vtex crop image#1591

Open
aline-pereira wants to merge 1 commit into
mainfrom
fix/vtex-image-crop
Open

fix: removed height that causes vtex crop image#1591
aline-pereira wants to merge 1 commit into
mainfrom
fix/vtex-image-crop

Conversation

@aline-pereira

@aline-pereira aline-pereira commented May 19, 2026

Copy link
Copy Markdown
Contributor

What is this Contribution About?

Please provide a brief description of the changes or enhancements you are proposing in this pull request.

Root cause - optimizeVTEX always forces the consumer's aspect ratio into the URL path ({id}-{width}-{height}), which triggers letterboxing whenever the requested ratio differs from the image's original ratio.

Solution - VTEX supports proportional resizing when one of the dimensions is 0. Replacing {id}-{width}-{height} with {id}-{width}-0 makes VTEX preserve the original aspect ratio, and the consumer's CSS object-cover then crops correctly within the layout.

image

Summary by cubic

Prevented VTEX images from being letterboxed by removing the forced height in the optimizer. URLs now use height=0 so VTEX keeps the original aspect ratio and object-cover crops correctly.

  • Bug Fixes
    • Updated optimizeVTEX in website/components/Image.tsx to build ${id}-${width}-0 instead of ${id}-${width}-{height}.
    • Preserves original aspect ratio and eliminates unintended cropping/letterboxing.

Written for commit 39689b3. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved image optimization to ensure better loading performance and consistent image display across the platform.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The optimizeVTEX function in Image.tsx was updated to change how VTEX image URLs are rewritten. The function now extracts the true image ID from the raw ID using dash-based splitting and constructs the new path segment with a hardcoded height value of 0 instead of the original height.

Changes

VTEX Image URL Optimization

Layer / File(s) Summary
VTEX URL path construction
website/components/Image.tsx
The optimizeVTEX function extracts trueId from rawId by splitting on - and generates a new pathname segment using ${trueId}-${width}-0 instead of the original ${trueId}-${width}-${height} format.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • deco-cx/apps#1530: Both PRs modify VTEX image URL optimization logic in Image.tsx; this PR refines the URL path construction while #1530 addresses the broader image optimization flow.

Suggested reviewers

  • vibegui
  • vibe-dex
  • mcandeia

Poem

🐰 A dash becomes the path divine,
True IDs split along the line,
With width and zero, images shine,
VTEX URLs now align! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: removing height from VTEX image URLs to fix cropping issues.
Description check ✅ Passed The description includes a clear root cause analysis, solution explanation, and implementation details, but is missing the Issue Link and Loom Video sections from the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/vtex-image-crop

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.

@github-actions

Copy link
Copy Markdown
Contributor

Tagging Options

Should a new tag be published when this PR is merged?

  • 👍 for Patch 0.151.2 update
  • 🎉 for Minor 0.152.0 update
  • 🚀 for Major 1.0.0 update

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@website/components/Image.tsx`:
- Around line 134-139: Reformat the array join expression that builds the image
path so it matches deno fmt style: ensure each array element (slash, arquivos,
ids, `${trueId}-${width}-0`, ...rest) is properly comma-separated and the
closing ] and .join("/") are on the correct lines, preserving the same values
and variables (trueId, width, arquivos, ids, rest) but adjusting whitespace/line
breaks to satisfy the formatter.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 62324289-c8a4-4e60-9957-bd7fca5cba1f

📥 Commits

Reviewing files that changed from the base of the PR and between bb78556 and 39689b3.

📒 Files selected for processing (1)
  • website/components/Image.tsx

Comment on lines +134 to +139
slash,
arquivos,
ids,
`${trueId}-${width}-0`,
...rest]
.join("/");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix formatting to unblock CI (deno fmt --check).

Line 134-Line 139 is currently failing formatter checks in pipeline; please reformat this block.

Proposed formatting fix
-  src.pathname = [
-    slash, 
-    arquivos, 
-    ids, 
-    `${trueId}-${width}-0`,
-    ...rest]
-    .join("/");
+  src.pathname = [
+    slash,
+    arquivos,
+    ids,
+    `${trueId}-${width}-0`,
+    ...rest,
+  ].join("/");
🧰 Tools
🪛 GitHub Actions: ci / 0_Bundle & Check Apps (ubuntu-latest).txt

[error] 134-139: deno fmt --check failed. Found 1 not formatted file in 2108 files. Formatting issue reported at this section.

🪛 GitHub Actions: ci / Bundle & Check Apps (ubuntu-latest)

[error] 134-139: deno fmt --check failed: Found 1 not formatted file in 2108 files.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@website/components/Image.tsx` around lines 134 - 139, Reformat the array join
expression that builds the image path so it matches deno fmt style: ensure each
array element (slash, arquivos, ids, `${trueId}-${width}-0`, ...rest) is
properly comma-separated and the closing ] and .join("/") are on the correct
lines, preserving the same values and variables (trueId, width, arquivos, ids,
rest) but adjusting whitespace/line breaks to satisfy the formatter.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 1 file

Re-trigger cubic

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.

1 participant