Support from Acquia helps fund testing for Drupal Acquia logo

Comments

droplet created an issue. See original summary.

nod_’s picture

Status: Needs review » Reviewed & tested by the community

They added filtering to their for loop, no code change in the tests we're using.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, Modernizr-3.2.patch, failed testing.

droplet’s picture

Status: Needs work » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, Modernizr-3.2.patch, failed testing.

droplet’s picture

Status: Needs work » Reviewed & tested by the community
xjm’s picture

Version: 8.0.x-dev » 8.1.x-dev

Thanks for the update!

I believe since this is a minor version library update, it should be targeted for 8.1.x now as per https://www.drupal.org/core/d8-allowed-changes#minor.

alexpott’s picture

Status: Reviewed & tested by the community » Needs review

I downloaded the library from Modernizr again and did a diff to the patch and it wasn't the same... not sure what is going on.

droplet’s picture

Status: Needs review » Needs work

I don't hijack it... [ just a joke :) ]

It's interesting. their builder using GIT master.

https://github.com/Modernizr/Modernizr/commit/695fd5ef3cad5b867785d5cff4...

droplet’s picture

lyalyuk’s picture

Title: Update JS lib: Modernizr to 3.2 » Update JS lib: Modernizr to 3.3.1
Status: Needs work » Needs review
FileSize
9.42 KB

Modernizr setClasses is required in 3.3.0 and higher.
Need review

andypost’s picture

Issue tags: +SprintWeekend2016
Trebor’s picture

Status: Needs review » Reviewed & tested by the community

Per #11, downloaded patch and applied it successfully in D8.0.3-dev, version is now 3.3.1 for Modernzr (modernizr.min.js), first line in modernizr.min.js reads /*! modernizr 3.3.1 (Custom Build) | MIT *

catch’s picture

Status: Reviewed & tested by the community » Needs work

This needs a libraries.yml change.

Trebor’s picture

Version: 8.1.x-dev » 8.0.x-dev

Agreed, in the /core/core.libraries.yml file, line 824 still reads the old version: version: "v3.1.0"

miteshmap’s picture

Status: Needs work » Needs review
FileSize
410 bytes
9.6 KB

Updated based on comments from @Trebor and @catch

miteshmap’s picture

Sorry! wrong patch... Updated new one..

Trebor’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed #16 (patch file update_js_lib-2614682-16.patch) and this one correctly updates the /core/core.libraries.yml file modernizr version to 3.3.1.

Note that #17 seems to be another patch to update the actual modernizr.min.js file. Some confusion here perhaps?

  • catch committed 33ba34e on 8.1.x
    Issue #2614682 by miteshmap, droplet, lyalyuk: Update JS lib: Modernizr...
catch’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +8.1.0 release notes

@Trebor #17 is the right patch - has both the library update and the libraries.yml change together.

Committed/pushed to 8.1.x, thanks! I don't think we should do this in 8.0.x unless there's a specific bug fix we need - please re-open if you think we should commit it to 8.0.x too.

Tagging for a release notes mention.

Trebor’s picture

Thanks @catch for the clarification on the #17 patch.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.