Skip to content
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

MWPW-153237: Test content for CCD validation #3040

Merged
merged 4 commits into from
Oct 23, 2024
Merged

Conversation

3ch023
Copy link
Contributor

@3ch023 3ch023 commented Oct 15, 2024

Add a gallery page with CCD card variations.

Cards are pulled from Odin. At the moment you need VPN to access Odin instance.
Includes Dark and Light mode.
Known Missing Items (due to feature incomplete of Studio/Cards):

  • strikethrough price
  • yellow badge
  • card without a mnemonic icon
  • card with arbitrary CTA text (e.g. 'Try '
  • Suggested Card - background image (aka border image)

Resolves: https://jira.corp.adobe.com/browse/MWPW-153237

Access the gallery at https://ccd--milo--adobecom.hlx.page/libs/features/mas/docs/ccd.html?theme=dark
CCD branch is required due to dependencies on other PRs.

Test URLs:

@3ch023 3ch023 requested a review from a team as a code owner October 15, 2024 10:40
Copy link
Contributor

aem-code-sync bot commented Oct 15, 2024

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

  • Re-run PSI checks
  • Re-sync branch
Commits

@3ch023 3ch023 requested review from yesil and removed request for a team October 15, 2024 10:40
@3ch023 3ch023 assigned Axelcureno and unassigned Axelcureno Oct 15, 2024
@3ch023 3ch023 requested review from Axelcureno and npeltier October 15, 2024 10:41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [eslint] reported by reviewdog 🐶
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [eslint] reported by reviewdog 🐶
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [eslint] reported by reviewdog 🐶
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [eslint] reported by reviewdog 🐶
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.

Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.43%. Comparing base (c30fa5b) to head (7e0c053).
Report is 9 commits behind head on stage.

Additional details and impacted files
@@           Coverage Diff           @@
##            stage    #3040   +/-   ##
=======================================
  Coverage   96.43%   96.43%           
=======================================
  Files         245      245           
  Lines       55727    55727           
=======================================
+ Hits        53740    53743    +3     
+ Misses       1987     1984    -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

aem-code-sync bot commented Oct 15, 2024

Page Scores Audits Google
📱 /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

#log {
#log,
#log2,
#log3 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this section is probably not part of your PR but that's okay, in the ccd branch for the rest of the docs it is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ho, true. copy paste from ccd state..

<h5>Percentage</h5>
<h5>Two icons & pricing</h5>
<h5>See Terms link</h5>
<merch-card><aem-fragment fragment="0ef2a804-e788-4959-abb8-b4d96a18b0ef"></aem-fragment></merch-card>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it would be good to add title="Actual Product Title". It would help whom reads the MD file or updates it.

Copy link
Contributor Author

@3ch023 3ch023 Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some of cards have no title though (and some have the same title)
I was just using the copy markup function of studio

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't need to be matching with the exact title.
It is more descriptive, like cc, photoshop, illustrator.
I think as a best practice we should adopt it, maybe I'm wrong.

@@ -97,8 +99,7 @@ build({
});

mods.forEach((mod) => {
if (mod === 'lit.js') return;
if (mod === 'polyfills') return;
if (mod === 'lit.js' || mod === 'polyfills' || mod === 'themes') return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: if (['lit.js', 'polyfills', 'themes']).indexOf(mod) > -1 return;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right, i remember i promised
will change to ['lit.js', 'polyfills', 'themes'].includes(mod), good?

Copy link
Contributor

@npeltier npeltier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

Reminder to set the Ready for Stage label - to queue this to get merged to stage & production.

@afmicka afmicka added verified PR has been E2E tested by a reviewer Ready for Stage labels Oct 17, 2024
@adobecom adobecom deleted a comment from github-actions bot Oct 21, 2024
@3ch023 3ch023 added the run-nala Run Nala Test Automation against PR label Oct 21, 2024
@3ch023 3ch023 added run-nala Run Nala Test Automation against PR and removed run-nala Run Nala Test Automation against PR labels Oct 23, 2024
@milo-pr-merge milo-pr-merge bot merged commit c67ab1b into stage Oct 23, 2024
23 checks passed
@milo-pr-merge milo-pr-merge bot deleted the ccd-gallery branch October 23, 2024 15:24
@milo-pr-merge milo-pr-merge bot mentioned this pull request Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commerce Ready for Stage run-nala Run Nala Test Automation against PR verified PR has been E2E tested by a reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants