Skip to content

Updated Unit Price Support #3772

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

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Updated Unit Price Support #3772

wants to merge 31 commits into from

Conversation

sunblaze
Copy link

@sunblaze sunblaze commented Apr 23, 2025

PR Summary:

Updates unit pricing to support new units and how they're displayed with translation support.

Why are these changes introduced?

New units would not be displayed correctly without updating the theme.

What approach did you take?

Chose to implement it completely in liquid.

Other considerations

  • We considered adding a new filter to liquid
  • We debated adding a formatted string in the unit price measurement drop

Visual impact on existing themes

  • remove css that capitalized all of unit prices (which included the units)
  • item/each display for new measured_type
  • m2 is now m² etc.

Testing steps/scenarios

  • Zip or push the theme from this branch with Shopify CLI
    • shopify theme push or shopify theme package
  • Test all the variants that they display correctly with different units
  • Test surfaces
    • Product page
    • Cart page
    • Cart drawer (you'll need to change the config manually in theme settings)
    • Order page (page buyer receives link to in email and order confirmation page)

Checklist

@sunblaze sunblaze force-pushed the updated-unit-price-support branch from 7565e40 to 5ba5e3b Compare April 23, 2025 20:29
This improved handling allows for languages to customize the format and
also display and handle the unit being generic and not unit based in the
case of item/count

Also includes:
- Removes whitespace around the / (eg. $1/g)
- Removes styling that capitalizes the unit (eg. $1/G)
- Update 4 translation files
@sunblaze sunblaze force-pushed the updated-unit-price-support branch from 3ab4c72 to 9859e3e Compare April 24, 2025 11:23
@sunblaze sunblaze changed the title [WIP] Updated Unit Price Support Updated Unit Price Support Apr 24, 2025
@sunblaze sunblaze marked this pull request as ready for review April 24, 2025 19:22
@trishrempel trishrempel requested a review from a team April 25, 2025 15:09
</span>
</span>
</small>
{%- if use_variant -%}
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does use_variant and target come from?
I'm wondering why the code changed to use these instead of product.selected_or_first_available_variant.

Choose a reason for hiding this comment

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

See lines 16 & 17 above

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok yeah, I see that now. It looks like we previously would have still shown the unit price from the selected or first available variant if use_variant was false, so I'd want to confirm from theme folks that this change makes sense.

@trishrempel
Copy link
Contributor

trishrempel commented Apr 25, 2025

The changes look great, here are screenshots with different scenarios:
Screenshot 2025-04-25 at 11 41 48 AM
Screenshot 2025-04-25 at 11 41 54 AM
Screenshot 2025-04-25 at 11 42 01 AM
Screenshot 2025-04-25 at 11 42 10 AM

@sunblaze sunblaze self-assigned this May 1, 2025
@sunblaze

This comment was marked as resolved.

@trishrempel
Copy link
Contributor

FYI, translations are updated as per translations review coordinated in #help-localization.

@bakura10
Copy link

Just found this PR. It feels wrong to have those translations at the theme level. Those should be provided by the platform directly.

@trishrempel trishrempel requested review from mkevinosullivan and removed request for mkevinosullivan May 21, 2025 14:13
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.

5 participants