Make WordPress Core

Opened 11 years ago

Closed 10 years ago

#25351 closed enhancement (fixed)

Updating the 'jquery-masonry' to a latest version

Reported by: rohitink's profile rohitink Owned by: nacin's profile 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 11 years ago.
Latest Version of the jQuery Masonry
masonry_patch.diff (30.5 KB) - added by rohitink 11 years ago.
Required Changes
25351.diff (32.0 KB) - added by shelob9 10 years ago.
Update Masonry to 3.1.2 and fix 2013&2014 footer widgets
25351.2.diff (32.2 KB) - added by shelob9 10 years ago.
Improved version of my previous diff 25351.diff
25351.3.diff (34.0 KB) - added by shelob9 10 years ago.
Updates Masonry to version 3 with a shim for backwards compatibility.
25351.4.diff (796 bytes) - added by kovshenin 10 years ago.
25351.5.diff (2.8 KB) - added by kovshenin 10 years ago.

Download all attachments as: .zip

Change History (29)

@rohitink
11 years ago

Latest Version of the jQuery Masonry

#1 follow-up: @helen
11 years ago

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

#2 in reply to: ↑ 1 @nacin
11 years 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.

@rohitink
11 years ago

Required Changes

#3 @rohitink
11 years 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 expicitly state the version.

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

Version 0, edited 11 years ago by rohitink (next)

#4 @shelob9
11 years 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() )?

#5 @shelob9
11 years ago

  • Cc shelob9 added

#6 @jjeaton
10 years ago

  • Cc jjeaton added

@shelob9
10 years ago

Update Masonry to 3.1.2 and fix 2013&2014 footer widgets

#7 @shelob9
10 years 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.

#8 @rohitink
10 years 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 10 years ago by rohitink (previous) (diff)

#9 @rohitink
10 years 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.

@shelob9
10 years ago

Improved version of my previous diff 25351.diff

#10 @shelob9
10 years ago

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

#11 @shelob9
10 years ago

  • Keywords needs-testing added

#12 @shelob9
10 years 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.

@shelob9
10 years ago

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

#13 @shelob9
10 years 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.

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


10 years ago

#15 @helen
10 years ago

  • Milestone changed from Awaiting Review to 3.9

For review.

#16 @nacin
10 years 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.

#17 @bpetty
10 years 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

#18 @nacin
10 years ago

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

In 27314:

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

#19 @nacin
10 years 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.

@kovshenin
10 years ago

#20 @kovshenin
10 years 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.

@kovshenin
10 years ago

#21 @nacin
10 years 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.

#22 @nacin
10 years 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.