Added styles and render logic to reduce load jank #56
Conversation
|
@jeffpaul I just noticed that the JS lint fails, it's saying there's a package missing which my changes didn't alter either. Possible version issue with the deps? |
| @@ -1,7 +1,7 @@ | |||
| import Webamp from 'webamp'; | |||
| import domReady from '@wordpress/dom-ready'; | |||
There was a problem hiding this comment.
@wordpress/dom-ready is no more a dependency for this and other scripts, could be removed from package.json
There was a problem hiding this comment.
@iansvo if you concur with ^, mind a quick commit to pull that out?
There was a problem hiding this comment.
@cadic could you handle that update and then look to get this merged in?
# Conflicts: # package-lock.json
|
@10up/open-source-practice would be great to get some assistance resolving the merge conflicts here so that this can get into final review/merge... thanks! |
# Conflicts: # package-lock.json # src/frontend.js
|
@10up/open-source-practice I've resolved the merge conflicts but am unable to reproduce the issue described so will need someone else to test and review. |
There was a problem hiding this comment.
Thanks for resolving the conflicts @peterwilsoncc
I was not able to reproduce this issue locally however I was able to on a live site.
I used instawp to test.
Issue on the develop branch:
Screen.Recording.2023-01-23.at.12.31.03.PM.mov
Issue resolved in the fix brach:
Screen.Recording.2023-01-23.at.12.37.56.PM.mov
I have tested on pages with and without content and it worked well for me.
Description of the Change
Closes #38
This adds some CSS to remove the element from the document flow (via absolute positioning) and using a timer that starts when the load event fires in the browser to then remove the absolute positioning.
The effect is that the element is inserted into the DOM after its dimensions are calculated, which prevents the scrolling.
Possible Drawbacks
The timeout value is at 1000ms, which is kind of arbitrary. There isn't a complete way to guarantee perfect loading as the widget could delay for any number of reasons unrelated to the webpage (such as network speed, device resources, and so on).
It is possible we might want to hook into this using some other mechanism (mutation change, perhaps?) but that rabbit hole can get deep really quickly and I think the solve should start simpler and move from there after we do some real world testing.
Verification Process
I started with replicating the issue locally on a barebones page that didn't have much on it besides the widget. I was able to replicate the issue with relative consistency when the page was not cached.
After the update I went through the same steps and found the issue was not able to be reproduced.
Note: This issue should be verified on a staging/production environment using a configured theme and a more realistic page setup to confirm the durability of the solution. Hopefully this is something triage in the related issue can resolve.
Checklist:
Changelog Entry
Added - Additional handling to prevent page jank and scrolling as the webamp widget loads
Credits
Props @iansvo