MWPW-140481: Delayed modal

[mirafedas]

This feature enables the delayed display of a modal, primarily intended for promotional purposes.

When parsing the promotional manifest, if we encounter the insertContentAfter rule containing a link with a delay parameter set to any positive number we dynamically generate a hidden link within the specified selector. The selector should point to any element inside the

tag. The inserted link behaves conventionally, triggering a modal upon click. Following this setup, we introduce a delay of some seconds before unveiling the modal, simultaneously appending a modal hash to the browser's URL. After the modal is displayed once on the page, we remember this in sessionStorage to prevent subsequent displays upon page reload.
When the modal is opened, we send the analytics event with the following name format: modal-hash:modalOpen.
The rest of the analytics events remain the same as before.

Resolves: MWPW-140481

Authoring documentation

Test URLs:

Before: not applicable, because this is a newly added feature
After:
First page
Second page
Third page
Feel free to change the delay params in this file.

[aem-code-sync]

Page	Scores	Audits	Google
/drafts/mirafedas/showdelayedmodalhere?manifest=%2Ffragments%2Fmirafedas%2Fpromo s%2F2024%2Fglobal%2Fdelayed-modal-promo%2Fdelayed-modal-promo.json--all
/drafts/mirafedas/secondpage
/drafts/mirafedas/thirdpage

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.34%. Comparing base (fa5f12a) to head (6aafe94).

❗ Current head 6aafe94 differs from pull request most recent head 7b9d092. Consider uploading reports for the commit 7b9d092 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            stage    #1939      +/-   ##
==========================================
- Coverage   96.36%   96.34%   -0.02%     
==========================================
  Files         166      166              
  Lines       42531    42562      +31     
==========================================
+ Hits        40986    41008      +22     
- Misses       1545     1554       +9     

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

[chrischrischris]

Does display have to be a required property? I guess it'd be annoying to have a delayed modal every time.

[mirafedas]

good point, I guess I can make one of the display values a default one, so that authors would need to specify this only if they want a second value to be applied. I will ask Fred which one should be a default one, 'pageload' or 'session' and update the PR

[mirafedas]

UPD: turns out I misunderstood the requirements, instead of two display modes there will be only one. The modal should be shown on every page a user visits only once during one browser session. I updated the code, tests, and documentation.

[Roycethan]

@mirafedas

1. Looks like modal-hash:modalOpen is not triggered during launch
   2)Click event outside the modal has web interaction name 'modalClose:buttonClose'
   Ideally it should be : 'modalClose:overlayClose' to distinguish between button click and outside clicks.
   3)Link clicks ( save, see terms) also should have defined web-interaction name

[mirafedas]

@Roycethan

1. I cannot reproduce, for me modal-hash:modalOpen is triggered every time. Could you please try in the browser dev tools > Application > Session storage > mwpw-140481-delayed-modal-stage--milo--mirafedas.hlx.live removing the key shown:#delayed-modal, and reload the page to check again?

2. Currently on production for all modals we emit the modalClose: button-close event no matter if the button or overlay was clicked.
   Changing this falls outside of the scope of this ticket, we'd need to create a separate ticket to change event names for all modals.

3. For this we also need a separate ticket, because this is related to the fragment links functionality, and not to the modal.
   Both 2 and 3 are not in the ticket AC.

[Roycethan]

Hi @mirafedas Since you asked analytics feedback in the ticket , updated these from my side as well
#1 was talking about analytics event and not session storage log
#2 Yes this existing a.com production behavior ex: https://www.adobe.com/creativecloud/plans.html#cct-all-apps-products-includes
#3 as well needed for tracking.
Plz add a future ticket for reference or create one when you have it... As far as Functionality perspective this looks good to me. Thank you

[mirafedas]

@Roycethan
About analytics, I'll ask Samuel Wang to create the ticket and describe all the requirements, just to be sure we don't mess up the statistics with this change.
About the open event not being triggered, let's have a call and take a look at it together, I'll explain about the session storage key :)

[chrischrischris]

When does regex test throw an error? I'm not aware of any value that would cause it to.

[mirafedas]

The line break can cause the error, e.g.

const delayedModalRegExp = /\/.*?#.*?:delay=[1-9][0-9]*/;
delayedModalRegExp.test('This is a string
with a line break.')
// Uncaught SyntaxError: Invalid or unexpected token

I know it's pretty unlikely this may happen, but authors may copy-paste such string into Excel from somewhere else, so I decided to add this checking as we don't have any validation on the authoring side.

[chrischrischris]

Do you think there is a use case where a delayed modal would be triggered by a cta on the page? I can't think of why that would ever be needed. If that's the case, then I think we can move this logic directly into modal.js.

function delayedModal could directly add el.classList.add('hide-block'); to the modal link.

Then we don't even need any support for #hide, and there's no code changes needed in personalization.js

[honstar]

@mirafedas , what's the reason for marking this PR as draft? Draft PRs are generally discouraged, please see https://github.com/adobecom/milo/wiki/Submitting-PRs#drafts-prs.

[Blainegunn]

@mirafedas PSI check is failing.
Can you get it to pass or give a reason why it won't hurt performance please?
I've looked into this and the reason is the test pages aren't meant to perform.
Please fix merge conflicts. thank you

[mirafedas]

@Blainegunn
Merge conflicts resolved.
I did not place a marquee on my test page, so the image inside a delayed modal was considered an LCP, and as the modal was rendered with a delay, it impacted the statistics. I added the marquee, and now the page speed result is green.