hlxsites / merative2

Merative.com site on Franklin
https://merative.com
Apache License 2.0
2 stars 9 forks source link

feature(global-cta-modal-implementation): Global CTA Video Modal. #335

Closed nimithshetty17 closed 1 year ago

nimithshetty17 commented 1 year ago

Issue

Fixes #MERATIVE-858

Description

We need to ensure that all of the button styles (e.g.. Primary, Secondary, Tertiary) (see - https://main--merative2--hlxsites.hlx.page/block-library/buttons/buttons-video) can support video modal when the href is defined as a video type.

Changed

image

Test URLs

Testing Instruction

To test if the video player is opening a modal for all the button and link styles.

aem-code-sync[bot] commented 1 year ago

Hello, I'm Franklin Bot and I will run some test suites that validate the page speed. In case there are problems, just click the checkbox below to rerun the respective action.

aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/block-library/buttons/buttons-video PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/block-library/buttons/buttons-video PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/block-library/buttons/buttons-video PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/block-library/buttons/buttons-all PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/block-library/buttons/buttons-all PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/block-library/buttons/buttons-all PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
proeung commented 1 year ago

@nimithshetty17 I decided to revert this PR from the main branch due to some of the 404 errors in the console log (see attached). Looks like the video model gets added to every block element (https://github.com/hlxsites/merative2/pull/335/files#diff-a5ee87ff9063f921ba01933977e6164e5729551986030f0ed7eb747bcffb740fR727), which tells the the loadBlock function to build and load in the blocks (https://github.com/hlxsites/merative2/blob/main/scripts/lib-franklin.js#L349) for classes .video-modal-overlay and .video-modal-container.

You might need to adjust your approach to appending the video overlay outside of the main HTML tag and append the video embed onClick.

Screenshot 2023-09-15 at 1 06 41 PM

Let's open a new PR for this work and include that fix that eliminates the 404 error in the console log.