Skip to content

Update tabs and product-marquee-grid C2 blocks for bizpro - #6657

Open
rgclayton wants to merge 1 commit into
stagefrom
bizpro-tabs-product-marquee-grid-stage
Open

Update tabs and product-marquee-grid C2 blocks for bizpro#6657
rgclayton wants to merge 1 commit into
stagefrom
bizpro-tabs-product-marquee-grid-stage

Conversation

@rgclayton

@rgclayton rgclayton commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Ports the tabs and product-marquee-grid C2 blocks from the mep/ace1209 sandbox (as they exist on site-redesign-foundation) into libs/c2/blocks/, so the bizpro pages going live next week have access to them. Scoped to only these two blocks — no other files touched.

  • tabs: adds radio and quiet variants alongside the existing pill tabs, with matching keyboard/ARIA handling for the radiogroup case.
  • product-marquee-grid: replaces the old promo-button/soft-offer CTA with a merch-card layout driven by mas-field/mas-price commerce data. Note: this changes rendering for any existing content authored against the old promo/soft-offer markup.

Test plan

  • Verify tabs pill/radio/quiet variants render and behave correctly (keyboard nav, ARIA)
  • Verify product-marquee-grid renders merch-card pricing content correctly
  • Confirm no regressions on existing pages using these blocks

🤖 Generated with Claude Code

Ports the latest tabs and product-marquee-grid block implementations
from the mep/ace1209 sandbox (site-redesign-foundation) into c2/blocks
so bizpro pages have access to them.

- tabs: adds radio and quiet variants alongside the existing pill tabs,
  plus matching keyboard/ARIA handling for the radiogroup case.
- product-marquee-grid: replaces the promo-button/soft-offer CTA with
  a merch-card layout driven by mas-field/mas-price commerce data.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@aem-code-sync

aem-code-sync Bot commented Sep 4, 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

@rgclayton

Copy link
Copy Markdown
Contributor Author

Needed for an exceptional release on 9/7

@github-actions

github-actions Bot commented Sep 5, 2026

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.

@SilviuLCF SilviuLCF self-assigned this Sep 7, 2026
@SilviuLCF
SilviuLCF self-requested a review September 7, 2026 07:28
@mokimo

mokimo commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review — PR #6657 (bizpro-tabs-product-marquee-grid-stagestage)

Size: +426/-206 across 4 files (both existing libs/c2/blocks/ dirs, no other files touched) — well within normal PR size, no split needed. Base branch is correctly stage.

Went through both blocks in detail (diff + head-branch source). No blockers. A few things worth a look before/after merge:

Suggestions

  • libs/c2/blocks/product-marquee-grid/product-marquee-grid.js:24getFederatedUrl(iconEl.src) was changed to getFederatedUrl(iconEl.getAttribute('src')). getFederatedUrl only takes its fast, safe path for values starting with /; anything else falls through to new URL(url), which throws (caught silently, original url returned unchanged) for a relative path with no leading slash. Using the raw attribute instead of the browser-resolved absolute .src means a relative icon path (no leading /) would now silently skip federation instead of resolving. Worth confirming authored icon paths are always root-relative, or reverting to .src.

  • Disclosed breaking change, flagging for visibility: the PR description already calls out that product-marquee-grid's CTA rendering changed from promo-button/soft-offer to a merch-card layout, and that this "changes rendering for any existing content authored against the old promo/soft-offer markup." buildSoftOfferCta/buildPromoButton are fully removed with no fallback path, so any already-published page still authored with the old markup will render broken/empty CTAs. Worth a quick check for live pages on the old variant before this merges, given it targets stage.

  • libs/c2/blocks/tabs/tabs.css:185 and :239 — this PR migrates the default/pill variant's focus ring to the new --s2a-color-focus-ring-default token (line 104), but the new radio variant's focus style (line 185) and the new quiet variant's active underline (line 239) still hardcode the old --s2a-color-blue-800. Minor token-consistency nit since both were added in this same PR.

  • No NALA coverage for the new behavior. nala/blocks/tabs/* only covers the classic (non-c2) tabs block; there's no test coverage (existing or added) for the new radio/quiet c2-tabs variants or their keyboard handling (Up/Down arrow nav, native radiogroup semantics), nor for the new merch-card layout in product-marquee-grid. Given the radiogroup interaction is genuinely new ARIA surface, a follow-up test would be good.

  • Pre-existing gap, not introduced here: CODEOWNERS has zero entries for anything under libs/c2/blocks/, including these two (compare to their classic counterparts, e.g. /libs/blocks/tabs/ @adobecom/milo-core). Not this PR's fault, but worth a separate follow-up since it applies to every c2 block.

Nice-to-have

  • tabs.css:1/* stylelint-disable no-descending-specificity */ is applied file-wide; could probably be scoped closer to the specific selector(s) that trip the rule.
  • PR description's test-plan checkboxes are all unchecked and there's no live preview URL (https://<branch>--milo--adobecom.aem.page/...). Given the breaking-change note above, a preview link would help reviewers confirm the regression risk before merge.

What looks good

  • Correctly scoped to exactly the two blocks named in the title, no incidental changes.
  • Reuses existing utils (createTag, decorateButtons, getFederatedUrl) rather than reinventing them.
  • No new dependencies, no console.log/debugger/dead code found.
  • The radio/quiet tabs implementation is careful about ARIA: role stays radiogroup for the radio variant (native <input type=radio> semantics used instead of aria-selected), tabpanel role correctly omitted only for the radio case, and the CSS selectors (.tabpanel vs [role="tabpanel"]) were updated consistently everywhere that mattered.

Net: solid PR, no blocking issues — the merch-card breaking-change point above is the one item I'd actually want a human answer on before merge, everything else is a follow-up-level nit.

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

QA verified , testing details https://jira.corp.adobe.com/browse/MWPW-206720

@narcis-radu
narcis-radu self-requested a review September 8, 2026 10:17
import { decorateViewportContent, decorateButtons } from '../../../utils/decorate.js';

const CHEVRON_SVG = '<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 12 12" fill="none"><path d="M6.58349 11.208L10.2837 7.50781C10.606 7.18554 10.606 6.66406 10.2837 6.34179C9.96142 6.01953 9.43994 6.01953 9.11767 6.34179L6.82568 8.63281V1.375C6.82568 0.918955 6.45654 0.549805 6.00048 0.549805C5.54442 0.549805 5.17528 0.918945 5.17528 1.375V8.63281L2.88329 6.34179C2.56102 6.01953 2.03954 6.01953 1.71727 6.34179C1.55614 6.50292 1.47508 6.71386 1.47508 6.9248C1.47508 7.13574 1.55613 7.34668 1.71727 7.50781L5.41747 11.208C5.73974 11.5303 6.26122 11.5303 6.58349 11.208Z" fill="#fff"/></svg>';
const MAS_FIELD_CLASSES = {

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.

@rgclayton - please add the updated product-marquee-grid tests

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.

4 participants