Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#37666 closed defect (bug) (fixed)

Masonry update to v3.3.2 breaks backwards compatibility with isAnimated option

Reported by: stephenharris's profile stephenharris Owned by: jorbin's profile jorbin
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.6
Component: External Libraries Keywords: has-patch commit
Focuses: Cc:

Description

In version 2 of Masonry, the isAnimated (boolean) option configured whether the grid elements were animated. Since WordPress switched to version 3 it has been providing a shim for backwards compatibility. However since #32802 use of that option throws an error:

jquery.masonry.js?ver=3.1.2:29 Uncaught TypeError: Cannot read property 'transitionDuration' of undefined

I'm not sure whether the last v2-shim works with Masonry 3.3.2. The shim was last updated Mar 26 2015 and moved to an external repo on April 1 2015 (where it appears to be unmaintained) which is when version Masonry 3.2.3 was released, and so it may not do so.

If the v2-shim no longer works, and we don't maintain it ourselves, we should probably make developers aware of this.

Attachments (3)

37666-masonry-shim.diff (530 bytes) - added by stephenharris 8 years ago.
37666.diff (577 bytes) - added by ocean90 8 years ago.
37666.2.diff (1.5 KB) - added by ocean90 8 years ago.

Download all attachments as: .zip

Change History (15)

#1 @ocean90
8 years ago

  • Keywords reporter-feedback added

@stephenharris Can you please provide some code to reproduce this issue?

#2 @stephenharris
8 years ago

Here's a demonstration of the bug: https://gist.github.com/stephenharris/5dd78b70dd81e082dc169ec4ba5c51c2

I have a patch: simply replace Masonry.prototype.options.transitionDuration with it's default value ('0.4s'). It's not clear to me why Masonry.prototype.options is no longer defined, but it would in any case, only store the default values.

Patch to follow.

#3 @stephenharris
8 years ago

(I had tested the latest version of the shim but it did not fix the bug)

@ocean90
8 years ago

#4 @ocean90
8 years ago

  • Keywords has-patch added; reporter-feedback removed

Thanks @stephenharris!

Using this instead of Masonry.prototype seems to work too, see 37666.diff. I think the only difference is that it wouldn't fall back to 0.4s if you initialize Masonry with isAnimated and transitionDuration.

#5 @stephenharris
8 years ago

Great, I also tested that and it works.

This ticket was mentioned in Slack in #core by ocean90. View the logs.


8 years ago

@ocean90
8 years ago

#7 @ocean90
8 years ago

  • Milestone changed from Awaiting Review to 4.6

#8 @ocean90
8 years ago

  • Keywords commit added

37666.2.diff looks good for 4.6

#9 @jorbin
8 years ago

In 38261:

External Libraries: Update Masonry shim to prevent error using isAnimated option

The isAnimated option throws an error and causes Masonry to stop functioning. Masonry.prototype.options is no longer defined in 3.3.2, this.options is and does the same. Masonry was updated to 3.3.2 in [37891].

Reported upstream: https://github.com/desandro/masonry-v2-3-shim/pull/1

Props stephenharris, ocean90, azaozz for testing and second sign off.
See #37666, #32802.

#10 @jorbin
8 years ago

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

In 38262:

External Libraries: Update Masonry shim to prevent error using isAnimated option

Merges [38261] to the 4.6 branch.

The isAnimated option throws an error and causes Masonry to stop functioning. Masonry.prototype.options is no longer defined in 3.3.2, this.options is and does the same. Masonry was updated to 3.3.2 in [37891].

Reported upstream: https://github.com/desandro/masonry-v2-3-shim/pull/1

Props stephenharris, ocean90, azaozz for testing and second sign off.
Fixes #37666.
See #32802.

#11 @jorbin
8 years ago

In 38276:

External Libraries: Update Minified version of jquery.masonry.js

WordPress maintains the minified version of jquery.masonry.js since there is no official build, however it has been excluded from grunt's minification process. This adds a minification task to grunt, adds it to the precommit hook for JS, minifies the file, and bumps the version on jquery.masonry.min.js. The change to the non minified version was introduced in [38261].

Fixes #37720. See #37666.

#12 @swissspidy
8 years ago

In 38339:

External Libraries: Update minified version of jquery.masonry.js.

WordPress maintains the minified version of jquery.masonry.js since there is no official build, however it has been excluded from grunt's minification process. This adds a minification task to grunt, adds it to the precommit hook for JS, minifies the file, and bumps the version on jquery.masonry.min.js. The change to the non minified version was introduced in [38261].

Merge of [38276] and [38281] to the 4.6 branch.

Fixes #37720. See #37666.

Note: See TracTickets for help on using tickets.