-
Notifications
You must be signed in to change notification settings - Fork 41
MWPW-170589: POC Loading Unity after LCP #1076
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
base: stage
Are you sure you want to change the base?
Conversation
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a proof-of-concept for loading the Unity block and its dependencies after the Largest Contentful Paint (LCP) event to optimize page performance.
- Added a new function to create and load a Unity block within the verb widget.
- Updated the analyticsLoad event listener to call the new function in both LCP and fallback branches.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## stage #1076 +/- ##
==========================================
- Coverage 73.87% 73.64% -0.23%
==========================================
Files 51 51
Lines 7732 7756 +24
==========================================
Hits 5712 5712
- Misses 2020 2044 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
63fe49c
to
2f82b8c
Compare
a68d2b2
to
adbd90c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
acrobat/blocks/verb-widget/verb-widget.js:463
- Consider awaiting loadUnityAfterLCP() to handle potential errors unless fire-and-forget execution is intentional.
loadUnityAfterLCP();
acrobat/blocks/verb-widget/verb-widget.js:468
- Consider awaiting loadUnityAfterLCP() to ensure any errors during Unity block loading are caught, unless non-blocking behavior is desired.
loadUnityAfterLCP();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
eslint
🚫 [eslint] <object-curly-spacing> reported by reviewdog 🐶
A space is required after '{'.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 801 in adbd90c
if(canSendDataToSplunk) window.analytics.sendAnalyticsToSplunk(event, VERB, {...metadata, errorData}, getSplunkEndpoint()); |
🚫 [eslint] <object-curly-spacing> reported by reviewdog 🐶
A space is required before '}'.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 801 in adbd90c
if(canSendDataToSplunk) window.analytics.sendAnalyticsToSplunk(event, VERB, {...metadata, errorData}, getSplunkEndpoint()); |
🚫 [eslint] <no-use-before-define> reported by reviewdog 🐶
'getSplunkEndpoint' was used before it was defined.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 801 in adbd90c
if(canSendDataToSplunk) window.analytics.sendAnalyticsToSplunk(event, VERB, {...metadata, errorData}, getSplunkEndpoint()); |
🚫 [eslint] <indent> reported by reviewdog 🐶
Expected indentation of 6 spaces but found 4.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 831 in adbd90c
? 'https://unity.adobe.io/api/v1/log' |
🚫 [eslint] <indent> reported by reviewdog 🐶
Expected indentation of 6 spaces but found 4.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 832 in adbd90c
: 'https://unity-stage.adobe.io/api/v1/log'; |
🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 832 in adbd90c
: 'https://unity-stage.adobe.io/api/v1/log'; |
🚫 [eslint] <max-len> reported by reviewdog 🐶
This line has a length of 108. Maximum allowed is 100.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 835 in adbd90c
function handleAnalyticsEvent(eventName, metadata, documentUnloading = true, canSendDataToSplunk = true) { |
🚫 [eslint] <keyword-spacing> reported by reviewdog 🐶
Expected space(s) after "if".
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 837 in adbd90c
if(!canSendDataToSplunk) return; |
🚫 [eslint] <no-multi-spaces> reported by reviewdog 🐶
Multiple spaces found before 'return'.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 837 in adbd90c
if(!canSendDataToSplunk) return; |
🚫 [eslint] <no-shadow> reported by reviewdog 🐶
'userAttempts' is already declared in the upper scope on line 491 column 9.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 845 in adbd90c
function handleUploadingEvent(data, userAttempts, cookieExp, canSendDataToSplunk) { |
🚫 [eslint] <no-shadow> reported by reviewdog 🐶
'userAttempts' is already declared in the upper scope on line 491 column 9.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 858 in adbd90c
function handleUploadedEvent(data, userAttempts, cookieExp, canSendDataToSplunk) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
eslint
🚫 [eslint] <no-shadow> reported by reviewdog 🐶
'userAttempts' is already declared in the upper scope on line 491 column 9.
dc/acrobat/blocks/verb-widget/verb-widget.js
Line 858 in adbd90c
function handleUploadedEvent(data, userAttempts, cookieExp, canSendDataToSplunk) { |
a594c3d
to
60e5874
Compare
* stage: send browser tab closure event to splunk # Conflicts: # acrobat/blocks/verb-widget/verb-widget.js
Description
This PR defers the loading of the Unity block and its libraries until after LCP event is completed. The goal is to reduce render-blocking resources during initial load and improve Core Web Vitals, specifically LCP
Related Issue
Resolves: MWPW-170589
Test URLs