-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
🌸✨ Marketplace
: Shopper
chooses DeliveryArea
#1757
Conversation
Marketplace
: Shopper
chooses DeliveryArea
independentlyMarketplace
: Shopper
chooses DeliveryArea
independently
Marketplace
: Shopper
chooses DeliveryArea
independentlyMarketplace
: Shopper
chooses DeliveryArea
independently
If I could get some review on the direction that would be appreciated; even though the tests aren't passing yet. |
6f1ffae
to
20a1445
Compare
Experience tells me that each section of the checkout experience needs a more human friendly heading that addresses the shopper in a conversation. For example, instead of just "Delivery Area" for the new delivery selection area you would have "Choose your delivery area" -- also that heading stays in edit or persisted mode. And for next section "Add items to your cart" or something. The latter would replace Name/Description/Price table-style headings. |
I like the idea of putting some headings! @anaulin has been advocating for splitting the How about: <Card>
<h3>Delivering To</h3>
<select>...</select>
</Card> |
116573e
to
eb3f4b4
Compare
Makes sense, especially for when product lists start to get longer. Like adding a "View cart" button or panel pinned to the bottom of the screen which pops up a summary overlay. I literally built this for ticket purchasing at Eventbrite 😂 |
@@ -0,0 +1,17 @@ | |||
<%= turbo_frame_tag(dom_id) do %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this should maybe be inside the CardComponent
? And then the form
shouldn't have a CardComponent
around it...
So we wind up with
<Card>
<TurboFrame>
<%- if delivery_area.present? %>
<Cart::DeliveryArea cart />
<%- else %>
<Cart::DeliveryAreaForm cart />
<%- end %>
</TurboFrame>
</Card>
9aa19dd
to
b34ac82
Compare
- #1672 So, in reviewing a bunch of the delivery app workflows, most of them have: - Choose Pickup or Delivery - Enter Address - Browse Menu - Tap Item - Add Item to Order This lends me to believe we want to switch to gathering the `DeliveryArea` *then* `DeliveryAddress` *then* showing the `Products` that are available. This uses TurboFrames to replace pieces of the page, rather than relying on TurboStreams. On the one hand, it's lot easier (just respond with html with matching `turbo_frame_tag`s`). Blerg; anyway, I'm checking this in now; but not sure if I like it enough. Thoughts? In particular: - Copy-editing - Design tweaks? - Name Collisions: How Undo? * Establish more appropriate DOM Hierarchy Card => TurboFrame => Content appears to be a little bit more reasonable for how we nest dom things. The Card is a layout and presentation element, so it sits as close to the top of the dom as possible. The Turbo Frame allows the pieces to be swapped; so it resides within the layout. The Content itself (be it a `DisplayArea::Form` or `DisplayArea`) is the most inwardly nested, so that it can be replaced with frames, or laid out differently depending on where it is being presented.
b34ac82
to
0eb9dc9
Compare
@zinc-collective/convene-maintainers and @zinc-collective/convene-contributors - I am marking this as ready for review, since it has test coverage on the new endpoints. |
Marketplace
: Shopper
chooses DeliveryArea
independentlyMarketplace
: Shopper
chooses DeliveryArea
@@ -0,0 +1,2 @@ | |||
<%- breadcrumb :marketplace_cart_delivery_area, cart %> | |||
<%= render Marketplace::Cart::DeliveryAreaComponent.new(cart: cart) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would one land one this show
page? I can't find any links to a standalone show page in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When Turbo fails (or a Shopper disables it... or they command + click/open in a new tab), submitting cart/delivery_areas#update
redirects them to the cart/delivery_areas#show
endpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes a lot of sense as our general product direction: ask for delivery area first, then display delivery costs, ordering windows, etc, depending on that, even before the shopper starts adding things to the cart.
Noodling out loud, for a future iteration, it would be nice to:
- prompt the shopper more clearly to select the delivery area before they can do anything else (maybe even like a modal overlay)
- if there is only 1 delivery area in the marketplace, select that for them automagically and don't prompt them to select anything
(See my question below about where is the show page with breadcrumb shown.)
Agreed.
Ya! I'm going to do that in a follow-on PR! |
Marketplace
- Improve the checkout experience #1672Marketplace
:Delivery
#1325Marketplace
:DeliveryArea
#1136Marketplace
: Buying 🥡Products
#1326So, in reviewing a bunch of the delivery app workflows, most of them have:
This lends me to believe we want to switch to gathering the
DeliveryArea
thenDeliveryAddress
then showing theProducts
that are available.This uses TurboFrames to replace pieces of the page, rather than relying on TurboStreams. On the one hand, it's lot easier (just respond with html with matching
turbo_frame_tag
s`).Blerg; anyway, I'm checking this in now; but not sure if I like it enough.
Thoughts? In particular:
After!
Video
Screen.Recording.2023-08-14.at.12.51.36.PM.mov