fix: shipping save and continue button loading state #15241
Conversation
| try { | ||
| if (shippingContext.state.stage === "fulfillment_details") { | ||
| await shippingContext.state.fulfillmentDetailsFormikContext?.submitForm() | ||
| router.push(`/orders/${data.internalID}/payment`) |
There was a problem hiding this comment.
I've added a redirect here as when the user clicks "save and continue" after returning to the shipping step from the stepper it will only hit the shippingContext.state.stage === "fulfillment_details" and not redirect until the button is clicked a second time. @erikdstock let me know if this solution makes sense to you.
There was a problem hiding this comment.
I think it's worth looking at where state.stage is set and how it interacts with the routing. From a quick glance here I don't know if it's true that we always want to continue to payment after fulfillment details - the user might need to deal with shipping quotes. Have we tested that case?
There was a problem hiding this comment.
I've tested with shipping quotes and it seems to work as expected, but I will take a closer look.
There was a problem hiding this comment.
Regarding the button behavior, in the video on the description i still see a disabled state on first load, then no spinner when actually doing the mutation. Is this what we want?
After looking and thinking about this, I don't think we should merge this because it cuts out the functionality of the shipping context stage which manages where in the multiple forms the user is and what to do when they click the single button. If that truly isn't necessary then we should rewrite it to not be used at all.
My guess is that the extra click bug is tied to how the shipping quotes work and the fact that they refresh when you reload the shipping step. This code also causes the brief button loading state, which has been ticketed for a long time but is ultimately due to the refetching behavior).
The shipping step tries to bring you back to your current position on load, refilling the address form or re-selecting your current saved address, but it's not perfect: the refresh behavior means a saved shipping quote will never match the new ones you receive.
So every load of this view causes the order to mutate and we cannot just automatically re-fill all forms and plop the user back at the end of the flow based on a queried order. Because of that, we use the shippingContext.state.stage and shippingContext.actions.setStage() to advance the user through the view. This is why in the current code there is only one case where we use router.push - when the stage is 'fulfillment_details_saved` (only set in one place, here), ie there is nothing left to do here, the page loaded with full details and no refreshed shipping quote to re-confirm.
My guess it is somewhere around that last link that the 2-click issue needs to be fixed. On load, if you have an address saved, you should only need to click one more time to proceed - maybe the isAutomaticSave variable there is being set wrong.
If we have a bug where revisiting the page means it takes an extra click to proceed, i would start out by checking what shippingContext.state.stage is on load. From here it looks like we start on fulfillment details (ie shipping/pickup/address/phone) no matter what, then rely on the hooks i linked above to possibly resave things and get us into either the fulfillment_details_saved or shipping_quotes stage.
However, I don't think it's a bad idea to move all router pushing into the SaveAndContinueButton component. I just think it will require some other changes.
| try { | ||
| if (shippingContext.state.stage === "fulfillment_details") { | ||
| await shippingContext.state.fulfillmentDetailsFormikContext?.submitForm() | ||
| router.push(`/orders/${data.internalID}/payment`) |
There was a problem hiding this comment.
I think it's worth looking at where state.stage is set and how it interacts with the routing. From a quick glance here I don't know if it's true that we always want to continue to payment after fulfillment details - the user might need to deal with shipping quotes. Have we tested that case?
| loading={shippingContext.state.isPerformingOperation || undefined} | ||
| loading={isLoading} |
There was a problem hiding this comment.
I don't think we should completely remove the check on isPerformingOperation - or if we do, we should remove the code as well. Its purpose is specifically about this button's loading/disabled states - see L36 above.
The type of this PR is: Fix
Description
This updates the shipping route "save and continue" button to only display the loading state when the user clicks on the button, and not when the user initially enters the checkout flow.
It also fixes a bug where the user needs to click on the "save and continue" button twice in order to proceed to the payment step when they enter the shipping step from the stepper.
Screen.Recording.2025-02-20.at.10.27.03.PM.mov