Skip to content
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

Price range input placeholder/max values do not handle comma separated currencies (e.g. Euros) #255

Open
willbroderick opened this issue Jul 23, 2021 · 16 comments
Labels
Category: Bug Something isn't working

Comments

@willbroderick
Copy link

willbroderick commented Jul 23, 2021

Describe the current behavior
Store setup:
Use Euros, set money format to "€{{amount_with_comma_separator}}", on a store with products containing prices in the thousands.
The price range filter inputs do not contain correct values.

See max/placeholder here:
image

Referring to the code here: https://github.com/Shopify/dawn/blob/main/sections/main-collection-product-grid.liquid#L147
placeholder="{{ filter.range_max | money_without_currency | replace: ',', '' }}"
This goes: 2836.10 -> "2.836,10" -> "2.83610"

Describe the expected behavior
The output should be placeholder="2836.10"

Version information (Dawn, browsers and operating systems)

  • Dawn Version: 2.0.0-alpha.23
  • Any OS/Browser

Possible solution
The ideal solution would be for Shopify to handle this conversion, perhaps like this:
placeholder="{{ filter.range_max | money_to_input_value }}"

Additional context/screenshots
In my opinion the logic is far too complex for theme code. I don't think it's reasonable to expect a theme developer to be aware of the nuances of all currencies and their delimiters, whether or not they have 0/2/3 decimal places, to know what is going to be output by the current shop.money_format formatting, and when to check against cart.currency.iso_code instead.

All of these appear to factor into this conversion, and I've yet to see an example that does this correctly.

Locking this logic down to the the state of the world in July 2021 also seems like an odd choice to me.

@willbroderick willbroderick changed the title Price range input values do not handle Euros Price range input values do not handle comma separated currencies (e.g. Euros) Jul 23, 2021
@willbroderick willbroderick changed the title Price range input values do not handle comma separated currencies (e.g. Euros) Price range input placeholder/max values do not handle comma separated currencies (e.g. Euros) Jul 23, 2021
@bakura10
Copy link

I agree. Shopify definitely need to give us a better way to format the pricing.

I don't think also that the decimal value should be exposed at all in the filter, because the increment is usually 1, and can make the prices extremely long.

For now I have been able to create integer values using this, but I have no idea if this will work constantly for all currencies:

{%- assign min_value = filter.min_value.value | default: 0.0 | divided_by: 100.0 -%}
{%- assign max_value = filter.max_value.value | default: filter.range_max | divided_by: 100.0 -%}

I think Shopify should expose additional variables:

  • {{ filter.min_value.formatted_value }}, {{ filter.max_value.formatted_value }}, {{ filter.formatted_range_max }}: this would return a pre-formatted values and max range to be used in the selector, without relying on hard-coded currencies list
  • {{ filter.step }}: return the most appropriate step for the "number" input based on the currency. For instance, it could return 1 for euro, but 100 for yen.

@tyleralsbury
Copy link
Contributor

For now I have been able to create integer values using this, but I have no idea if this will work constantly for all currencies

That's definitely a concern. There are currencies that don't use a 100 cent basis. Some examples that I've been given are:

OMR subdivides into 1000 baisa
MRU subdivides into 5 khoums (so each khoum is 1/5 of an ouguiya)
JPY has not had any subunits since 1953

I agree - it would be very useful for there to be a platform feature to strip all of the special formatting and return a price in a way that the number input can use. My current thinking is something along the lines of a money filter that, similar to money_without_currency, can take a raw money value and turn it into a xxxx.yyyy format, regardless of the more typical formatting of a particular currency.

So it would be a filter that will always output the currency with no thousands separator and a . for the decimal, regardless of locale or currency. It wouldn't be very useful for presentation, but it would be very useful for use in a <input type="number">.

{{ filter.step }}: return the most appropriate step for the "number" input based on the currency. For instance, it could return 1 for euro, but 100 for yen.

This is very interesting. We'd need to base this on some sort of standard for each particular currency, and maybe it would be less on the filter and more on the store's current currency. This is a very cool idea because it's true that 1 yen means very little, so the steps should reflect something meaningful for the currency.

@tyleralsbury
Copy link
Contributor

Possible solution
The ideal solution would be for Shopify to handle this conversion, perhaps like this:
placeholder="{{ filter.range_max | money_to_input_value }}"

This proposal is really good and is in line with my current thinking on the matter as well. I'm going to speak with some folks who have more context on money formatting as a whole and see if we can come up with something that works well in all cases. Currency is complex and even differs between locales that use the same currency, and some currencies don't use 100 cent subdivisions, etc... so we'll need to be careful to ensure that it all works properly.

But you're 100% correct that the current implementation in Dawn is too complex. It's also fragile in the sense that if anything changes with regards to currencies, it's hard-coded, and thus can easily break.

@bakura10
Copy link

In addition of those improvements, I came into another issue.

When building the input for the price filter, I use the {{cart.currency.symbol}} to prepend the currency symbol:

image

Unfortunately, for some languages like French, currency code should be after, so it would make more sense to build it like this:

image

But this is impossible for us to know as it depends on the active locale, and we can't hardcode this at theme level.

I therefore suggest the following improvement by adding a new {{filter.currency_prefix}} and {{filter.currency_suffix}} drops. Shopify would automatically make one or the other blank depending on the active locale, which would allow us to build a locale-aware price filter.

@willbroderick
Copy link
Author

Just adding a little more context to this old ticket, since it caused us support debt today.

A merchant using RON (in the comma-separated list) contacted us as they are using period-separated decimals on their store (they said this was for an affiliate integration).

It may be an edge-case, but editing theme code to remove RON from the hard-coded list of comma-sep currencies was the only way to make the price filter work correctly on their store - in our theme, and also in Dawn.

@bacounts bacounts mentioned this issue Mar 18, 2022
8 tasks
@willbroderick
Copy link
Author

Would like to mention https://github.com/Shopify/theme-updates/issues/164 here.
On a new store with CLP currency:

product.price | money => $12.000
product.price | money_without_currency => 12,000

This makes price formatting difficult to handle or predict. 'money' seems to magically select a currency-specific format, but 'money_without_currency' uses the format in the currency settings.

@stufreen stufreen added the Category: Bug Something isn't working label Apr 18, 2023
@spy-v2
Copy link

spy-v2 bot commented Jun 4, 2023

Bot-B04L4P01WBU [2023-06-04 05:32PM UTC]

Github Comment from TMS Advisor: @Badj

🧵 Slack Thread

Slack Thread

Bot-B04L4P01WBU [2023-06-04 05:32PM UTC]

Github Comment from TMS Advisor: @Badj

Bot-B04L4P01WBU [2023-06-04 05:32PM UTC]

Good day folks, we received a report where using the price filter, adding a price in minimum or maximum field would default it back to 3.4 after a quick second. Or specifically, when leaving the input text fields where the numbers are being applied to the filter.

I want to provide this screen recording](https://shopify.click/23-04-86244-6231.webm)) for issue identification. And this slack thread from #help-search-and-discovery](https://shopify.slack.com/archives/C03GAQ1M1RA/p1681755772734339?thread_ts=1681755722.757809&cid=C03GAQ1M1RA)) for more detailed context.

May we ask if we have some updates for this issue?


Internal Dash: https://app.shopify.com/services/internal/shops/56157405251
Zendesk Ticket: https://shopify.zendesk.com/agent/tickets/37706487
Github URL: #255

Spy [2023-06-04 05:35PM UTC]

Comment successfully added: https://github.com/Shopify/shopify/issues/255#issuecomment-1575650962

Melanie Rawluk [2023-06-04 05:37PM UTC]

Hey, the repo should be dawn instead of shopify

Adriane Bugayong [2023-06-04 05:38PM UTC]

🤦 I'll try that again. Thanks!


Created originally at 2023-06-04 05:38PM UTC by Adriane Bugayong

@itshaunter
Copy link

itshaunter commented Oct 8, 2023

Hey team, we've received a new report of this behaviour found on the merchant's storefront. Currency is Hungarian Forints (HUF) and inspecting the price filter fields confirms the max values are being replaced with decimal point conversions. eg 32,790 listed as a max of 32.79:

image

Interestingly the Drawer filter layout does not exhibit this same issue in the Theme Editor environment, currently suggesting the merchant tests this live as a potential workaround: https://screenshot.click/08-27-at0iu-gwiwd.mp4

Edit: Merchant has confirmed the drawer layout works correctly and is currently using it as a workaround.

Internal

@willbroderick
Copy link
Author

Just wanted to share that we are now using this:

  assign uses_comma_decimals = false
  assign currency_test_string_unit = 101 | money_without_currency
  assign currency_test_string_thou = 100000 | money_without_currency

  if currency_test_string_unit == '1,01' or currency_test_string_thou == '1.000'
  	assign uses_comma_decimals = true
  endif

With output like this:

{%- if uses_comma_decimals -%}
  max="{{ filter.range_max | money_without_currency | remove: ' ' | remove: "'" | remove: '.' | replace: ',', '.' | ceil }}"
{%- else -%}
  max="{{ filter.range_max | money_without_currency | remove: ' ' | remove: "'" | remove: ',' | ceil }}"
{% endif %}

And we haven't received any support tickets about price filter for a while. I discovered that Dawn does not currently handle Kr.

@CianCroke
Copy link

Hi team, have a merchant experiencing this issue when attempting to filter by price on their storefront collections. Currency is set to Chilean Pesos (CLP), and when attempting to enter a price option, the filter is defaulting to display a different minimum amount than what's manually entered.

Internal
Zendesk
Screen recording example from merchant - SA was able to replicate unexpected behaviour via Theme Editor also : screen recording

@NeevusPeevus
Copy link

Hey team, another ticket with this issue on Refresh 12 and replicated on other themes. Currency is set to COP Colombian Peso. When entering price option the filter can display a different amount and products are not appearing once filtered.

Internal
Zendesk
Screen recording

@milan3015809
Copy link

Hi team! I hope you're doing well. Got another merchant who reported that when filtering the prices, the max price defaults to 5.790. They are currently using Dawn 12.0.0 theme and the filter issue is happening on all of the themes in their admin.

More details of the investigation here
  • Upon checking the Chrome Dev Tool, noticed that there is a placeholder of 5.790 even if there are no value input in the filter

1155-61197-10980-41970-90608

@PaulNewton
Copy link

@milan3015809 et al , while this should be a liquid issue, I've noticed people are showing that they are only checking with devtools for the computed DOM.
Out of curiosity is this "5.790" or similar cases also being checked if it's actually rendered in either the HTML source (ctrl+u)
, or any returned HTML from the section rendering api (either dig into devtools network responses, or directly navigate to the url for the section to be rendered for the feature).

Meaning:
if only the computed DOM is being checked then some cases could be a completely different issue for different sites with different javascript OR liquid.
if the rendered source is checked then it should be liquid for any specific case.

@milan3015809
Copy link

Hi @PaulNewton Thank you for the feedback. I tried viewing the HTML source while the price filter was set to 100 and it defaulted to 5.790, though I'm not entirely sure how to interpret this.
Screenshot 2024-02-24 3 38 56 AM

@tyleralsbury
Copy link
Contributor

I have a pull request up that should resolve this issue, as well as other issues around different currency formats in filters: #3280

@PaulNewton
Copy link

I'm not entirely sure how to interpret this

Interpret it as you knew it was a bug. It's just the DOM gets more and more misleading for bugs involved with complicated features: currency + filters.
This quick step solidifies the type of bug by trying to rule out any misleading javascript issues from things like search apps, currency apps, javascript customizations*, or the occasional browser cache annoyance etc etc ad nausem.
Always still a chance of liquid customizations, or js issues but 🤷

If you mean interpret the code itself , the placeholder's value is the filter range_max property value
https://github.com/Shopify/dawn/blob/main/snippets/facets.liquid#L925

*A crude example of a filter price javascript customization is monkey patching the frontend to suppress the prices of high cost products, free products, or specific products, or decimal-to-comma conversion etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

9 participants