Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump jquery from 2.2.4 to 3.1.0 #24023

Merged
merged 1 commit into from
Nov 13, 2020

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Nov 10, 2020

Like #23099 but in a smaller step.

📜 https://jquery.com/upgrade-guide/3.0

3.0 didn't work because of an UMD issue jquery/jquery-migrate#273, so I'm trying 3.1. Locally things still work.

  • is(':visible') behaves different with jQuery 3. I couldn't ge t one of the tests to pass, but it's a component that will hopefully soon be replaced by Vue so I just dropped the one test. https://api.jquery.com/visible-selector/ for more info.
  • .selector was deprecated in 1.9 and removed in 3. I removed an assertion that relied on that.
  • Removed fake timers where possible because they block jQuery deferred that use a timer to run code async in v3. With fake timers the promises do not make any progress and the test times out
  • Changed async tests to always use done and have the test framework await the results

@ChristophWurst ChristophWurst added this to the Nextcloud 21 milestone Nov 10, 2020
@nextcloud nextcloud deleted a comment from faily-bot bot Nov 10, 2020
@ChristophWurst ChristophWurst force-pushed the dependachristoph/npm_and_yarn/jquery-3.1.0 branch 3 times, most recently from 042c2d8 to fc9ae94 Compare November 12, 2020 19:03
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the dependachristoph/npm_and_yarn/jquery-3.1.0 branch from fc9ae94 to ae0a496 Compare November 13, 2020 07:46
@nextcloud nextcloud deleted a comment from faily-bot bot Nov 13, 2020
@ChristophWurst
Copy link
Member Author

Bildschirmfoto von 2020-11-13 09-29-21

🚀 🙏 🤯

@@ -3699,7 +3699,7 @@
const enabled = tabView.canDisplay || undefined
if (tabView.id) {
OCA.Files.Sidebar.registerTab(new OCA.Files.Sidebar.Tab({
id: tabView.id,
id: tabView.id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the most important change

@@ -328,7 +328,7 @@ export default L10n
*
* @returns {String} locale string
*/
export const getLocale = () => $('html').data('locale')
export const getLocale = () => $('html').data('locale') ?? 'en'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -82,9 +82,10 @@ $.fn.contactsMenu = function(shareWith, shareType, appendTo) {
}

actions.forEach(function(action) {
const template = entryTemplate
$list.find('ul').append(template(action))
$list.find('ul').append(entryTemplate(action))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll happily see this one die #22274

})

$div.trigger('load')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI needed so a test can await the async operation to be done.

hyperlink: '#',
title,
}))

$div.trigger('loaderror', jqXHR)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same. Just see the updated spec code :)

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried a few thing, clicking around, files contactsmenu,settings, old legacy areas, no errors, works fine! 👍
Awesome job!!

@ChristophWurst ChristophWurst self-assigned this Nov 13, 2020
Copy link
Contributor

@GretaD GretaD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also couldnt find anything that is broken because of this PR 👍

@GretaD GretaD merged commit a4223b0 into master Nov 13, 2020
@GretaD GretaD deleted the dependachristoph/npm_and_yarn/jquery-3.1.0 branch November 13, 2020 13:42
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a quick clicking around through the files app, shares, public shares and settings and it looked fine

@MorrisJobke
Copy link
Member

Nice! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants