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

shippingService.isLoading - Question #3

Closed
sfritzsche opened this issue Dec 20, 2021 · 5 comments
Closed

shippingService.isLoading - Question #3

sfritzsche opened this issue Dec 20, 2021 · 5 comments
Labels
question Further information is requested

Comments

@sfritzsche
Copy link

Hi @mam08ixo || Christioph,

i'm currently working on a problem related to the Amasty Checkout (amasty/module-single-step-checkout) and this module.

As far as I can tell at the moment, using both modules at the same time, creates a javascript ajax loop, which is technically caused by the following 2 circumstances:

  1. your module encloses the storing of the shipping information with the calls

shippingService.isLoading(true); resp.
shippingService.isLoading(false);

  1. the Amasty Checkout module monitors the change of "shippingService.isLoading" and in case of "false" reacts by saving the shipping information.

set-shipping-information --> (your module) --> shippingService.isLoading(false) --> (amasty checkout) --> "set-shipping-information" --> ...

Question:

When developing the mixin, were you able to determine if the encapsulation using "shippingService"

shippingService.isLoading(true);
updateSelection()
shippingService.isLoading(false);

is absolutely necessary ?

Background:

Unfortunately, I don't have a complete overview of all the interrelationships in all the checkout JS components,
but as far as I can tell at first glance, the shippingService or the isLoading property is always linked to changes in shippingRates.

In other words: Whenever I want to reset the shippingRates in the frontend, there should be an encapsulation with the "shippingService". But the call "updateSelection" only includes updating your own fields, so the encapsulation doesn't seem necessary.

Fallback question:

If from your point of view it should be necessary after all: Do you see another way to solve the problem between these modules ?

@mam08ixo
Copy link
Contributor

My first impression is that the isLoading calls are made to control the loading spinner and prevent any additional user input while the service selection is being persisted. I need to do some more research though to validate that.

@mam08ixo
Copy link
Contributor

A little more detail: We have a mixin that intercepts the click on the Next button in the checkout shipping step. Before returning back to the original action, we persist the service selection via API call. This should not take too long. However, depending on connection, server load, etc. it may take long enough for the customer to go crazy with the inputs and Next button. Therefore, we launch the loading spinner on the shipping method area because the overlay prevents any user input until the API call was finished.

If you do not consider this a risk in your environment, feel free to remove the isLoading calls in your custom mixin.

@sfritzsche
Copy link
Author

The purpose of the mixin was already known to us. We had already developed a compatibility module in parallel:

https://github.com/mediarox/module-compatibility-amasty-checkout-netresearch-shipping-ui

However, maintaining the compatibility module seems a touch "too much" for us. We had hoped to solve the problem somehow differently.

After your last message, we took another look at the DHL module in a clean 2.4.3p1 (luma). At the moment it seems that the mixin (Netresearch_ShippingUi/js/mixin/checkout/set-shipping-information) is not needed. Your additional fields are already all monitored and on any change, already
"nrshipping/shipping-option/selection/update" will be executed. So the additional execution via the "Next" button seems unnecessary.

Could you please check this again ?

@mam08ixo
Copy link
Contributor

There are scenarios where a validation of the service selection makes sense at this stage. If you are asking to remove the mixin from the module, then I have to decline.

Replacing the mixin by a version with no isLoading call, like you did, is the proper workaround in my opinion.

@sfritzsche
Copy link
Author

"There are scenarios where a validation of the service selection makes sense at this stage." --> Without a concrete scenario, we can't quite follow that. After all, that's what we were doubting.

But we don't want to "nag" any further here. We have a solution for us.

We just hoped that we would find a general solution, so that the compatibility module is not needed.

As far as many thanks.

@ngolatka ngolatka added the question Further information is requested label Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants