Skip to content

Conversation

@DanielWright
Copy link

Several Spree modules rely on certain checkout-transition callbacks being triggered during checkout. As a result of smushing delivery and payment steps together, Sprangular inadvertently skips these callbacks, which causes some weird behaviours. See #168 for additional details.

This commit attempts to correct this issue by performing the two checkout steps discretely.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line exceeds maximum allowed length

@joshnuss
Copy link
Contributor

joshnuss commented Jun 9, 2015

@dw Looks good 👍

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line contains inconsistent indentation

@DanielWright
Copy link
Author

CI builds (other than Hound) are not supposed to pass, right?

@DanielWright
Copy link
Author

@joshnuss Any issues merging this?

@DanielWright
Copy link
Author

Actually, hold pls.

@DanielWright
Copy link
Author

The only bit that makes me slightly uncomfortable passing in an on-selection handler to the shipping-rate selection directive. It feels awkward; ng-model should handle it. But I can't figure out how to impel advancement of the checkout process if the shipping-rate doesn't change.

Thoughts?

@dw
Copy link

dw commented Jun 9, 2015

@joshnuss Much as I appreciate the complement, I wish I didn't receive quite as much random GitHub mail of this sort :)

@joshnuss
Copy link
Contributor

joshnuss commented Jun 9, 2015

@DanielWright if you've tested it, it looks good to merge

(sorry I keep using the wrong username)

@DanielWright
Copy link
Author

Hah, it's a bit confusing since I go by DW everywhere else. (Sorry, the-real-dw.)

Rod and I are working through the UX implications in one of our host apps, I'm going to leave this PR unmerged until we are feeling a bit more comfortable.

@DanielWright
Copy link
Author

@rodrigodalcindev

How do you feel about where the UX is at for this?

@rodrigodalcindev
Copy link

@DanielWright I'm not sure we have got to a consensus regarding the UX for the delivery-payment split yet.

Several Spree modules rely on certain checkout-transition callbacks
being triggered during checkout. As a result of smushing delivery and
payment steps together, Sprangular inadvertently skips these callbacks,
which causes some weird behaviours.

This commit attempts to correct this issue by performing the two steps
discretely.
@DanielWright DanielWright force-pushed the discrete-delivery-and-payment branch from d7e6b58 to 899adda Compare June 19, 2015 14:15
@DanielWright
Copy link
Author

@rodrigodalcindev

Same question as last time. D'you think we're close enough on UX now that we can get this integrated and mergeable?

This commit tweaks the view's UX in order to remove the onSelection method from the shipping rates list, requiring the delivery step to be manually advanced by the user.
@rodrigodalcindev
Copy link

@DanielWright I've tweaked the checkout view's UI/UX in order to reflect the step splitting. I'll rebase it to #63 once the PR is merged to master.

The current UX is to return the user with a popup when the data fails to load and there is a routeChangeError. This is a bit aggressive and confusing to the user. This commit changes this behaviour to logging a message in the console and redirecting to a 404 page, as seen in #198.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants