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

Update jQuery to 3.4.1 in the legacy Back Office pages #15245

Merged
merged 10 commits into from Oct 9, 2019

Conversation

@matthieu-rolland
Copy link
Contributor

matthieu-rolland commented Aug 23, 2019

Questions Answers
Branch? develop
Description? Migrate jquery in back office (from 1.11 to 3.4.1)
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? fixes #14930
How to test? Check that nothing is broken in back office

This change is Reviewable

@matthieu-rolland matthieu-rolland requested a review from PrestaShop/prestashop-core-developers as a code owner Aug 23, 2019
@matthieu-rolland matthieu-rolland added WIP and removed WIP labels Aug 23, 2019
@matthieu-rolland

This comment has been minimized.

Copy link
Contributor Author

matthieu-rolland commented Aug 23, 2019

Note that I didn't delete our jquery 1.11.0 file because it is still used for the installation process.

@matthieu-rolland matthieu-rolland force-pushed the matthieu-rolland:jquery-migrate branch 3 times, most recently from 7860402 to 2af616f Aug 23, 2019
classes/Media.php Outdated Show resolved Hide resolved
Copy link
Member

eternoendless left a comment

I think the old jQuery and jQuery migrate should be removed as well shouldn't it?

@matthieu-rolland

This comment has been minimized.

Copy link
Contributor Author

matthieu-rolland commented Aug 26, 2019

@eternoendless I was going to delete it but the old jquery (1.11) is still used during the installation process...

@matthieu-rolland matthieu-rolland force-pushed the matthieu-rolland:jquery-migrate branch from 2af616f to 61e475f Aug 26, 2019
@mickaelandrieu

This comment has been minimized.

Copy link
Member

mickaelandrieu commented Aug 28, 2019

is it possible to also use jQuery 3 in the Front Office?

@Progi1984

This comment has been minimized.

Copy link
Contributor

Progi1984 commented Aug 29, 2019

@mickaelandrieu Hi, jQuery 3 will be updated on BO, Core/Classic Theme and UI Kit (EPIC #13829)

@matthieu-rolland matthieu-rolland force-pushed the matthieu-rolland:jquery-migrate branch 7 times, most recently from 75ec3ab to 1313961 Aug 29, 2019
@matthieu-rolland matthieu-rolland requested a review from eternoendless Sep 6, 2019
@matthieu-rolland matthieu-rolland force-pushed the matthieu-rolland:jquery-migrate branch from 1313961 to b40f601 Sep 6, 2019
@matthieu-rolland matthieu-rolland force-pushed the matthieu-rolland:jquery-migrate branch from a78960c to 7e95576 Sep 26, 2019
@matthieu-rolland matthieu-rolland dismissed eternoendless’s stale review Sep 27, 2019

suggestions have been done

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Sep 30, 2019

@matthieu-rolland it seems you wait for @jolelievre validation of the changes you added following his code review, is that correct ?

@matthieu-rolland

This comment has been minimized.

Copy link
Contributor Author

matthieu-rolland commented Sep 30, 2019

@matks true ! (up)

@jolelievre I deleted the empty addJquery methods in the controllers extending ControllerCore, the one in controllerCore has the deprecation notice.

@matthieu-rolland matthieu-rolland force-pushed the matthieu-rolland:jquery-migrate branch from 7b38ab8 to d159dbc Oct 2, 2019
@sarahdib sarahdib added QA ✔️ and removed waiting for QA labels Oct 9, 2019
@sarahdib sarahdib added this to the 1.7.7.0 milestone Oct 9, 2019
@matthieu-rolland matthieu-rolland merged commit d7eb44f into PrestaShop:develop Oct 9, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
@zuk3975

This comment has been minimized.

Copy link
Contributor

zuk3975 commented Oct 9, 2019

@matthieu-rolland, @matks - after this pr legacy order create page is not working well.

  • Cannot choose customer;
  • Carts/orders are shown before even choosing customer.

Screenshot from 2019-10-09 14-18-47
Checked locally, that undoing this pr changes brings back the expected behavior.

@matthieu-rolland

This comment has been minimized.

Copy link
Contributor Author

matthieu-rolland commented Oct 9, 2019

@zuk3975 The goal of this PR is to keep legacy jquery running smoothly, but for some reason jquery-migrate doesn't fix all the holes 🤔 (especially features that were already deprecated before our former version of jquery).

So from now on we should use jQuery's latest API only.

The live method doesn't exist anymore, one should use the on method instead.

Here is the on method:
https://api.jquery.com/on/

Here is how to use the on method to achieve the same thing as with the live method:
$( document ).on( events, selector, data, handler );

The equivalent with the live method would be:
$( selector ).live( events, data, handler );

The main difference is that you put a parent selector in $(), and the target selector as the second parameter of the on method. If you just put "document" in $()it will be costlier that if you use a closer parent selector.

@matthieu-rolland

This comment has been minimized.

Copy link
Contributor Author

matthieu-rolland commented Oct 9, 2019

@zuk3975 I just realised you were talking about the LEGACY order page, and not the new one you are working on !

I will make an issue and fix it in legacy code. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.