Skip to content

Conversation

willbouch
Copy link
Contributor

@willbouch willbouch commented Aug 22, 2025

Handled a nicer way the cart for promotions. Here is what it looks like when there is a promotion on items:
Screenshot 2025-08-22 at 2 17 30 PM

And this is when the discount is free shipping:
Screenshot 2025-08-22 at 2 18 02 PM

Still waiting for approval from design team but it should be ok

CLOSES CORE-1132

Copy link

vercel bot commented Aug 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
nextjs-starter-medusa Ready Ready Preview Comment Aug 25, 2025 9:30pm
storefront-v2-staging Ready Ready Preview Comment Aug 25, 2025 9:30pm

{!!discount_total && (
<div className="flex items-center justify-between">
<span>Discount</span>
<span>Subtotal (excl. shipping and taxes)</span>
Copy link
Contributor Author

@willbouch willbouch Aug 22, 2025

Choose a reason for hiding this comment

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

This part is weird. It says "excl. shipping and taxes" but then we display subtotal which has shipping costs inside. Looks like a bug to me

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be "excl. discount and taxes"?

Also, it's a bit odd that the subtotal is crossed since nothing is "reduced" from the subtotal when a discount is applied - should we cross the total instead?

Copy link
Contributor Author

@willbouch willbouch Aug 26, 2025

Choose a reason for hiding this comment

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

it could be the 3. Cause there is also no shipping. However, I am more used to seeing "excl. discount and taxes" in other websites. I guess the reason would be to not shove into someone's face that they didn't get a discount ahha

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean about the subtotal crossed. Idk, I checked with Ludvig and we settled on this way. I think that crossing the total could more confusing though no? You would see something like

subtotal 10$
shipping 100$
taxes 0$

total 110$ 104$

You don't really know where the promotion comes from I guess? Although with "subtotal", I kind of assume that promotion comes from the items directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@olivermrbl you can let me know what you think

Copy link
Contributor

@olivermrbl olivermrbl Aug 26, 2025

Choose a reason for hiding this comment

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

Hmm, first of all, should subtotal contain shipping? This seems like a potential mishap from our side, if we include shipping in the core. Subtotal is typically the sum of the subtotal of items.

Copy link
Contributor Author

@willbouch willbouch Aug 26, 2025

Choose a reason for hiding this comment

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

subtotal should not contain shipping. At least, I am used to seeing it without including shipping. This is a screenshot from SSENSE

Screenshot 2025-08-26 at 8 14 27 AM

And ChatGPT thinks the same
Screenshot 2025-08-26 at 8 15 30 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, exactly. We need to update the core then. Could I get you to open a ticket for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ok I see what you mean. Can I do it while reworking the totals?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes sure!

Comment on lines +38 to +39
const totalItemDiscount = items?.flatMap(i => i.adjustments).reduce((acc, curr) => (curr?.amount || 0) + acc, 0)
const totalShippingDiscount = shipping_methods?.flatMap(sm => sm.adjustments).reduce((acc, curr) => (curr?.amount || 0) + acc, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

tought: should we introduce these as separate total props on cart/order in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be reworking the totals section and the order details section. I believe what you are saying would make sense. If I go that route, I will come back to change it here after I do it in the admin dashboard

{!!discount_total && (
<div className="flex items-center justify-between">
<span>Discount</span>
<span>Subtotal (excl. shipping and taxes)</span>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be "excl. discount and taxes"?

Also, it's a bit odd that the subtotal is crossed since nothing is "reduced" from the subtotal when a discount is applied - should we cross the total instead?

@willbouch willbouch requested a review from fPolic August 26, 2025 11:49
@willbouch willbouch merged commit 0ad5a8f into main Aug 26, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants