Skip to content

Fix IterableEmbeddedView image height to match campaign preview#1016

Closed
franco-zalamena-iterable wants to merge 1 commit intomasterfrom
fix/997-embedded-view-image-height
Closed

Fix IterableEmbeddedView image height to match campaign preview#1016
franco-zalamena-iterable wants to merge 1 commit intomasterfrom
fix/997-embedded-view-image-height

Conversation

@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor

@franco-zalamena-iterable franco-zalamena-iterable commented Apr 7, 2026

Summary

  • Fix embedded message image height to use wrap_content instead of 0dp in card_view.xml (both layout/ and layout-v21/)
  • Add adjustViewBounds="true" and fitCenter scaleType for correct image display
  • Add layout_constraintVertical_bias="0.0" to keep image pinned to top

Test plan

  • Verify embedded view displays images matching campaign preview
  • Test with various image aspect ratios

🤖 Generated with Claude Code

Change embedded_message_image height from 0dp to wrap_content, add
adjustViewBounds="true", use fitCenter scaleType, and set vertical
bias to 0.0 so images display at their intrinsic aspect ratio.

Fixes #997

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor Author

This fix is actually ok. It's the bare minimum but fixes the issue

@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor Author

PR Analysis

Problem: The IterableEmbeddedView card layout's ShapeableImageView has layout_height="0dp" which, combined with how ConstraintLayout resolves dimensions in a chain between the image, text container (fixed 120dp), and buttons container, causes the image to collapse to zero height when it cannot fill remaining space — making it invisible.

Ideal fix plan:

  • Change layout_height from 0dp to wrap_content so the image can size itself based on its intrinsic dimensions
  • Add adjustViewBounds="true" so the ImageView respects aspect ratio when width is constrained
  • Change scaleType from centerCrop to fitCenter (or centerInside as the issue reporter suggested) to scale images proportionally within the available width rather than cropping
  • Apply to both layout/card_view.xml and layout-v21/card_view.xml
  • Consider whether the root ConstraintLayout height of match_parent should be wrap_content instead — the current match_parent means the card always fills its parent's full height, which may cause the image to stretch or leave excess space depending on the parent container. This is worth verifying.
  • Verify the text container's fixed 120dp height (@dimen/card_text_container_height) still works well with a now-variable-height image

What the PR did:

  • Changed layout_height from 0dp to wrap_content in both card_view.xml files
  • Added adjustViewBounds="true" in both files
  • Changed scaleType from centerCrop to fitCenter in both files
  • Added layout_constraintVertical_bias="0.0" to pin the image to the top

Assessment:

  • Root cause identified: yes — the 0dp height on the image is the core problem
  • Fix correctness: partially correct — the three main changes (wrap_content, adjustViewBounds, fitCenter) are the right approach for making the image visible and properly sized. However, layout_constraintVertical_bias="0.0" has no effect here since the image's vertical constraints form a chain (top to parent, bottom to text container) and the view is no longer 0dp height — bias only affects views with 0dp dimensions in ConstraintLayout. It is harmless but dead code.
  • Missed:
    • The root ConstraintLayout has layout_height="match_parent" in both layout files. With the image now being wrap_content, the overall card still fills its parent completely. Depending on how IterableEmbeddedView is hosted, this may cause the card to be taller than its content (image + 120dp text + buttons). It may be worth changing to wrap_content or confirming the parent handles this correctly.
    • No validation of behavior when no image URL is set (image view presumably has no drawable — does wrap_content collapse to 0 correctly in that case, or does it need GONE visibility logic?).
    • The issue reporter also mentioned wanting configurable scaleType. While that's a feature request beyond a bug fix, it is worth noting.
  • Wrong assessment: none
  • Tests: UI layout fixes like this are hard to unit test. Ideally verified with visual/instrumentation tests across different image aspect ratios and the no-image case. The PR's test plan mentions manual verification, which is reasonable. Not strictly needed but manual QA is essential.

@franco-zalamena-iterable franco-zalamena-iterable deleted the fix/997-embedded-view-image-height branch April 8, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant