-
Notifications
You must be signed in to change notification settings - Fork 145
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
Remove jQuery from CoBlocks #2276
Conversation
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.
@olafleur-godaddy this seems to work as expected on my test CoBlocks page. I think this will probably need another pair of eyes though before merging.
if ( $this->is_page_gutenberg() || has_block( 'coblocks/gallery-masonry' ) || has_block( 'core/block' ) ) { | ||
wp_enqueue_script( | ||
'coblocks-masonry', | ||
$dir . 'coblocks-masonry.js', | ||
array( 'jquery', 'masonry', 'imagesloaded' ), | ||
COBLOCKS_VERSION, | ||
true | ||
); | ||
} |
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.
I think we still need this because the v2 Masonry block is not compatible with WordPress version below 5.7.2. We allow users to use both versions of the block using a special wrapper and we need to keep the old logic around for that purpose.
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.
No, we are good on this I suppose. Turns out the Masonry block broke with 5.6 somewhere along the way. Let us go ahead and get this in and then we will have to look at that Masonry block later.
Please allow me to give this a good test with everything around the Masonry block and the v1-v2 wrapper that it uses before we merge this. |
Download coblocks.zip: https://45097-128991767-gh.circle-artifacts.com/0/tmp/artifacts/coblocks-2276.zip |
Performance Test Results:
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
Description
Remove the remains of jQuery that are not needed anymore.
Please tell me if I broke something or if I forgot to remove something :)
Fixes #1365
This is the culmination of the work of a lot of folks, especially @AnthonyLedesma and @snovosel :)