hlxsites / macquarie-help-centre

Apache License 2.0
1 stars 2 forks source link

Add Video block #60

Closed rhudea closed 1 year ago

rhudea commented 1 year ago

Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):

Fix #27

Test URLs:

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

Hello, I'm the AEM Code Sync 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
/personal/home-loans/manage-repayments-and-interest-rate/your-home-loan-interest PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
volteanu commented 1 year ago

The video-wrapper should match the width of default-content-wrapper (width: calc(100% * 2/3);). Right now it expands to 100% in desktop mode.

Not sure how best we can achieve it, especially that this will change with the "long article" styling...

volteanu commented 1 year ago

🐛 Clicking outside of the modal restarts the video instead of closing it.

volteanu commented 1 year ago

🐛 click on the X to close the video triggers a js error in the console:

Uncaught TypeError: player?.stopVideo is not a function
    at toggleVideoOverlay (video.js:82:15)
    at HTMLDivElement.<anonymous> (video.js:197:45)
volteanu commented 1 year ago

The "Watch now" button should be aligned with the bottom-left corner in mobile mode, i.e.:

image

Currently there's some padding (from the desktop view):

image
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/personal/home-loans/manage-repayments-and-interest-rate/your-home-loan-interest PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
asthabh23 commented 1 year ago

The video-wrapper should match the width of default-content-wrapper (width: calc(100% * 2/3);). Right now it expands to 100% in desktop mode.

Not sure how best we can achieve it, especially that this will change with the "long article" styling...

I agree on this, the width of the video wrapper needs to match the width of default content (for now, its full width almost)

Screenshot 2023-10-25 at 10 17 17 AM
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/personal/home-loans/manage-repayments-and-interest-rate/your-home-loan-interest PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/personal/home-loans/manage-repayments-and-interest-rate/your-home-loan-interest Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 404) PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/personal/home-loans/manage-repayments-and-interest-rate/your-home-loan-interest Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 404) PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/personal/home-loans/manage-repayments-and-interest-rate/your-home-loan-interest Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 404) PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/help/personal/home-loans/manage-repayments-and-interest-rate/your-home-loan-int
erest
PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/help/personal/home-loans/manage-repayments-and-interest-rate/your-home-loan-int
erest
PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
asthabh23 commented 1 year ago

@rhudea , customer confirmed that they just need inline support for videos. you could update the same in this PR itself

rhudea commented 1 year ago

Close in favor #84