WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#29833 closed enhancement (fixed)

Update to jQuery UI 1.11.2

Reported by: Fab1en Owned by:
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.1
Component: External Libraries Keywords:
Focuses: javascript Cc:
PR Number:

Description

The next version of jQuery UI is out since last august : the release post is here. That would be nice to bring it back into core. Change logs : 1.11.0, 1.11.1

I'm specifically interested by the following change :

  • Fixed: Enabled draggable from within iframe

This would allow to use jQuery UI to drag elements in the customizer preview iframe or in a TinyMCE view iframe.

Attachments (6)

29833.patch (733.5 KB) - added by Fab1en 5 years ago.
new files for jQuery UI 1.11.1
29833.2.patch (498.2 KB) - added by Fab1en 5 years ago.
update files to jQuery UI 1.11.1 without renaming them
29833.grunt-task.patch (1009 bytes) - added by ocean90 5 years ago.
29833.grunt-task.2.patch (1.3 KB) - added by ocean90 5 years ago.
Don't ship non-minified jUI files
29833.3.patch (945.7 KB) - added by ocean90 5 years ago.
29833.4.patch (71.5 KB) - added by ocean90 5 years ago.
jQuery UI 1.11.2

Change History (23)

#1 @wonderboymusic
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.1

@helen or @ocean90 - whoever knows how to update this, someone please grab this

#2 @georgestephanis
5 years ago

And a recurring friendly reminder to merge #26843 as well once this lands.

@Fab1en
5 years ago

new files for jQuery UI 1.11.1

#3 @Fab1en
5 years ago

  • Keywords has-patch added; needs-patch removed

29833.patch drops new files in. I downloaded the files from the official github repo, and minify them using Grunt uglify task.

I changed the names from wp-includes/js/jquery/ui/jquery.ui.*.min.js to wp-includes/js/jquery/ui/*.min.js because they were renamed like that in the 1.11.1 release.

They say that

This makes usage with AMD easier (see below) and makes it easier to deal with the individual source files.

I also changed dependencies between modules in order to have the minimum dependency list.

#4 @ocean90
5 years ago

  • Focuses administration removed
  • Keywords needs-patch added; has-patch removed

Yeah, so they changed the build process which doesn't provide separated minified files anymore. :(

@Fab1en Can you please create a patch without renaming the files?

Also for the record: 1.11.x drops support for IE 7 and Opera 12. They will leave the workarounds in 1.11.x, but will drop them in 1.12.x.

@Fab1en
5 years ago

update files to jQuery UI 1.11.1 without renaming them

#5 @Fab1en
5 years ago

  • Keywords has-patch added; needs-patch removed

@ocean90 : I updated the patch.

#6 @ocean90
5 years ago

  • Keywords dev-feedback added

Thanks Fabien.

So we now have 3 options:

1) Use jQuery UI without AMD definitions
2) Don't rename current files and update AMD definitions
3) Rename current files, and use the files as they are

Beside this, now and in future we have to the fetch the files from the GitHub repo and minify them by ourselves. Another idea would be to bundle the sources and update our grunt task.

#7 @Fab1en
5 years ago

I vote for option 3 (so version 1 of my 29833.patch): I cannot think about any drawbacks to rename the files.
Other options involve complex files manipulations (remove or update part of the source code) without evident benefits.

I also vote for bundling the sources (and the source for jquery as well) as this allows easier js debug. I can propose a new patch for this solution if you want. Or is it a new task (new ticket) ?

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


5 years ago

@ocean90
5 years ago

Don't ship non-minified jUI files

@ocean90
5 years ago

#9 @ocean90
5 years ago

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

In 29847:

Update jQuery UI to 1.11.1.

Because jQUI's build process no longer provides individual minified files we need some additional changes:

  • Rename all files, remove the "jquery.ui." prefix. Add old files to $_old_files.
  • Add and use non-minified files in /src.
  • Add grunt task to minify jQuery UI files.
  • (Non-minified files will not be shipped.)

Changelogs:

props Fab1en, ocean90.
fixes #29833.

#10 follow-up: @azaozz
5 years ago

  • Keywords needs-unit-tests added; has-patch dev-feedback removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

We've always included the "official" builds of external libraries. Not happy at all that we have to rebuild jQuery UI and bypass the official build on every update. This method is closest to our current setup but might introduce problems if/when the upstream build process changes. Even using different versions of the build tools might be problematic at some point.

It may be possible to split the official build in four parts/files: UI Core, Interactions, Widgets, Effects. The same way as the sections on the download page. However not sure this will be better.

As we will be rebuilding UI, we loose the "real life" testing of the builds (i.e. in production we will be running a slightly different code than everybody else). In that terms it is imperative that we adapt and implement the UI's automated tests: https://github.com/jquery/jquery-ui/tree/master/tests/unit.

Last edited 5 years ago by azaozz (previous) (diff)

#11 in reply to: ↑ 10 @ocean90
5 years ago

Replying to azaozz:

We've always included the "official" builds of external libraries. Not happy at all that we have to rebuild jQuery UI and bypass the official build on every update.

That's right. I wasn't happy about it too. But as mentioned above and in linked IRC chat with Scott there will be no official build for individual files anymore.

It may be possible to split the official build in four parts/files: UI Core, Interactions, Widgets, Effects.

Sure, but that means we would blow up our scripts. For example on Dashboard we only need 4 files, when we split it up we would have 9 files in 2 files.

As we will be rebuilding UI, we loose the "real life" testing of the builds (i.e. in production we will be running a slightly different code than everybody else). In that terms it is imperative that we adapt and implement the UI's automated tests: https://github.com/jquery/jquery-ui/tree/master/tests/unit.

I'm fine with adding the unit tests too. Is it the same reason why we have the TinyMCE tests?

#12 @nacin
5 years ago

The TinyMCE tests are nice especially because we hack the hell out of TinyMCE with plugins and such, and because ContentEditable is such a mess.

Committing UI's tests is going to greatly increase our burden in terms of maintaining those tests, while not actually giving us any real benefit. We use an identical build process as the jQuery project. I signed off on this as did the development lead for jQuery UI.

I'd be down for some way to run jQuery UI's tests on uglify'd jQuery UI, and then automate that, but I'd be weary of doing it if it makes it particularly hard for us to maintain it going forward.

@ocean90
5 years ago

jQuery UI 1.11.2

#13 @ocean90
5 years ago

  • Summary changed from Update to jQuery UI 1.11.1 to Update to jQuery UI 1.11.2

#14 @ocean90
5 years ago

In 29920:

Update jQuery UI to 1.11.2.

Changelog: http://jqueryui.com/changelog/1.11.2/

see #29833.

#15 @ocean90
5 years ago

azaozz, what's you opinion on nacin's comment:12?

#16 @azaozz
5 years ago

  • Keywords close added; needs-unit-tests removed

Agree that it's going to be hard to maintain UI's tests, quite harder than updating UI. Will have to do more testing "by hand" on every update :)

#17 @ocean90
5 years ago

  • Keywords close removed
  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.