gvas / knockout-jqueryui

Knockout bindings for the jQuery UI widgets.
http://gvas.github.com/knockout-jqueryui/
MIT License
103 stars 38 forks source link

knockout-jqueryui 2.2.3 not fully compatible with Jquery UI 1.12 (buttons break) #61

Open Hiswe opened 7 years ago

Hiswe commented 7 years ago

jquery-ui 1.12 introduce a new API for buttons and knockout-jqueryui is still based on the old one.

bago commented 7 years ago

Can you describe what is "not fully compatible"? An use case, a test case, error messages... a jsbin/jsfiddle to reproduce the issue...

Hiswe commented 7 years ago

@bago It's mostly some icon only buttons being converted to icon + text

What it should look like (gallery icon + cancel/confirm icons):

screen shot 2016-11-30 at 16 33 22

After the update: gallery is now textual, cancel/confirm show their label (the buttons overflow is not related to this of course):

screen shot 2016-11-30 at 16 31 38

For the test case I really don't know how to make a good one for now…

But as I have read from jquery-ui doc:

instead of (from button.js): this.options = ['disabled', 'icons', 'label', 'text']

I hope it helps a little bit ^^

bago commented 7 years ago

The jqueryui 1.12 upgrade guides says that the old options are "DEPRECATED" BUT they should works in a backward compatible way unless specifically disabled: https://jqueryui.com/upgrade-guide/1.12/#deprecated-support-for-checkbox-and-radio-types-in-favor-of-checkboxradio-widget

So, this sounds a bug in jQueryUI 1.12 :-(

Hiswe commented 7 years ago

I should come up with a pull request. Looking at the codebase this shouldn't take long. But It will break old jquery-ui versions support for this particular component

bago commented 7 years ago

I opened 2 issues against jQuery UI 1.12

https://bugs.jqueryui.com/ticket/15108 https://bugs.jqueryui.com/ticket/15109

Now, docs says that jQuery UI 1.12 should be backward compatible with 1.11, but it clearly is not so we probably should simply declare knockout-jqueryui compatible with jquery ui 1.11 and tell people to avoid using the buggy 1.12.

Also, 1.12 introduces new API that will be the only API available in 1.13. Using the "new API" doesn't only means altering the options array, but also changing the html markup.. so I don't know what is the best option. Maybe knockout-jqueryui should simply stick to 1.11.

Creating a knockout-jqueryui release compatible with the new API is "out of scope" for me, so unless anyone one else is ready to take the big issue, we'll probably have to work on the documentation side to let people know jQueryUI 1.12 issues and jQueryUI 1.13 breaking changes. I really don't like the jQuery idea to break API compatibility in a 1.12 to 1.13 release (well, they really broke it in 1.11 to 1.12 but this was not by purpose).

Hiswe commented 7 years ago

@bago Seems fair to stick to 1.11

And yes that's a good demonstration of how following semver convention could prevent those problems!

I won't close the issue… people may look in for informations :)

bago commented 7 years ago

I updated the bower declaration, the readme and the index page for the website so to include explicit reference to the jquery ui 1.8.x to 1.11.x as compatible.

Hiswe commented 7 years ago

@bago Thanks a lot!