-
Notifications
You must be signed in to change notification settings - Fork 72
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
Speed up FidesJS initialization #4896
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
04954b4
to
d285545
Compare
Passing run #7835 ↗︎
Details:
Review all test suite changes for PR #4896 ↗︎ |
92f4d68
to
f4a8e46
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.
This approach looks pretty good. Couple bugs and comments in here
eec5b73
to
abef798
Compare
Co-authored-by: Neville Samuell <[email protected]>
Co-authored-by: Neville Samuell <[email protected]>
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.
Final hesitation...
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.
Thanks 👍. This feels pretty good now
Closes PROD-2082
Description Of Changes
FidesJS' initialization was depending on the DOM to complete loading. Here we change that initialization to fire off when the document is loaded and the DOM is in
interactive
state instead, ensuring that FidesJS is available much faster. However, in turn this creates a race condition for dynamic pages and slow loading pages between FidesJS initialization and having the page render the appropriate containers and links. That issue is addressed here as well.Code Changes
readyState
to beinteractive
instead ofcomplete
(this is the soonest we can safely proceed with the script).fides_embed
is set to true, set up polling to check if it gets added to the page later.modalLinkId
to see if it's an empty string rather than a real value or null. If it has and empty string, we don't look for the modal link at all. This effectively provides an "opt out" for developers who are rolling their own link usingFides.showModal();
Steps to Confirm
verify FidesJS is initializing faster
fides-js-demo.html
page in Privacy Center and uncomment these lines of code:interactive
state, and notcomplete
. Previously this could have caused the overlay banner to appear after the image had fully loaded. Now the banner will appear immediately despite the slow image. Verify that this is the case.fides-js-demo.html
verify the DOM checker polling is working
fides-js-demo.html
:window.render_delay = 5000;
to cause the page to update with embed container and modal link after 5 secondsfides-js-demo.html
Pre-Merge Checklist
CHANGELOG.md