search

mollie / magento2-hyva-checkout

8 stars 7 forks source link

Hiding apple pay after payment with javascript is bad practice #12

Closed jeroenbalk-adwise closed 1 year ago

jeroenbalk-adwise commented 1 year ago

Sorry for not responding for quite a time. This is the followup from: https://github.com/mollie/magento2-hyva-checkout/issues/5

Original message: _In vendor/mollie/magento2-hyva-checkout/src/Mollie_HyvaCheckout/view/frontend/templates/component/payment/method/applepay_after.phtml

there are some checks to validate if apple pay can be used and conditionally puts a display none on applepay method. This seems like a bad practice. Checks should be done in a different way so the array with available payment methods can be updated, and a refresh can be emitted on the magewire Payment MethodList.

The current 'quickfix' makes it more difficult to correctly tweak and style the checkout because pseudo classes like nth-child, odd/even can't be used correctly._

This is the new response from me to @Frank-Magmodules _Hello @Frank-Magmodules , I'm sorry for the late reaction.

If the Apple Pay calls are done client-side it's best to do some sort of callback to the server too. Not sure exactly what's the best way, it can be an ajax call or magewire, but somehow it is the cleanest to remove or hide the payment method from the MethodList in the backend (and probably save it on the quote or session?) and then fire a refresh for the Magewire component.

Just hiding the List item on the frontend with javascript does feel like a quick workaround because all other methods' conditionally show/hide are known to the backend. Also this interferes with frontend styling and makes the correct use of nth-child(odd) and (even) impossible. Feel free to contact me in the hyva slack workspace if you want. :)_

Frank-Magmodules commented 1 year ago

Hi There @jeroenbalk-adwise !

Thank you for coming back on this issue. We get your point about styling, and we will adjust the code so that the node is deleted and nth-child(even)call will work as expected.

About your proposed solution: We don’t agree that that is a good idea. The information about Apple Pay is only available on the frontend. Sending this information to the backend and then refreshing the list feels like more overkill causing weird refreshes that customers and other developers won’t understand. So for that reason we are closing this issue.