WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 6 weeks ago

#25351 closed enhancement (fixed)

Updating the 'jquery-masonry' to a latest version

Reported by: rohitink Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.6.1
Component: External Libraries Keywords: needs-testing has-patch
Focuses: Cc:

Description

Wordpress currently has the version 2.1.05 of the jquery-masonry bundled with the core. This version of the script is currently outdated, and not very useful.

The latest version of the Script is 3.1.2 which is a great enhancement, and has more features as compared to the WordPress core bundled version.

Latest Version: http://masonry.desandro.com/masonry.pkgd.min.js

So, I request that script be updated to the latest version.

Attachments (7)

masonry.pkgd.min.js (23.9 KB) - added by rohitink 7 months ago.
Latest Version of the jQuery Masonry
masonry_patch.diff (30.5 KB) - added by rohitink 7 months ago.
Required Changes
25351.diff (32.0 KB) - added by shelob9 5 months ago.
Update Masonry to 3.1.2 and fix 2013&2014 footer widgets
25351.2.diff (32.2 KB) - added by shelob9 5 months ago.
Improved version of my previous diff 25351.diff
25351.3.diff (34.0 KB) - added by shelob9 5 months ago.
Updates Masonry to version 3 with a shim for backwards compatibility.
25351.4.diff (796 bytes) - added by kovshenin 6 weeks ago.
25351.5.diff (2.8 KB) - added by kovshenin 6 weeks ago.

Download all attachments as: .zip

Change History (29)

rohitink7 months ago

Latest Version of the jQuery Masonry

comment:1 follow-up: helen7 months ago

Seems that v3 introduced breaking (not backwards compatible) changes?

comment:2 in reply to: ↑ 1 nacin7 months ago

Replying to helen:

Seems that v3 introduced breaking (not backwards compatible) changes?

Correct. Grumble.

WordPress should introduce a wp.masonry wrapper (or something to that effect) that can attempt to offer backwards compatibility for future breaking changes. We might even be able to hijack the Masonry object and make it an instance of our object. It should be fairly easy to ensure back compat, all that changed were some options: http://masonry.desandro.com/appendix.html#upgrading-from-v2.

Another alternative is forking it and making our own backwards compatible build. I haven't asked the maintainer yet if he would be willing to help support such a build, but I doubt it.

The final alternative is just upgrading it and ignoring the back compat issues. We will have to update core for isRTL => isOriginLeft, but I don't think that would affect any others. I would suggest, as I did above, we build our own wrapper for future plugin and theme use. We could also ask the maintainer if he plans for version 4 to contain so many backwards incompatible changes, and whether there is anything we could do to prepare.

rohitink7 months ago

Required Changes

comment:3 rohitink7 months ago

Yes, the changes are too many. This does indeed create the problem of backwards compatibility. Since, the changes are too many, so it wouldn't be efficient to create a custom build. As newer themes/plugins would prefer to use the latest version of this script.

But, to offer compatibility to older themes/plugins, we can include the 2 versions, and give access to the older(2.1.05) version by default. And the developers who would like to use the latest version, should use something like 'jquery-masonry3' in the wp_enqueue_script('jquery-masonry3') function call.

This update is important, as it has been backed by a lot of theme reviewers at our mailing list.

And we can remove the older version in any of the coming updates.

Last edited 7 months ago by rohitink (previous) (diff)

comment:4 shelob97 months ago

If there was a different handle for Masonry 3 than Masonry 2, as @rohitink wisely suggests, wouldn't that make it as easy as having core deregister masonry3 if ( is_admin() ) just in case theme/plugin developer didn't only register Masonry 3 if ( ! is_admin() )?

comment:5 shelob97 months ago

  • Cc shelob9 added

comment:6 jjeaton5 months ago

  • Cc jjeaton added

shelob95 months ago

Update Masonry to 3.1.2 and fix 2013&2014 footer widgets

comment:7 shelob95 months ago

I have uploaded a new patch that upgrades Masonry to the latest version and fixes the issues this caused in the footer widget areas in TwentyThirteen and TwentyFourteen, which use Masonry for layout.

According to my detective work, which may be incomplete, the only places in core that use Masonry are the custom handler admin page, and the two places mentioned above. I tested the custom header admin page after uploading a variety of custom headers of different sizes and they rearranged just as they had before without making any changes to the Masonry JS in wp-admin/js/custom-header.js.

Upgrading Masonry did break the footer widget areas in TwentyThirteen and TwentyFourteen do to the way that Masonry 3 handles column widths differently then version 2. Please see the diff for how I handled this and if you are better with javascript then I am please suggest or make improvements to my changes.

If anyone knows of or finds a place that core uses Masonry that I missed, please comment and I will look into it.

comment:8 rohitink5 months ago

I reviewed the changes you made. The Twnety Thirteen theme seems to be running Perfectly Fine. However, There is one change you still need to make in wp-content/themes/twentythirteen/js/functions.js on line no. 77.

The Parameter: gutterWidth is no longer valid. It has been renamed to gutter

http://masonry.desandro.com/appendix.html#upgrading-from-v2

Last edited 5 months ago by rohitink (previous) (diff)

comment:9 rohitink5 months ago

You Also need to change, the following:

isRTL to isOriginLeft: false
isResizable to isResizeBound

Reference: http://masonry.desandro.com/appendix.html#upgrading-from-v2

I think that's it. Couldn't find any more issues.

shelob95 months ago

Improved version of my previous diff 25351.diff

comment:10 shelob95 months ago

I made the changes suggested by @rohitink and tested with RTL tester that everything worked fine in RTL mode.

comment:11 shelob95 months ago

  • Keywords needs-testing added

comment:12 shelob95 months ago

David DeSandro wrote a shim in response to an issue I posted in the Github repo for Masonry:)

https://github.com/desandro/masonry/blob/master/examples/js/masonry-v2-shim.js

I'm not sure what the best way to implement this is and would appreciate guidance or a patch that makes this work.

shelob95 months ago

Updates Masonry to version 3 with a shim for backwards compatibility.

comment:13 shelob95 months ago

This new patch 25351.3.diff​ updates to Masonry 3. I modified wp-includes/script-loader.php using a strategy suggested by Otto in the theme review team discussion list. The handle 'jquery-masonry' now enqueues the shim and has a dependency of 'jquery-masonry3' which is the file with the actual Masonry v3 code.

This patch does not modify js/functions.js in TwentyThirteen or TwentyFourteen. But, thanks to the backwards compatibility shim they're footer widgets work just fine:) If this patch is accepted then someone who has better Javascript skills should update the relevant code to Masonry 3 syntax, though that is not urgent, since they will work as-is with this patch.

comment:14 ircbot3 months ago

This ticket was mentioned in IRC in #wordpress-dev by shelob9. View the logs.

comment:15 helen7 weeks ago

  • Milestone changed from Awaiting Review to 3.9

For review.

comment:16 nacin7 weeks ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 27271:

Update the Masonry JavaScript library to version 3.

The new script handle is 'masonry'. The old 'jquery-masonry' handle is the official shiv that sits on top of the v3 library. While v3 no longer depends on jQuery, a theme or plugin may have bee
n implicitly loading jQuery though Masonry, rather than additionally declaring it as a dependency for themselves. Thus, the shiv is separate.

Themes should switch to 'masonry' and declare jQuery as a dependency on their own if they need it. Upgrade guide: http://masonry.desandro.com/appendix.html#upgrading-from-v2. imagesLoaded remai
ns included with Masonry here.

props shelob9.
fixes #25351.

comment:17 bpetty7 weeks ago

  • Keywords has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Just wanted to re-open this to address the issue of broken jshint tests that this commit introduced.

See: https://travis-ci.org/tierra/wordpress/jobs/19603866

Also related: #26446

comment:18 nacin7 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 27314:

JSHint: Generically blacklist all minified files. fixes #25351.

comment:19 nacin7 weeks ago

If it wasn't clear from the commit message for [27271], after studying this I realized Masonry dropped the jQuery dependency, which meant the juggling that shelob9 did in the latest patch could be taken a step further. In the end, it's a bit cleaner and it provides a solid path for themes and plugins.

I also decided to include imagesLoaded directly in masonry.min.js, at least for now. They happen to be separate libraries upstream, but it's tough to use Masonry without it. In the future we could always spin it off into its own file.

kovshenin6 weeks ago

comment:20 kovshenin6 weeks ago

  • Keywords has-patch added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Looks like r27271 assumes the existence of jquery.masonry.min.js, but we never added our new shim to Grunt. See 25351.4.diff. Using static mapping, otherwise Grunt will treat ".masonry.js" as an extension.

kovshenin6 weeks ago

comment:21 nacin6 weeks ago

After some discussion (and a confirmation from the library's author that there isn't an official minified build of this file), let's commit our own uglified file to save ourselves the trouble.

comment:22 nacin6 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 27389:

Add jquery.masonry.min.js. fixes #25351.

Note: See TracTickets for help on using tickets.