MWPW-160511: badge & switch to AEM Prod #3094
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
|
| @@ -25,7 +25,10 @@ export class CCDSuggested extends VariantLayout { | |||
| return html` | |||
| <div style="${this.stripStyle}" class="body"> | |||
| <div class="header"> | |||
| <slot name="icons"></slot> | |||
| <div class="top-section"> | |||
| <slot name="icons"></slot> | |||
There was a problem hiding this comment.
should we rename the slot to mnemonics?
There was a problem hiding this comment.
i'd say yes, but then we need to touch all variants? it's 'icons' across the whole thing
There was a problem hiding this comment.
maybe it's my laziness, but imo we for now could just rename the authoring UI and keep them icons in code, since it's clear enough what it is. but up to you
There was a problem hiding this comment.
we have a mechanism already to map properties to slot names:
https://github.com/adobecom/milo/blob/stage/libs/features/mas/web-components/src/variants/ccd-suggested.js#L10
so we can afford mnemonic slot and map it correctly.
later it would be good, when someone inspects the DOM and sees icon instead of mnemonic.
| @@ -68,6 +71,9 @@ styles.innerHTML = ` | |||
| --merch-color-focus-ring: #1473E6; | |||
| --merch-color-grey-60: var(--spectrum-global-color-gray-600, #6D6D6D); | |||
| --merch-color-grey-80: #2c2c2c; | |||
| /* beware, 'gray' not 'grey'*/ | |||
There was a problem hiding this comment.
I think the grey comment is not needed. but message is understood.
There was a problem hiding this comment.
yeah i was wondering if it's helpful or not..
i had a bit of pain noticing the diff, so decided to add a note for the next person
There was a problem hiding this comment.
removed the message and actually renamed it to 'grey' to be consistent.
There was a problem hiding this comment.
this PR doesn't seem to have merged stage into correctly.
already merged changes are reported as new here: https://github.com/adobecom/milo/blob/stage/libs/blocks/merch/merch.js#L48
3ch023
force-pushed
the
MWPW-160511-badge
branch
from
c031e94 to
a1506e9
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
libs/deps/mas/mas.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
libs/deps/mas/merch-card.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
libs/features/mas/mas/dist/mas.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #3094 +/- ##
=======================================
Coverage 96.34% 96.35%
=======================================
Files 245 245
Lines 56312 56329 +17
=======================================
+ Hits 54252 54274 +22
+ Misses 2060 2055 -5 ☔ View full report in Codecov by Sentry. |
|
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. |
3ch023
force-pushed
the
MWPW-160511-badge
branch
from
70504ef to
53a4864
Compare
|
@3ch023 Do you know what exactly size the badge font should be? |
There was a problem hiding this comment.
@3ch023 why this file is in your PR? Are you fixing a flaky test?
There was a problem hiding this comment.
yes. it was failing on my PR and i have a feeling it fails for other prs sometimes.
trying to be a good samaritan :D
|
Approving the PR to unblock the work. As agreed created the follow-up jira for the above observations. https://jira.corp.adobe.com/browse/MWPW-161357 cc: @3ch023 @Roycethan |
This PR will have to go to stage after #3063, so for now it's based on the Axels feature branch
Resolves: MWPW-160511
CCD test page is not yet ready, waiting for the Prod publish of CFs:
MWPW-160511-badge--milo--adobecom.hlx.page/libs/features/mas/docs/ccd.html
Test URLs: