PAYONE-GmbH / magento-1

PAYONE Payment Extension for Magento 1
22 stars 41 forks source link

jQuery Dependency #294

Closed sprankhub closed 5 years ago

sprankhub commented 6 years ago

You include jQuery if Masterpass is used:

https://github.com/PAYONE-GmbH/magento-1/blob/ddb733b63060a72cb537a62677f156dedcb52301/app/design/frontend/base/default/template/payone/core/mastercard/masterpass/review.phtml#L30

However, you also use jQuery at other places like Amazon Pay or Ratepay or CC. See https://github.com/PAYONE-GmbH/magento-1/search?q=jquery&type=Code. You should remove these jQuery usages, because they fail if the shop does not use jQuery.

fjbender commented 6 years ago

Maybe I'm seeing this wrong: We should also make sure that jQuery is always there when we require it, right?

sprankhub commented 6 years ago

True. But you mostly use it only, because selectors are a bit more handy to write with it. You could as well use standard Prototype, which is included in each Magento system.

fjbender commented 6 years ago

Would make sense, however, I see it as more of a wishlist kinda thing right now, sorry.

sprankhub commented 6 years ago

I currently do not care, because my customers using PAYONE also use jQuery. However, it is weird that you did not have any problems with it yet. Any system, which does not use jQuery will not work with PAYONE. It obviously just shows how widely used jQuery is. However, since this is a bug, I would handle it like a bug. By the way, the fix is simple and straightforward as well.

rkconsulting commented 5 years ago

In the file ratepay.js you are using Vanilla JS, Prototype and jQuery ALL TO DO THE SAME THING at different times!? Ratepay will NOT work with this extension on a default Magento installation as there is no jQuery included. This is definitely a BUG, not a wishlist item!

fjbender commented 5 years ago

We'll take on some JS refactoring for the new year.

However, I'd appreciate it if we can have a friendly tone here, so nobody gets frustrated.

Happy holidays and thanks for your continued input and support!

rkconsulting commented 5 years ago

my apologies, no offense was intended. thanks for changing the status on this.

like the original poster noted, this also affects the credit card payment type, but only in the self-hosted mode (which prob nobody uses anymore these days). i've fixed these for our client and converted the jquery stuff to prototype, will submit an update here for your review when our integration is complete.

fjbender commented 5 years ago

No hard feelings :)

Thanks for that, looking forward! Self hosted mode shouldn't be widely used these days, but we'll take it into account as well.

fjbender commented 5 years ago

We analyzed the usage of different JS techniques now:

Vanilla Prototype jQuery File
V P x js/payone/core/addresscheck.js
V x J js/payone/core/amazonpay.js
V x x js/payone/core/client_api.js
V P x js/payone/core/creditcard.js
x P x js/payone/core/debitpayment.js
x P x js/payone/core/klarna.js
x P J js/payone/core/masterpass.js
x P x js/payone/core/onlinebanktransfer.js
x x x js/payone/core/opcheckoutmod.js
V P x js/payone/core/payolution.js
x P x js/payone/core/payolutionfraudprevention.js
V P J js/payone/core/ratepay.js
x P x js/payone/core/safe_invoice.js
x x x js/payone/core/sepa_input.js
x x x js/payone/core/sepa_validation.js
x P x js/payone/core/wallet.js
x x J app/design/frontend/base/default/template/payone/core/mastercard/masterpass/review.phtml
x P J app/design/frontend/base/default/template/payone/core/payment/method/form/creditcard.phtml
x x J app/design/frontend/base/default/template/payone/core/mastercard/masterpass/shortcut.phtml

We'll ditch jQuery where possible to avoid confusion and dependency weirdness. However, it's required by some external parties (e.g. MasterPass), so we'll have to rely on it there.

rkconsulting commented 5 years ago

Was fuer eine schoen bunte Liste! :) Ich werd demnaechst mal nen Pull Request machen fuer unsere Anpassungen fuer Ratepay und Kreditkarte, dort haben wir jQuery schon rausgehauen, koennt ihr ja als Vorlage fuer weitere Aenderungen nehmen.