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] Localize currency symbol #8299

Merged
merged 32 commits into from
Jun 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
7890252
fix: localize currency symbol
mdneyazahmad Mar 23, 2022
3fa9bc3
refactor: move common function to hoc component
mdneyazahmad Mar 28, 2022
fad813c
refactor: rename variable names
mdneyazahmad Mar 28, 2022
0d56d96
refactor: ltr symbol component
mdneyazahmad Mar 28, 2022
1f50122
feat: add currency symbol utils
mdneyazahmad Mar 31, 2022
df003d6
refactor: currency utils functions
mdneyazahmad Mar 31, 2022
3cd30fa
test: add CurrencySymbolUtilsTest
mdneyazahmad Apr 1, 2022
155dc02
test: replace require to import
mdneyazahmad Apr 1, 2022
3e57e26
test: fix lint error
mdneyazahmad Apr 1, 2022
6a0e400
test: fix lint error
mdneyazahmad Apr 1, 2022
84bcdeb
refactor: CurrencySymbolUtils
mdneyazahmad Apr 1, 2022
696b7d5
test: refactor names
mdneyazahmad Apr 1, 2022
3ef60e8
docs: update comments
mdneyazahmad Apr 1, 2022
5992dfc
docs: update comment
mdneyazahmad Apr 1, 2022
b6d518c
merge main branch
mdneyazahmad May 1, 2022
21d3bc0
merge main branch
mdneyazahmad May 9, 2022
671f2d2
add comment for currencyList.json
mdneyazahmad May 10, 2022
7c4aa7e
refactor: IOUAmountPage
mdneyazahmad May 10, 2022
291782f
update comment in tests/unit/CurrencySymbolUtilsTest.js
mdneyazahmad May 12, 2022
1841022
refactor: TextInputWithCurrencySymbol
mdneyazahmad May 12, 2022
6147523
refactor: move inline function to class method
mdneyazahmad May 12, 2022
8e9764f
refactor: IOUAmountPage
mdneyazahmad May 12, 2022
46148a3
docs: add jsdocs
mdneyazahmad May 12, 2022
f85bbde
style: format code
mdneyazahmad May 17, 2022
04882ce
fix merge conflict
mdneyazahmad May 17, 2022
355e65d
refactor: remove unused code
mdneyazahmad May 17, 2022
fc91394
Merge branch 'fix/7915-localize-currency-symbol' of github.com:mdneya…
mdneyazahmad May 17, 2022
c25bf64
style: fix eslint error
mdneyazahmad May 17, 2022
133a62e
chore: update comment
mdneyazahmad May 17, 2022
f018a24
test: fix typo
mdneyazahmad Jun 2, 2022
c93b4cb
test: update comment
mdneyazahmad Jun 2, 2022
d87f569
fix: merge conflict
mdneyazahmad Jun 3, 2022
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
52 changes: 52 additions & 0 deletions src/components/AmountTextInput.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import React from 'react';
import PropTypes from 'prop-types';
import TextInput from './TextInput';
import styles from '../styles/styles';
import CONST from '../CONST';

const propTypes = {
/** Formatted amount in local currency */
formattedAmount: PropTypes.string.isRequired,

/** A ref to forward to amount text input */
forwardedRef: PropTypes.oneOfType([
PropTypes.func,
PropTypes.shape({current: PropTypes.instanceOf(React.Component)}),
]),

/** Function to call when amount in text input is changed */
onChangeAmount: PropTypes.func.isRequired,

/** Placeholder value for amount text input */
placeholder: PropTypes.string.isRequired,
};

const defaultProps = {
forwardedRef: undefined,
};

function AmountTextInput(props) {
return (
<TextInput
disableKeyboard
autoGrow
hideFocusedState
inputStyle={[styles.iouAmountTextInput, styles.p0, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
textInputContainerStyles={[styles.borderNone, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
onChangeText={props.onChangeAmount}
ref={props.forwardedRef}
value={props.formattedAmount}
placeholder={props.placeholder}
keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
/>
);
}

AmountTextInput.propTypes = propTypes;
AmountTextInput.defaultProps = defaultProps;
AmountTextInput.displayName = 'AmountTextInput';

export default React.forwardRef((props, ref) => (
// eslint-disable-next-line react/jsx-props-no-spreading
<AmountTextInput {...props} forwardedRef={ref} />
));
26 changes: 26 additions & 0 deletions src/components/CurrencySymbolButton.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import React from 'react';
mountiny marked this conversation as resolved.
Show resolved Hide resolved
import {TouchableOpacity} from 'react-native';
import PropTypes from 'prop-types';
import Text from './Text';
import styles from '../styles/styles';

const propTypes = {
/** Currency symbol of selected currency */
currencySymbol: PropTypes.string.isRequired,

/** Function to call when currency button is pressed */
onCurrencyButtonPress: PropTypes.func.isRequired,
};

function CurrencySymbolButton(props) {
return (
<TouchableOpacity onPress={props.onCurrencyButtonPress}>
<Text style={styles.iouAmountText}>{props.currencySymbol}</Text>
</TouchableOpacity>
);
}

CurrencySymbolButton.propTypes = propTypes;
CurrencySymbolButton.displayName = 'CurrencySymbolButton';

export default CurrencySymbolButton;
83 changes: 83 additions & 0 deletions src/components/TextInputWithCurrencySymbol.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import React from 'react';
import PropTypes from 'prop-types';
import AmountTextInput from './AmountTextInput';
import CurrencySymbolButton from './CurrencySymbolButton';
import * as CurrencySymbolUtils from '../libs/CurrencySymbolUtils';

const propTypes = {
/** A ref to forward to amount text input */
forwardedRef: PropTypes.oneOfType([
PropTypes.func,
PropTypes.shape({current: PropTypes.instanceOf(React.Component)}),
]),

/** Formatted amount in local currency */
formattedAmount: PropTypes.string.isRequired,

/** Function to call when amount in text input is changed */
onChangeAmount: PropTypes.func,

/** Function to call when currency button is pressed */
onCurrencyButtonPress: PropTypes.func,

/** Placeholder value for amount text input */
placeholder: PropTypes.string.isRequired,

/** Preferred locale of the user */
preferredLocale: PropTypes.string.isRequired,

/** Currency code of user's selected currency */
selectedCurrencyCode: PropTypes.string.isRequired,
};

const defaultProps = {
forwardedRef: undefined,
onChangeAmount: () => {},
onCurrencyButtonPress: () => {},
};

function TextInputWithCurrencySymbol(props) {
const currencySymbol = CurrencySymbolUtils.getLocalizedCurrencySymbol(props.preferredLocale, props.selectedCurrencyCode);
const isCurrencySymbolLTR = CurrencySymbolUtils.isCurrencySymbolLTR(props.preferredLocale, props.selectedCurrencyCode);

const currencySymbolButton = (
<CurrencySymbolButton
currencySymbol={currencySymbol}
onCurrencyButtonPress={props.onCurrencyButtonPress}
/>
);

const amountTextInput = (
<AmountTextInput
formattedAmount={props.formattedAmount}
onChangeAmount={props.onChangeAmount}
placeholder={props.placeholder}
ref={props.forwardedRef}
/>
);

if (isCurrencySymbolLTR) {
return (
<>
{currencySymbolButton}
{amountTextInput}
</>
);
}

return (
<>
{amountTextInput}
{currencySymbolButton}
</>
);
}

TextInputWithCurrencySymbol.propTypes = propTypes;
TextInputWithCurrencySymbol.defaultProps = defaultProps;
TextInputWithCurrencySymbol.displayName = 'TextInputWithCurrencySymbol';

export default React.forwardRef((props, ref) => (
// eslint-disable-next-line react/jsx-props-no-spreading
<TextInputWithCurrencySymbol {...props} forwardedRef={ref} />
));
37 changes: 37 additions & 0 deletions src/libs/CurrencySymbolUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import _ from 'underscore';
import * as NumberFormatUtils from './NumberFormatUtils';

/**
* Get localized currency symbol for currency(ISO 4217) Code
* @param {String} preferredLocale
* @param {String} currencyCode
* @returns {String}
*/
function getLocalizedCurrencySymbol(preferredLocale, currencyCode) {
const parts = NumberFormatUtils.formatToParts(preferredLocale, 0, {
style: 'currency',
currency: currencyCode,
});
return _.find(parts, part => part.type === 'currency').value;
}

/**
* Whether the currency symbol is left-to-right.
* @param {String} preferredLocale
* @param {String} currencyCode
* @returns {Boolean}
*/
function isCurrencySymbolLTR(preferredLocale, currencyCode) {
const parts = NumberFormatUtils.formatToParts(preferredLocale, 0, {
style: 'currency',
currency: currencyCode,
});

// Currency is LTR when the first part is of currency type.
return parts[0].type === 'currency';
}

export {
getLocalizedCurrencySymbol,
isCurrencySymbolLTR,
};
3 changes: 2 additions & 1 deletion src/pages/iou/IOUCurrencySelection.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import KeyboardAvoidingView from '../../components/KeyboardAvoidingView';
import Button from '../../components/Button';
import FixedFooter from '../../components/FixedFooter';
import * as IOU from '../../libs/actions/IOU';
import * as CurrencySymbolUtils from '../../libs/CurrencySymbolUtils';
import {withNetwork} from '../../components/OnyxProvider';
import networkPropTypes from '../../components/networkPropTypes';

Expand Down Expand Up @@ -127,7 +128,7 @@ class IOUCurrencySelection extends Component {
getCurrencyOptions() {
const currencyListKeys = _.keys(this.props.currencyList);
const currencyOptions = _.map(currencyListKeys, currencyCode => ({
text: `${currencyCode} - ${this.props.currencyList[currencyCode].symbol}`,
text: `${currencyCode} - ${CurrencySymbolUtils.getLocalizedCurrencySymbol(this.props.preferredLocale, currencyCode)}`,
searchText: `${currencyCode} ${this.props.currencyList[currencyCode].symbol}`,
currencyCode,
}));
Expand Down
61 changes: 20 additions & 41 deletions src/pages/iou/steps/IOUAmountPage.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import {
View,
TouchableOpacity,
InteractionManager,
} from 'react-native';
import PropTypes from 'prop-types';
Expand All @@ -16,10 +15,9 @@ import ROUTES from '../../../ROUTES';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import compose from '../../../libs/compose';
import Button from '../../../components/Button';
import Text from '../../../components/Text';
import CONST from '../../../CONST';
import TextInput from '../../../components/TextInput';
import canUseTouchScreen from '../../../libs/canUseTouchscreen';
import TextInputWithCurrencySymbol from '../../../components/TextInputWithCurrencySymbol';

const propTypes = {
/** Whether or not this IOU has multiple participants */
Expand All @@ -31,18 +29,6 @@ const propTypes = {
/** Callback to inform parent modal of success */
onStepComplete: PropTypes.func.isRequired,

/** The currency list constant object from Onyx */
currencyList: PropTypes.objectOf(PropTypes.shape({
/** Symbol for the currency */
symbol: PropTypes.string,

/** Name of the currency */
name: PropTypes.string,

/** ISO4217 Code for the currency */
ISO4217: PropTypes.string,
})).isRequired,

/** Previously selected amount to show if the user comes back to this screen */
selectedAmount: PropTypes.string.isRequired,

Expand Down Expand Up @@ -75,6 +61,7 @@ class IOUAmountPage extends React.Component {
this.updateAmount = this.updateAmount.bind(this);
this.stripCommaFromAmount = this.stripCommaFromAmount.bind(this);
this.focusTextInput = this.focusTextInput.bind(this);
this.navigateToCurrencySelectionPage = this.navigateToCurrencySelectionPage.bind(this);

this.state = {
amount: props.selectedAmount,
Expand Down Expand Up @@ -205,8 +192,19 @@ class IOUAmountPage extends React.Component {
.value();
}

navigateToCurrencySelectionPage() {
if (this.props.hasMultipleParticipants) {
return Navigation.navigate(ROUTES.getIouBillCurrencyRoute(this.props.reportID));
}
if (this.props.iouType === CONST.IOU.IOU_TYPE.SEND) {
return Navigation.navigate(ROUTES.getIouSendCurrencyRoute(this.props.reportID));
}
return Navigation.navigate(ROUTES.getIouRequestCurrencyRoute(this.props.reportID));
}

render() {
const formattedAmount = this.replaceAllDigits(this.state.amount, this.props.toLocaleDigit);

return (
<>
<View style={[
Expand All @@ -217,32 +215,14 @@ class IOUAmountPage extends React.Component {
styles.justifyContentCenter,
]}
>
<TouchableOpacity onPress={() => {
if (this.props.hasMultipleParticipants) {
return Navigation.navigate(ROUTES.getIouBillCurrencyRoute(this.props.reportID));
}
if (this.props.iouType === CONST.IOU.IOU_TYPE.SEND) {
return Navigation.navigate(ROUTES.getIouSendCurrencyRoute(this.props.reportID));
}
return Navigation.navigate(ROUTES.getIouRequestCurrencyRoute(this.props.reportID));
}}
>
<Text style={styles.iouAmountText}>
{lodashGet(this.props.currencyList, [this.props.iou.selectedCurrencyCode, 'symbol'])}
</Text>
</TouchableOpacity>
<TextInput
disableKeyboard
autoGrow
hideFocusedState
inputStyle={[styles.iouAmountTextInput, styles.p0, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
textInputContainerStyles={[styles.borderNone, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
onChangeText={this.updateAmount}
ref={el => this.textInput = el}
value={formattedAmount}
<TextInputWithCurrencySymbol
formattedAmount={formattedAmount}
onChangeAmount={this.updateAmount}
onCurrencyButtonPress={this.navigateToCurrencySelectionPage}
placeholder={this.props.numberFormat(0)}
keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
blurOnSubmit={false}
Copy link
Member

@rushatgabhane rushatgabhane Jun 7, 2022

Choose a reason for hiding this comment

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

Hello 👋

While refactoring this code, blurOnSubmit must have been removed by mistake. This effectively reverted #9064 which fixed #8362

@mdneyazahmad can you please follow up with a PR to fix this regression, thank you!
cc: @parasharrajat @mountiny

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah it seems like this got missed while merging main. Good catch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great catch, the PR is up here #9351

Would any one of you @rushatgabhane or @parasharrajat be able to give this a quick test based on the PR Rushat linked to make sure the behaviour works fine and approve the PR, I will merge. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Merged, thanks!

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 missed it while fixing the merge conflict. Thanks @rushatgabhane for catching it early and @mountiny for creating the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries, this is easy to miss.

preferredLocale={this.props.preferredLocale}
ref={el => this.textInput = el}
selectedCurrencyCode={this.props.iou.selectedCurrencyCode}
/>
</View>
<View style={[styles.w100, styles.justifyContentEnd]}>
Expand Down Expand Up @@ -273,7 +253,6 @@ IOUAmountPage.defaultProps = defaultProps;
export default compose(
withLocalize,
withOnyx({
currencyList: {key: ONYXKEYS.CURRENCY_LIST},
iou: {key: ONYXKEYS.IOU},
}),
)(IOUAmountPage);
38 changes: 38 additions & 0 deletions tests/unit/CurrencySymbolUtilsTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import _ from 'underscore';
import * as CurrencySymbolUtils from '../../src/libs/CurrencySymbolUtils';

// This file can get outdated. In that case, you can follow these steps to update it:
// - in src/libs/API.js
// - call: GetCurrencyList().then(data => console.log(data.currencyList));
// - copy the json from console and format it to valid json using some external tool
// - update currencyList.json
import currencyList from './currencyList.json';

const currencyCodeList = _.keys(currencyList);
const AVAILABLE_LOCALES = ['en', 'es'];

// Contains item [isLeft, locale, currencyCode]
const symbolPositions = [
[true, 'en', 'USD'],
[false, 'es', 'USD'],
];

describe('CurrencySymbolUtils', () => {
describe('getLocalizedCurrencySymbol', () => {
test.each(AVAILABLE_LOCALES)('Returns non empty string for all currencyCode with preferredLocale %s', (prefrredLocale) => {
_.forEach(currencyCodeList, (currencyCode) => {
const localizedSymbol = CurrencySymbolUtils.getLocalizedCurrencySymbol(prefrredLocale, currencyCode);

expect(localizedSymbol).toBeTruthy();
});
});
});

describe('isCurrencySymbolLTR', () => {
test.each(symbolPositions)('Returns %s for preferredLocale %s and currencyCode %s', (isLeft, locale, currencyCode) => {
const isSymbolLeft = CurrencySymbolUtils.isCurrencySymbolLTR(locale, currencyCode);
expect(isSymbolLeft).toBe(isLeft);
});
});
});

Loading