-
Notifications
You must be signed in to change notification settings - Fork 12
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
Shipping method validation message not showing #20
Comments
Actually seems that something is calling the |
ohh I ran into this bug, and did not find an excellent solution. The problem is, there are two validations for the shipping blocks:
The problem was one of the validations always return true; the messages are actually shown, but then immediately hidden. tbh I'm not too sure how to resolve this at the minute. I will ask my teamm |
Thanks @andrewhowdencom this does seem to be the behaviour I am seeing. I'm no JS guru so this is quite time consuming for me to figure out but I'll try and get a stack trace on the second validation and see where it is occuring and whether I can find an elegant fix. If you can dig anything out please do let me know. |
Hola @bluec; In chatting to my colleagues it appears that we also did not find a solution in the time allotted, but rather side-stepped the issue by selecting a method "by default" (and thus never being invalid). If memory serves me, the error comes about as a result of special handling of the shipping method; it calls isValid and then $super.isValid (akin to parent::isValid()). To address this, it might be possible to set some sort of state on the parent, and skip revalidation if it has already been marked as invalid. Something like (in psuedo code):
and add special handling in the parent. I remember this being a difficult issue though, and unfortunately I do not have lots of time to look into it at the minute. If you do find something, I'm sure a PR would be much appreciated. <3 |
Thanks for this. I think I may have found the cause: it seems the shipping method step is repeated in the steps hash and is therefore validated twice. The second time it is validated it removes the validation error that was set when the previous step was validated. I am not entirely sure whether the problem is that the steps hash should not contain a repeated step for shipping method or whether the second validation is broken and should actually also fail. By setting a breakpoint inside the isValid() function in checkout.js you can see the steps array has two objects both with the same container:
The isValid() method iterates over each of the steps, and when it hits the first one it correctly validates the shipping method but when it hits the second one it overwrites the validation and removes the validation message. Digging back further, these two steps are added to the steps hash when the ShippingMethod class is initialised in /skin/[...]/js/ecomdev/checkitout/steps/shipping.js. It appears that the second duplicated step is added via this call on line 55:
Commenting out that call fixes the problem but I do not understand enough about why this method was even implemented to know what the unintentional consequences of doing this might be. I'm pretty out of my depth here in terms of JS and Prototype but I hope now that I've dug this far someone else may be able to offer some further insight. |
If memory serves, it's attempting to validate a block full of arbitrary content that may or may not exist; used to populate additional data such as gift wrapping messages or whatever. So, validating breaks it, but not validating breaks it also. (It's been a while lol) |
When a user fails to select a shipping method the validation message is not showing. I am seeing this on multiple sites, can anyone confirm or offer a solution?
I can step into the code and see that the
isValid
method is being called inskin/frontend/base/default/js/ecomdev/checkitout/steps/shipping.js
and this in turn is correctly calling theValidation.ajaxError
method found inskin/frontend/base/default/js/ecomdev/checkitout/compatibility/global.js
. This correctly call theshowAdvice
method which is supposed to trigger the Appear effect but from what I can see this never happens (i.e. the display stays as none and the opacity never moves to 100).Can anyone help?
The text was updated successfully, but these errors were encountered: