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

Fix: disable currency abbreviations in custom amounts #7625

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import classNames from 'classnames';
import {__} from '@wordpress/i18n';
import CurrencyInput from 'react-currency-input-field';
import { CurrencyInputOnChangeValues } from "react-currency-input-field/dist/components/CurrencyInputProps";
import {useEffect, useState} from "react";
import useCurrencySeparator from "@givewp/forms/registrars/templates/fields/Amount/useCurrencySeparators";

/**
* @since 3.0.0
Expand All @@ -21,13 +23,18 @@ type CustomAmountProps = {
const CustomAmount = (
{defaultValue, fieldError, currency, value, onValueChange}: CustomAmountProps
) => {
const {groupSeparator, decimalSeparator} = useCurrencySeparator();

return (
<div className={classNames('givewp-fields-amount__input-container', {invalid: fieldError})}>
<CurrencyInput
intlConfig={{
locale: navigator.language,
currency,
}}
disableAbbreviations
decimalSeparator={decimalSeparator}
groupSeparator={groupSeparator}
className="givewp-fields-amount__input givewp-fields-amount__input-custom"
aria-invalid={fieldError ? 'true' : 'false'}
id="amount-custom"
Expand All @@ -36,7 +43,7 @@ const CustomAmount = (
defaultValue={defaultValue}
value={value}
decimalsLimit={2}
onValueChange={(value) => onValueChange(value !== undefined ? value : '')}
onValueChange={(value) => {onValueChange(value !== undefined ? value : '')}}
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { useState, useEffect } from 'react';

/**
* Custom hook to determine the group and decimal separators based on locale.
*/
export default function useCurrencySeparator(){
const [groupSeparator, setGroupSeparator] = useState(',');
const [decimalSeparator, setDecimalSeparator] = useState('.');

useEffect(() => {
const formatter = new Intl.NumberFormat();
Copy link
Member

Choose a reason for hiding this comment

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

This will override the browser's locale and assume a US locale for separators.

While is does prevent the NaN issue, we shouldn't do so by locking the local config.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I don't think we need a hook for this, since the value shouldn't change after page load. Instead, we could initialize these variables at the top of the component file,

import ...

const formatter = new Intl.NumberFormat();
// ...

const Component = () => {
    // ...
}

export default Component;

Copy link
Member

Choose a reason for hiding this comment

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

@JoshuaHungDinh I updated this to use the browser locale configuration and replaced the non-breaking space on the group separator to avoid any conflicts with the suffix separator used by the <CurrencyInput /> component.

const formatter = new Intl.NumberFormat(navigator.language);
const groupSeparator = formatter.format(1000).replace(/[0-9]/g, '');
const decimalSeparator = formatter.format(1.1).replace(/[0-9]/g, '');

  //
  groupSeparator={
    /**
     * Replace non-breaking space to avoid conflict with the suffix separator.
     * @link https://github.com/cchanxzy/react-currency-input-field/issues/266
     */
    groupSeparator.replace(/\u00A0/g, ' ')
  }
  //

Copy link
Contributor Author

@JoshuaHungDinh JoshuaHungDinh Dec 9, 2024

Choose a reason for hiding this comment

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

Awesome, these changes make sense.

Thanks for helping me move this forward. I'll send it to QA.

const getGroupSeparator = formatter.format(1000).replace(/[0-9]/g, '');
const getDecimalSeparator = formatter.format(1.1).replace(/[0-9]/g, '');

// Ensure separators are not the same.
if (getGroupSeparator === getDecimalSeparator) {
setDecimalSeparator(getDecimalSeparator === '.' ? ',' : '.');
} else {
setGroupSeparator(getGroupSeparator);
setDecimalSeparator(getDecimalSeparator);
}
}, []);

return { groupSeparator, decimalSeparator };
};

Loading