Use the new wp.domReady helper if it is available #757
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes issues with Fieldmanager fields that require JavaScript hooks not initializing properly when Gutenberg is active. wp.domReady fires later than jQuery's document.ready, so this should resolve race conditions that prevent initialization of dynamic fields across the board in Fieldmanager.
This reverts commit 0f836e5.
mboynes
approved these changes
Aug 26, 2019
There was a problem hiding this comment.
Thank you!
From a code-level perspective, this is fantastic. We should run a systematic QA on it, testing a version each of WP 4.x and 5.x, and testing a variety of browsers down to IE11. Can you coordinate that @kevinfodness?
|
@mboynes yep, I can coordinate that. |
|
Would love to know the status of this update? |
|
@rfair404 this is a fairly significant change to the way we are loading JS in the plugin, so we want to run it through a gamut of tests before merging, which we haven't completed yet. We are dogfooding it on a few production sites with no issues though, so if it's possible for you to use this branch while we complete testing, I'd love to hear your feedback. |
|
I loaded the branch in my local environment and can confirm that this fixes the issues that I'm currently having with colorpicker, datepicker, autocomplete and richtext. |
|
I was not able to get fm-zones to work properly. |
|
We've been trying this out and it's working really well for dropdown / autocompletes inside of Gutenberg. The one thing I noticed was that I sometimes get "wp is not defined" in non-Gutenberg contexts. The way I fixed it was to change line 8 of fieldmanager-loader.js to: Adding that extra check for undefined appears to solve the issue |
|
These Issues were found by @montchr. a roundup of the issues i ran into: On my local environment I noticed this error message. I did have him try the fix for the |
|
@JasonHoffmann I just pushed an update that should fix the |
|
This has been battle-tested enough that we're confident with shipping it. Merging to master. |
This was referenced Apr 15, 2020
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes issues with Fieldmanager fields, which require JavaScript hooks, not initializing properly when Gutenberg is active.
wp.domReadyfires later than jQuery'sdocument.ready, so this, combined with moving most FM scripts to the footer, should resolve race conditions that prevent initialization of dynamic fields across the board in Fieldmanager.Additionally, uses
FM_VERSIONto set version numbers for Fieldmanager JS and CSS to ensure cache busting when shipping a new version of Fieldmanager to aggressively cached sites, such as VIP Go. Accordingly, bumps the version to 1.2.5.Finally, moves script load for most FM scripts to the footer, which is required for proper initialization of fields in a Gutenberg context, and is generally a best practice. This move should not break anything in practice, but may require a change to the qunit tests in order for them to pass.
This PR obviates the need for the following PRs:
#745
#744
#717
It also resolves the following issues:
Fixes #753 #730 #720 #713 #710