Fix for jqueryUI 1.12 #73
Conversation
|
There are a few issues the need to be resolved before we can merge:
There seems to be an issue with the Travis-CI build in general. I'll fix it when I can find time to look at it. |
|
Will Check No idea how to reduce repetitions withiout loosing bckward comp. If You have any idea.. |
|
According to jQuery UI 1.12 Upgrade Guide:
However you can disable backwards compatibility by: <script src="jquery.js"></script>
<script>$.uiBackCompat = false;</script>
<script src="jquery-ui.js"></script>It seems they want to remove backwards compatibility in jQuery UI 1.13 so unfortunately I still think it's worth supporting both APIs. The idea I have is to define the options once according to the new API and always pass them through a jQuery plugin we create which converts the options to the old API if necessary. Example: var _isNewApi = $.ui.version >= '1.12';
$.fn.monthPickerButton = function(opts) {
if (_isNewApi) {
return this.jqueryUIButton(opts);
} else {
var newOpts = {};
if (opts.showLabel !== void 0) newOpts.text = opts.showLabel;
// etc...
return this.jqueryUIButton(newOpts);
}
};
this._prevButton.monthPickerButton({
showLabel: false
}); |
|
I reworked the parts, fixed all bugs. Hopefully implementation is better now ;) Learned a bit, never used functions for These type of work. Thanks a lot |
|
The @wallenium Please revert the changes to use the source files in the demo and read the Contributing section of the project's Wiki. |
|
@wallenium It looks like the tests are filming please fix the test so we can merge the pull request. |
|
@wallenium Please reach out if you need help fixing the tests. We can also send you an invite to join our slack room if you want. |
|
After pulling your branch and adjusting the test environment to use jQuery UI 1.12 it looks like there are a lot of visual glitches: @wallenium Please run the tests from master to see how the plugin should look |
This reverts commit 87cbe7c.
|
Gott possible to run the test on my system, not sure why it fails: PhantomJS threw an error:ERROR
latest its version available. Should work without a problem.. Checked the contribute page. |
|
I am also running into this issue on my work machine. You will have to run the tests from the browser until I can get around to fixing that issue. |
|
@wallenium Don't forget to adjust the |
|
Some of the failing tests seems to be because of internal problems. Not sure why done() fails... RTL is working, the test needs to be adjusted because of the missing span, not sure why the test result shows wrong dates with strange numbers while the manual tests on the same page are working fine.. only strange things seems to be the click behavior and the disable of prev/next button if limit is reached.. |
|
@wallenium What do you mean by internal problems? Can you be more specific? Which test are you having a problem fixing? |
| @@ -35,11 +35,12 @@ along with this program. If not, see | |||
| // conflict with Bootstrap.js button (#35) | |||
| $.widget.bridge('jqueryUIButton', $.ui.button); | |||
|
|
|||
| var _isNewApi = $.ui.version >= '1.12'; | |||
There was a problem hiding this comment.
The tests are paling because this tests is incorrect.
The unit tests are using version 1.9.2 of jQuery UI and this test returns true because the number 1.9 is greater than 1.12.
I pushed a fix for this to my jquery_1.12_support branch.
|
When I finish writing support for jQuery 1.12 I will open a pull request which will include @wallenium commits. I think once that PR gets merged this PR will be considered merged as well so I'm leaving it open for now. @KidSysco Do you think we should keep this PR open unit we merge my PR or we should close this PR? |
|
Sorry, was a bit busy the last weeks. Hopefully i have the time this week to finalize this issue. Or should i wait for you, @benjamin-albert |
|
@wallenium I think before we implement support for both versions of the jQuery UI APIs in a single file I want to change our test suits to:
After I adjust the test suite I'd be happy to help you write support for the new API or pick up writing support if you're not up for it. @KidSysco Can you please send @wallenium an invite to our Slack team. I think this would make collaborating with him easier. |
|
Sorry I did not reply until now. Springtime chores got me all busy. I am fine with leaving the PR open until it is ready to merge. It might result in a merge conflict but that should be OK. Either way, whatever is easiest for you guys. I would be happy to send @wallenium an invite to our slack channel. I will need an email address from you @wallenium in order to do that. The slack channel is nice for chatting. Let me know if there is anything else I can do for use guys! |
|
@wallenium I've pushed changes to my jquery_1.12_support branch which allow you to test both jQuery UI API versions. Just open @wallenium If you'd prefer that I write support for jQuery UI 1.12 then just say so, and I will be glad to do it whenever I can get around it (usually on the weekends). |
|
@wallenium I almost finished fixing the tests. I will start fixing the visual glitches when I can find time for it. (usually on the weekends). Take a look at my jquery_1.12_support branch. |
|
A bit of an update I've been busy trying to fix the visual glitches and I came to a conclusion that I could not fix them. Since then I've managed to figure out how to git in touch with the jQuery UI development team (which unfortunately isn't as straightforward as opening a GitHub issue) and I have been discussing the issues with @scottgonzalez. For details discussion: https://forum.jquery.com/topic/adjusting-the-jquery-ui-month-picker-plugin-for-jquery-ui-1-12 |
#72
Might not be the best implentation, but works.
Hint: I changed the JS file to the unminified Version, it is easier to debug. Needs to be altered once you tested the fix.
It should work with < 1.12 and > 1.12, i added a $ui.version check.