-
Notifications
You must be signed in to change notification settings - Fork 10
Add S2 button whitelist with authorable outline variant #675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stage
Are you sure you want to change the base?
Changes from all commits
f4dc83f
f66005b
4b638fb
57ad10d
0366c17
c96fd54
58bc3ea
daf90de
3aabf2a
8505f5a
27eaaa6
548955d
812a36a
a71a3d1
f1c4048
01fea4b
effc97f
c78a05b
eec39ac
4287eb9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -365,6 +365,21 @@ function preloadLCPImage(img) { | |
| }()); | ||
|
|
||
| let fragmentLcpPreloaded = false; | ||
|
|
||
| // Rollout gate for the Spectrum-2 (s2) button system. Blocks listed here get the | ||
| // `s2` class applied centrally — no per-page authoring needed — so they render | ||
| // the proper S2 CTA styling (see the `.s2` rules in styles.css). Add a block | ||
| // name once its buttons are verified; when every block is verified, retire this | ||
| // gate and make s2 the default. Authors can still opt a single block in by | ||
| // adding the `s2` variant in the block's name row. | ||
| const S2_BUTTON_BLOCKS = ['ax-columns', 'transparent-img-marquee', 'comparison-table-v2', 'blog-columns']; | ||
|
|
||
| function applyS2ButtonClasses(area) { | ||
| S2_BUTTON_BLOCKS.forEach((name) => { | ||
| area.querySelectorAll(`.${name}`).forEach((block) => block.classList.add('s2')); | ||
| }); | ||
| } | ||
|
|
||
| // eslint-disable-next-line import/prefer-default-export | ||
| export function decorateAreaWithLCP(area = document, options = {}) { | ||
| const { fragmentLink } = options; | ||
|
|
@@ -381,6 +396,11 @@ export function decorateAreaWithLCP(area = document, options = {}) { | |
| } | ||
| } | ||
| decorateArea(area, options); | ||
| // Milo's core loadArea() never calls config.decorateArea for the main | ||
| // document — only its fragment block does, for fragment content (see | ||
| // libs/blocks/fragment/fragment.js). Blocks authored directly on the page | ||
| // (the common case) are classed explicitly after loadArea() in loadPage(). | ||
| applyS2ButtonClasses(area); | ||
|
Comment on lines
+399
to
+403
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because the button decoration function is called in the block. It'd be good to check with Ernest that we want to deviate from milo in this way. It sounds like once we retire the gate, buttons would get decorated in the body too. |
||
| } | ||
| CONFIG.decorateArea = decorateAreaWithLCP; | ||
|
|
||
|
|
@@ -575,6 +595,7 @@ async function loadPage() { | |
| }); | ||
|
|
||
| await loadArea(); | ||
| applyS2ButtonClasses(document); | ||
|
|
||
| const { fixIcons } = await import('./utils.js'); | ||
| document.querySelectorAll('.section>.text').forEach((block) => fixIcons(block)); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -309,7 +309,7 @@ export async function decorateButtonsDeprecated(el, size) { | |
| const { decorateButtons } = await import(`${getLibs()}/utils/decorate.js`); | ||
| // eslint-disable-next-line max-len | ||
| // DO NOT add any more exceptions here. Authors must learn to author buttons the new milo way, even with old blocks | ||
| if (!el.closest('.ax-columns') && !el.closest('.banner') && !el.closest('.fullscreen-marquee') && !el.closest('.link-list')) decorateButtons(el, size); | ||
| if (!el.closest('.banner') && !el.closest('.fullscreen-marquee') && !el.closest('.link-list')) decorateButtons(el, size); | ||
| // DO NOT add any more exceptions above. We should be removing the exceptions and not adding more. | ||
| el.querySelectorAll(':scope a:not(.con-button, .social-link)').forEach(($a) => { | ||
| // Mirrors decorateButtons' own #_button-<name> handling (milo's utils/decorate.js) | ||
|
|
@@ -364,6 +364,24 @@ export async function decorateButtonsDeprecated(el, size) { | |
| $a.classList.add('button', 'accent', 'light'); | ||
| $twoup.classList.add('button-container'); | ||
| } | ||
| // Custom button variants authored the milo way via `#_button-<class>` | ||
| // hashes on the href (e.g. `#_button-fill`, `#_button-outline`). The | ||
| // blocks that reach this fallback (ax-columns, banner, | ||
| // fullscreen-marquee, link-list) bypass milo's decorateButtons, which | ||
| // would normally strip these hashes and add the classes — so replicate | ||
| // that here: add each class, strip the hash, and drop the default | ||
| // `accent` when an explicit fill/outline variant is requested. | ||
| // Additive and hash-gated: no effect on content without these hashes. | ||
| if ($a.classList.contains('button')) { | ||
| const customClasses = [...originalHref.matchAll(/#_button-([a-zA-Z-]+)/g)]; | ||
| customClasses.forEach(([token, cls]) => { | ||
| $a.classList.add(cls); | ||
| $a.setAttribute('href', $a.href.replace(token, '')); | ||
| }); | ||
| if ($a.classList.contains('fill') || $a.classList.contains('outline')) { | ||
| $a.classList.remove('accent'); | ||
| } | ||
| } | ||
|
Comment on lines
+367
to
+384
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally we would upgrade to the new button decoration rather than continue to add code to the deprecated function. Do you have an idea of how much work that would be?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why ax-columns stays on decorateButtonsDeprecated (whitelist), not milo's native decorateButtons We prototyped switching ax-columns to call milo's real decorateButtons directly, matching the pattern already used by blog-columns, comparison-table-v2, transparent-img-marquee, etc. It doesn't hold up for this block:
Given ax-columns is one of our highest-traffic, highest-regression-risk blocks, the whitelist/deprecated-fallback approach remains the safer choice until/unless we're ready to also handle the plain-link gap deliberately, rather than fold it into a "quick" migration. https://main--da-express-milo--adobecom.aem.page/drafts/echen/plain-link-decoration-demo |
||
| } | ||
| if (linkText.startsWith('{{icon-') && linkText.endsWith('}}')) { | ||
| const $iconName = /{{icon-([\w-]+)}}/g.exec(linkText)[1]; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you confirm with design that S2 is the right name for our design system? We're not fully on S2 so they might have a name more specific to express.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its based on the S2 buttons in the design, I think this is a good name for it.