Shortcodes: minify jQuery Cycle script #6024

Merged
merged 3 commits into from Jan 3, 2017
@eliorivero
Contributor
eliorivero commented Dec 31, 2016 edited

I noticed this on my new site elio.blog and searched for an issue here and there's this one that this PR now fixes #3335. I know that #5620 exists but I'm not sure if we need to minify these kind of files on each build since we won't be changing them often. So, here's this PR that specifically solves #3335.

Also, the reason why I noticed this script was because after running a test in the site I saw the script loaded, and I wasn't using it at all. I realized that when Infinite Scroll is supported, this script and others are always loaded. However, in my tests this wasn't true. I tested with this same branch and whenever the post that had a slideshow gallery was loaded, the script was loaded, so it's not necessary to always load it.

Changes proposed in this Pull Request:

  • use minified jQuery Cycle JS
  • don't load the script always when Infinite Scroll is activated

Testing instructions:

  • add a slideshow gallery and verify it works normally
  • test Infinite Scroll and add a slideshow in a post that is displayed after IS fetches more entries and verify it works

Proposed changelog entry for your changes:

Shortcodes: the jQuery Cycle script used by slideshow galleries is now minified resulting in faster loading times.

cc @lancewillett for review

@eliorivero eliorivero added this to the 4.5 milestone Dec 31, 2016
@eliorivero eliorivero self-assigned this Dec 31, 2016
@eliorivero eliorivero requested a review from lancewillett Dec 31, 2016
@lancewillett

I noticed the related test isn't active. Want to re-enable it?

modules/shortcodes/slideshow.php
-
- if ( $enqueued ) {
- return;
- }
wp_enqueue_script( 'jquery-cycle', plugins_url( '/js/jquery.cycle.js', __FILE__ ), array( 'jquery' ), '2.9999.8', true );
@lancewillett
lancewillett Jan 3, 2017 Member

Do we need to bump the version number here to bust cache?

@lancewillett
Member

Tested locally with Jetpack dev environment, works well. Going to give it a spin on WP.com test site also.

@lancewillett

Should the file be renamed to jquery.cycle.min.js instead?

@lancewillett
Member
lancewillett commented Jan 3, 2017 edited

Tested successfully on WP.com. 👍 (I renamed the file there to add in min, though.)

Related WP.com code change (pending review): D3841-code

@eliorivero
Contributor

@lancewillett I have:

  • renamed file to jquery.cycle.min
  • updated version to bust cache
  • re-enabled the tests and modify the slideshow.php file to pass tests
@lancewillett
Member

@eliorivero Tests out both Jetpack local and on WP.com, looks great.

@dereksmart
Member

Nice, LGTM -- Thanks @lancewillett for testing on wpcom!

@dereksmart dereksmart merged commit 80d82ca into master Jan 3, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dereksmart
Member

merged to 4.5 fb7e160

@dereksmart dereksmart deleted the clean/minify-jquery-cycle branch Jan 3, 2017
@jeherve jeherve added a commit that referenced this pull request Jan 9, 2017
@jeherve jeherve Changelog: add #6024 da35d9c
@jeherve jeherve added a commit that referenced this pull request Jan 12, 2017
@jeherve jeherve Changelog: add #6024 2edd7c4
@dereksmart dereksmart added a commit that referenced this pull request Jan 17, 2017
@jeherve @dereksmart jeherve + dereksmart Changelog: add 4.4.2 release post link.
CHangelog: add #5457

Changelog: add #5487

Changelog: add #5708

Changelog: add #5879

Changelog: add #5932

Changelog: add #5963

Changelog: add #5968

Changelog: add #5996

Changelog: add #5998

Changelog: add #5999

Changelog: add #6012

Changelog: add #6013

Changelog: add #6014

Changelog: add #6015

Changelog: add #6023

Changelog: add #6024

Changelog: add #6030

Changelog: add #5465

CHangelog: add #6063

Changelog: add #6025

Changelog: add #5974

Changelog: add #6059

Changelog: add #6046

Changelog: add #5418

Changelog: move things around and add missing information.

Changelog: add #5565

Changelog: add #6087

Changelog: add #6095
711efa1
@dereksmart dereksmart added a commit that referenced this pull request Jan 17, 2017
@jeherve @dereksmart jeherve + dereksmart Changelog: add #5866
Changelog: add #5867

Changelog: add #5874

Changelog: add #5905

Changelog: add #5906

Changelog: add #5931

Changelog: add #5933

Changelog: add #5934

Bring over 4.4.2 changelog from branch-4.4

@see 18012a3

Changelog: add #5976, #5978, #5983

Changelog: add #5917

Changelog: add #5832

Changelog: add 4.4.2 release post link.

CHangelog: add #5457

Changelog: add #5487

Changelog: add #5708

Changelog: add #5879

Changelog: add #5932

Changelog: add #5963

Changelog: add #5968

Changelog: add #5996

Changelog: add #5998

Changelog: add #5999

Changelog: add #6012

Changelog: add #6013

Changelog: add #6014

Changelog: add #6015

Changelog: add #6023

Changelog: add #6024

Changelog: add #6030

Changelog: add #5465

CHangelog: add #6063

Changelog: add #6025

Changelog: add #5974

Changelog: add #6059

Changelog: add #6046

Changelog: add #5418

Changelog: move things around and add missing information.

Changelog: add #5565

Changelog: add #6087

Changelog: add #6095

Readme: add @tyxla to the list of contributors.

Improved changelog for your readability and enjoyment

updated the release date

finalizing the changelog with a few more edits
10bc6c3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment