Skip to content

feat(): handle free shipping in cart display #508

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 26, 2025
Merged

feat(): handle free shipping in cart display #508

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73360ae
feat(): handle free shipping in cart display
willbouch Aug 22, 2025
310af50
remove console log
willbouch Aug 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 39 additions & 19 deletions src/modules/common/components/cart-totals/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import { convertToLocale } from "@lib/util/money"
import React from "react"
import { Badge } from "@medusajs/ui"

type CartTotalsProps = {
totals: {
Expand All @@ -12,50 +13,69 @@ type CartTotalsProps = {
discount_total?: number | null
gift_card_total?: number | null
currency_code: string
item_subtotal?: number | null
item_total?: number | null
shipping_subtotal?: number | null
shipping_methods?: {adjustments?: {amount: number}[]}[]
items?: {adjustments?: {amount: number}[]}[]
}
}

const CartTotals: React.FC<CartTotalsProps> = ({ totals }) => {
const {
currency_code,
total,
subtotal,
tax_total,
discount_total,
gift_card_total,
item_subtotal,
item_total,
shipping_subtotal,
shipping_total,
shipping_methods,
items
} = totals

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)
Comment on lines +38 to +39
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


return (
<div>
<div className="flex flex-col gap-y-2 txt-medium text-ui-fg-subtle ">
<div className="flex items-center justify-between">
<span className="flex gap-x-1 items-center">
Subtotal (excl. shipping and taxes)
</span>
<span data-testid="cart-subtotal" data-value={subtotal || 0}>
{convertToLocale({ amount: subtotal ?? 0, currency_code })}
</span>
</div>
{!!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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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!

<div className="flex items-center gap-2">
<span className={totalItemDiscount ? 'line-through text-ui-fg-muted' : ''} data-testid="cart-item-subtotal" data-value={item_subtotal || 0}>
{convertToLocale({ amount: item_subtotal ?? 0, currency_code })}
</span>
{!!totalItemDiscount && (
<div className="flex items-center justify-between">
<span
className="text-ui-fg-interactive"
data-testid="cart-discount"
data-value={discount_total || 0}
data-value={item_total || 0}
>
-{" "}
{convertToLocale({ amount: discount_total ?? 0, currency_code })}
{convertToLocale({ amount: item_total ?? 0, currency_code })}
</span>
</div>
)}
</div>
)}
</div>
<div className="flex items-center justify-between">
<span>Shipping</span>
<span data-testid="cart-shipping" data-value={shipping_subtotal || 0}>
{convertToLocale({ amount: shipping_subtotal ?? 0, currency_code })}
</span>
<div className="flex items-center gap-2">
<span className={totalShippingDiscount ? 'line-through text-ui-fg-muted' : ''} data-testid="cart-shipping" data-value={shipping_subtotal || 0}>
{convertToLocale({ amount: shipping_subtotal ?? 0, currency_code })}
</span>
{!!totalShippingDiscount && (
<span
className="text-ui-fg-interactive"
data-testid="cart-shipping-discount"
data-value={shipping_total || 0}
>
{convertToLocale({ amount: shipping_total ?? 0, currency_code })}
</span>
)}
</div>
</div>
<div className="flex justify-between">
<span className="flex gap-x-1 items-center ">Taxes</span>
Expand Down