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-153363] Countdown Timer implementation based on page metadata #2928

Open
wants to merge 11 commits into
base: stage
Choose a base branch
from

Conversation

rahulgupta999
Copy link
Contributor

@rahulgupta999 rahulgupta999 commented Sep 20, 2024

As presented in AST, the regions have been using a ticker/timer/countdown visual to boost click-through rates and engagement as an enticement for certain promotions; e.g., Black Friday / Cyber Monday

image

Countdown Timer (CDT)
Properties:

Resolves: MWPW-153363

Test URLs:

Before: https://main--milo--adobecom.hlx.page/drafts/rahulgup/marquee-parent?martech=off
After: https://cdt-metatag--milo--rahulgupta999.hlx.page/drafts/rahulgup/marquee-parent?martech=off

This has the following authoring dependencies
New Placeholder texts are to be added in the placeholder file
{{cdt-ends-in}}
{{cdt-days}}
{{cdt-hours}}
{{cdt-mins}}

additionally, the following metadata should be present on the page
<meta name="countdown-timer" content="2024-08-26 12:00:00 PST,2024-09-30 00:00:00 PST">

I do not have access to promotions sheet to add metadata.

Copy link
Contributor

aem-code-sync bot commented Sep 20, 2024

@rahulgupta999 rahulgupta999 added run-nala Run Nala Test Automation against PR commerce new-feature New block or other feature labels Sep 20, 2024
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 93.05556% with 10 lines in your changes missing coverage. Please review.

Project coverage is 96.24%. Comparing base (1791eb8) to head (2a9199d).

Files with missing lines Patch % Lines
libs/features/cdt/cdt.js 92.12% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            stage    #2928      +/-   ##
==========================================
- Coverage   96.24%   96.24%   -0.01%     
==========================================
  Files         236      237       +1     
  Lines       54278    54421     +143     
==========================================
+ Hits        52241    52375     +134     
- Misses       2037     2046       +9     

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

@rahulgupta999
Copy link
Contributor Author

I am waiting for access to promotions sheets to test this PR. For now, I have tested using injection.

@rahulgupta999 rahulgupta999 changed the title [MWPW-153363] Countdown Timer implementation based on Page Metadata [MWPW-153363] Countdown Timer implementation based on page metadata Sep 20, 2024
@rahulgupta999 rahulgupta999 added the high priority Why is this a high priority? Blocker? Critical? Dependency? label Sep 20, 2024
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.

a few nits, but most of those changes are imo required

libs/blocks/hero-marquee/hero-marquee.js Outdated Show resolved Hide resolved
libs/blocks/marquee/marquee.js Outdated Show resolved Hide resolved
libs/blocks/media/media.js Outdated Show resolved Hide resolved
libs/features/cdt/cdt.js Outdated Show resolved Hide resolved
libs/features/cdt/cdt.js Outdated Show resolved Hide resolved
libs/features/cdt/cdt.js Outdated Show resolved Hide resolved
libs/features/cdt/cdt.js Outdated Show resolved Hide resolved
libs/features/cdt/cdt.js Outdated Show resolved Hide resolved
libs/features/cdt/cdt.js Outdated Show resolved Hide resolved
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

nit, in createTag, last param can be { parent: container }, and it will be appended to container which should make you save a few lines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commerce high priority Why is this a high priority? Blocker? Critical? Dependency? new-feature New block or other feature run-nala Run Nala Test Automation against PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants