-
Notifications
You must be signed in to change notification settings - Fork 7.6k
FOR REVIEW: Update to jQuery 2.0.1, LESS 1.3.3 and Bootstrap 2.3.1 #4054
Conversation
explicitly set margin-top to 0
I added this because somehow the relative path of the WebPlatform logo isn't found.
+ one UI fix
Conflicts: src/htmlContent/about-dialog.html src/htmlContent/project-settings-dialog.html src/htmlContent/update-dialog.html src/styles/brackets.less
Conflicts: src/thirdparty/jstree_pre1.0_fix_1/jquery.jstree.js
Required an update to jQuery Mockjax 1.5.2, and quoting in a selector string used in one of the tests.
When I run All of the extension tests, I see a failure in JavaScript Code Hinting that I don't see when I run the JS Code Hinting test on its own. Do you see that as well? I have a feeling that's independent of the work we're tracking here. |
As documented in the [jQuery 1.9 upgrade guide](http://jquery.com/upgrade-guide/1.9/#attr-versus-prop-), the recommended way to update input values is via .val() not .attr().
It turns out that the InlineColorEditor failure was actually a case of a jQuery change that required a fix in the code. |
Per the jQuery 1.9 upgrade guide.
@@ -771,7 +771,7 @@ define(function (require, exports, module) { | |||
* Closes all menus that are open | |||
*/ | |||
function closeAll() { | |||
$(".dropdown").removeClass("open"); | |||
$(".dropdown").removeClass("open").dropdown("toggle"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for doing both of these things? It seems like this does some redundant work, since .dropdown("toggle")
calls clearMenus()
, which removes the open
class from all dropdown parents. Conversely, since we know for sure we want to remove that class (as opposed to toggling an open dropdown), it seems like just clearing the open
class should be good enough (it doesn't look like Dropdown.toggle()
does much more than that).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dropdown("toggle")
call doesn't seem necessary. I removed it (along with the calls on line 944 and 953) and everything worked fine.
@njx - thanks for the review. Changes pushed, and ready for re-review. |
} else if ( $.browser.opera ) { | ||
transitionEnd = "oTransitionEnd" | ||
} | ||
transitionEnd = $.support.transition.end; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand how this change works. It looks like the previous block of code sets $.support.transition
to a simple boolean, so it seems like $.support.transition.end
would throw an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old code set transitionEnd
to a string, and so does the new code. (the $.browser
property was removed, and the $.support.transition.end
property is new)
Review complete. The remaining issues I think we should consider addressing before landing the branch are:
There are a few other minor visual issues I'm seeing that I'll go ahead and file with [framework-update] in the subject. |
Actually, I think we can land this before the in-browser menu work. I'll file a bug on that. |
I added notes to the bottom of https://github.com/adobe/brackets/wiki/Research:-jQuery-2.0-Upgrade about the changes we made for jQuery 1.9. @dangoor - do you think there's anything else in the 1.9 upgrade guide that's worth specifically warning people about (so they don't have to read all the various obscure things they probably won't care about)? |
Looks good aside from the filed bugs. Merging. |
FOR REVIEW: Update to jQuery 2.0.1, LESS 1.3.3 and Bootstrap 2.3.1
This pull requests updates jQuery to 2.0.1, LESS to 1.3.3 and Bootstrap to 2.3.1. This is a major update that has several breaking changes.
Major API changes are listed in the Sprint 26 release notes
Big thanks to @WebsiteDeveloper for the LESS and Bootstrap updates, and @TuckerWhitehouse for the jQuery update.
Remaining tasks: