Skip to content
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

ISBAT use CoBlocks without jQuery #1365

Closed
7 of 16 tasks
Tracked by #2078 ...
AnthonyLedesma opened this issue Feb 17, 2020 · 3 comments · Fixed by #2276
Closed
7 of 16 tasks
Tracked by #2078 ...

ISBAT use CoBlocks without jQuery #1365

AnthonyLedesma opened this issue Feb 17, 2020 · 3 comments · Fixed by #2276
Assignees
Labels
[Priority] High This issue/pull request needs resolving before the next release [Type] Enhancement Something new that adds functionality

Comments

@AnthonyLedesma
Copy link
Member

AnthonyLedesma commented Feb 17, 2020

Is your feature request related to a problem? Please describe.

CoBlocks should consider removing jQuery as a required dependency for the plugin. Getting a look at the plugin I find that 20 files reference jQuery which could be refactored to use plain JavaScript. This change would help to make CoBlocks more performant.

Required changes

Events & Post Carousel block

  • Remove Slick carousel in favor of Flickity. (Slick is a jQuery extension) - Will be removed following Events/Post-Carousel refactor
  • src/js/coblocks-events.js initializes slick in the DOM. - Needs Block refactor
  • src/js/coblocks-slick-initializer-front.js - Will be removed following Events/Post-Carousel refactor ( In progress )
  • src/js/coblocks-slick-initializer.js - Will be removed following Events/Post-Carousel refactor ( In progress )

Form block - jQuery UI Datepicker | Should be another open-source date picker.

Gif block

Gallery Carousel src/js/coblocks-accordion-carousel.js - ( PR: #2147 )

  • Refactor to vanilla-js swiper package

Gallery Masonry src/js/coblocks-masonry.js

Google Maps Script src/js/coblocks-google-maps.js

CoBlocks Lightbox src/js/coblocks-lightbox.js

CoBlocks Crop Settings Extension & Component

Cleanup tasks

  • Review includes/class-coblocks-block-assets.php and remove jQuery from dependency arrays.
    - This file will be able to remove all references to jQuery once the following blocks are refactored
    - Events
    - Post-Carousel
  • Review package.json and remove jQuery from devDependencies
    - As stated above, both the Events and Post-Carousel blocks must be refactored away from Slick before we can remove jQuery from the devDependencies file

Inspired by WordPress.org support request
https://wordpress.org/support/topic/did-you-start-using-jquery-in-a-recent-release/

@AnthonyLedesma AnthonyLedesma added [Type] Enhancement Something new that adds functionality [Priority] Low This issue/pull request is not immediate labels Feb 17, 2020
@jrtashjian
Copy link
Member

We can use the Fetch API to replace this.

Reference:
https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API

@josephscott
Copy link
Contributor

This would be a very welcome improvement.

I was running performance tests on a scratch site and just activating the CoBlocks plugin was enough to trigger adding jQuery, jQuery Migrate ( and slick.js ) for page views. In mobile performance tests I ran, activating the CoBlocks plugin slowed down start render by more than 30%.

There are great resources on replacing jQuery code with vanilla JS, like http://youmightnotneedjquery.com/ - which can be very helpful in an update like this, in combination with targeting IE and IE versions ( if that is a concern ).

See also WordPress/gutenberg#15256

@jrtashjian jrtashjian added [Priority] High This issue/pull request needs resolving before the next release and removed [Priority] Low This issue/pull request is not immediate labels Feb 22, 2020
@jrtashjian jrtashjian added this to the Next Release milestone Feb 22, 2020
@richtabor
Copy link
Contributor

We'll likely need to break this down into phases, the first being removing what we can more easily remove/migrate from. The second being the bigger moves - such as the lightbox/slider functionalities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Priority] High This issue/pull request needs resolving before the next release [Type] Enhancement Something new that adds functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants