impress-org / givewp

GiveWP - The #1 Donation Plugin for WordPress. Easily accept donations and fundraise using your WordPress website.
https://givewp.com/
GNU General Public License v3.0
345 stars 191 forks source link

formatCurrency is missing space between number and symbol #5367

Closed flack closed 2 years ago

flack commented 4 years ago

User Story

As a user, I want to see the properly formatted amount so that I can be sure about what I'm about to donate.

Details

In PHP, all currencies that are not explicitly listed get a space between the amount and the symbol:

https://github.com/impress-org/givewp/blob/194015ed4583da3fa84c7fe9be21a156649edd95/includes/currency-functions.php#L309

So when I open a donation form give-final-total-amount contains a string like this (because it's rendered by PHP):

50,00 CHF

when I click a donation level or manually enter a sum, the display changes to this (because it's rendered by JS):

50,00CHF

which is ugly and inconsistent

Expected Behavior

The amounts should be rendered consistently throughout the site, the JS and PHP renderer should produce the same result

Steps to Reproduce

  1. Go to any form (the prefilled donation amount will be rendered correctly)
  2. enter a manual amount or click a donation level
  3. the whitespace between the amount and the symbol is now missing

Visuals

rendered by PHP:

Screenshot_20201013_164942

rendered by Javascript:

Screenshot_20201013_165000

Acceptance Criteria

karlsauter commented 3 years ago

Hi, this is my first time trying to contribute to Open Source and this seems like a good bug to start. If no one is working on this, can I take it on?

kjohnson commented 3 years ago

@karlsauter please feel free to open a Pull Request. We will happy review and consider the change.

karlsauter commented 3 years ago

Hey @kjohnson, I found the code, made the change and it works. But different countries and regions format currency differently, some add spaces, others don't. A better solution would be to use the Intl.NumberFormat function, and ask the user their locale settings.

Besides, php formats currencies differently depending on the currency. For example CHF has a space before it, but the Polish Zloty doesn't.

Any thoughts?

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 14 additional days.

kjohnson commented 2 years ago

Code is /includes is now considered legacy code and is being slowly replaced in /src.

Closing this issue as we migrate functionality to /src.

flack commented 2 years ago

@kjohnson Maybe I'm missing something, but this ticket is about JS not rendering correctly, i.e. this function:

https://github.com/impress-org/givewp/blob/62b2716bd069a3e83bebe692e376f5f4cc28a6ec/assets/src/js/plugins/give-api/api.js#L42

So removing includes/ won't change the issue