Skip to content

[MWPW-207415] - Fix ace1209 rich-content media padding/margin overrides - #6744

Open
DKos95 wants to merge 9 commits into
stagefrom
rich-content-media-reconcile
Open

DKos95 wants to merge 9 commits into
stagefrom
rich-content-media-reconcile

Conversation

@DKos95

@DKos95 DKos95 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Title

Fix ace1209 rich-content media padding/margin overrides

Summary

  • Gate the media block's edge-to-edge bleed styling (border/radius removal, full-bleed image sizing) behind a .full-width modifier class instead of applying to all .rich-content.media
  • Fix an invalid unscoped --grid-margin-width custom property declaration inside a bare @media block (no selector, so it was silently dropped by the parser) — rescoped it to .rich-content.media.full-width at the correct >= 768px breakpoint
  • Extend full-width media resets (border/radius/padding) to picture, not just .video-container, so full-width images get the same treatment as full-width video
  • Drop the specificity of the block's default padding-block rule to zero via :where() so authored GWP spacing combo classes (e.g. spacing-4xl) can override the default padding, instead of losing to the block's own hardcoded rule

Test plan

  • Verify .rich-content.media renders with default padding at mobile/tablet/desktop breakpoints when no spacing class is authored
  • Add a spacing-4xl (or similar) class to a .rich-content.media block and confirm the authored padding now overrides the default
  • Verify .rich-content.media.full-width bleeds edge-to-edge with the correct --grid-margin-width reset at >= 768px
  • Verify non-full-width media blocks are unaffected by the bleed styling
  • Verify full-width picture images get the same border/radius/aspect-ratio treatment as full-width video

Resolves: MWPW-207415

Mobile video Test URLs:

Mobile image Test URLs:

Dusan Kosanovic added 6 commits September 4, 2026 13:19
The mobile edge-to-edge margin override (--grid-margin-width: 0) was applied
unconditionally to every .rich-content.media block, so any author using the
media variant got full-bleed images on mobile whether they wanted it or not.
Move it into an explicit .full-width class so it's an opt-in variant.
…ent media query

Move .rich-content.media.full-width margin overrides into top-level
media queries; the previous @media (width >= 768px) block declared
--grid-margin-width with no selector, so it was dropped by the parser.
Extend the border/radius/padding reset to picture (not just
video-container) and give full-width images their own aspect-ratio
and radius, matching video framing.
…ding

Drop the .rich-content.media padding-block rule's specificity to zero
via :where() so authored spacing-* classes (e.g. spacing-4xl) win the
cascade instead of being blocked by the block's own default rule.
…border on full-width

Wrap media-cell images in a bordered picture frame at a fixed max-width
instead of stretching to 100%, and remove the leftover border on
full-width video-container images now that they render edge-to-edge.
The parent picture/video-container already constrains width, so the
100% max-width on glass-border img/video was a no-op.
@DKos95
DKos95 requested a review from a team September 16, 2026 12:01
@aem-code-sync

aem-code-sync Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-sync branch
Commits

@DKos95
DKos95 deployed to milo_pr_merge September 16, 2026 12:01 — with GitHub Actions Active
@DKos95 DKos95 changed the title [MWPW-206452] - Fix ace1209 rich-content media padding/margin overrides [MWPW-207415] - Fix ace1209 rich-content media padding/margin overrides Sep 16, 2026
@SilviuLCF SilviuLCF self-assigned this Sep 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.

@zagi25

zagi25 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor
Screenshot 2026-09-17 at 10 47 26

Noticed this at width < 1280px is this expected ?

@overmyheadandbody overmyheadandbody 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.

How is the scaling mechanism supposed to work for the full width scenario? Right now it seems to exceed the viewport bounds and scales down to where it's fully visible quite late. Should the scaling effect be removed/tweaked in this case?

Image

@DKos95

DKos95 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

How is the scaling mechanism supposed to work for the full width scenario? Right now it seems to exceed the viewport bounds and scales down to where it's fully visible quite late. Should the scaling effect be removed/tweaked in this case?

Looking at design again I am wondering if intentions are clear enough on what happens between some resolutions as specs aren't marked from x-y resolution, left a comment. Will re-visit this once everything is more clear, thanks for the review.

@DKos95

DKos95 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Noticed this at width < 1280px is this expected ?

Will re visit once specs are cleared up, thanks for the review.

@rgclayton

Copy link
Copy Markdown
Contributor

This one kinda feels like it might create a whack-mole game with the different aspect-ratios,object-fit: cover and the animation-timeline/animation-range at the narrow/smaller end of tablet, just before it goes to mobile. Slight cut off. 🤷‍♂️. Still looks good.

@DKos95

DKos95 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

How is the scaling mechanism supposed to work for the full width scenario? Right now it seems to exceed the viewport bounds and scales down to where it's fully visible quite late. Should the scaling effect be removed/tweaked in this case?

This is fixed by narrowing the full-width to only mobile after latest design clear up.

@DKos95

DKos95 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Noticed this at width < 1280px is this expected ?

Fixed, thank you!

@DKos95
DKos95 requested review from a team and overmyheadandbody September 21, 2026 10:47
@zagi25

zagi25 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

If glass-border is removed, img will shrink, but there is still a glass-border, that doesn't correspond to the asset size

Screenshot 2026-09-21 at 15 06 47

@DKos95

DKos95 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

If glass-border is removed, img will shrink, but there is still a glass-border, that doesn't correspond to the asset size

Screenshot 2026-09-21 at 15 06 47

Updated, thanks!

This branch was successfully deployed

2 active (1 outdated) deployments
rich-content-media-reconcile aad96ef2 Deployed Sep 21, 2026 by aem-code-sync[bot]
milo_pr_merge 8c103455 Deployed Sep 16, 2026 by DKos95 via merge-to-stage #6750
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.

5 participants