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

IOU - Localize amount text input #7221

Merged
merged 7 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ localize the following types of data when presented to the user (even accessibil

- Texts: See [translate method](https://github.com/Expensify/App/blob/655ba416d552d5c88e57977a6e0165fb7eb7ab58/src/libs/translate.js#L15)
- Date/time: see [DateUtils](https://github.com/Expensify/App/blob/f579946fbfbdc62acc5bd281dc75cabb803d9af0/src/libs/DateUtils.js)
- Numbers and amounts: see [numberFormat](https://github.com/Expensify/App/tree/965f92fc2a5a2a0d01e6114bf5aa8755b9d9fd1a/src/libs/numberFormat)
- Numbers and amounts: see [NumberFormatUtils](https://github.com/Expensify/App/blob/55b2372d1344e3b61854139806a53f8a3d7c2b8b/src/libs/NumberFormatUtils.js) and [LocaleDigitUtils](https://github.com/Expensify/App/blob/55b2372d1344e3b61854139806a53f8a3d7c2b8b/src/libs/LocaleDigitUtils.js)
- Phones: see [LocalPhoneNumber](https://github.com/Expensify/App/blob/bdfbafe18ee2d60f766c697744f23fad64b62cad/src/libs/LocalePhoneNumber.js#L51-L52)

In most cases, you will be needing to localize data used in a component, if that's the case, there's a HOC [withLocalize](https://github.com/Expensify/App/blob/37465dbd07da1feab8347835d82ed3d2302cde4c/src/components/withLocalize.js).
Expand Down
7 changes: 5 additions & 2 deletions src/components/BigNumberPad.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ import PropTypes from 'prop-types';
import styles from '../styles/styles';
import Button from './Button';
import ControlSelection from '../libs/ControlSelection';
import withLocalize, {withLocalizePropTypes} from './withLocalize';

const propTypes = {
/** Callback to inform parent modal with key pressed */
numberPressed: PropTypes.func.isRequired,

...withLocalizePropTypes,
};

const padNumbers = [
Expand Down Expand Up @@ -57,7 +60,7 @@ class BigNumberPad extends React.Component {
<Button
key={column}
style={[styles.flex1, marginLeft]}
text={column}
text={column === '<' ? column : this.props.toLocaleDigit(column)}
onLongPress={() => this.handleLongPress(column)}
onPress={() => this.props.numberPressed(column)}
onPressIn={ControlSelection.block}
Expand All @@ -78,4 +81,4 @@ class BigNumberPad extends React.Component {

BigNumberPad.propTypes = propTypes;

export default BigNumberPad;
export default withLocalize(BigNumberPad);
13 changes: 10 additions & 3 deletions src/components/ReportActionItem/IOUPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ const propTypes = {
/** Email address of the creator of this iou report */
ownerEmail: PropTypes.string,

/** Outstanding amount of this transaction */
cachedTotal: PropTypes.string,
/** Outstanding amount in cents of this transaction */
total: PropTypes.number,

/** Currency of outstanding amount of this transaction */
currency: PropTypes.string,

/** Does the iouReport have an outstanding IOU? */
hasOutstandingIOU: PropTypes.bool,
Expand Down Expand Up @@ -108,7 +111,11 @@ const IOUPreview = (props) => {
const ownerName = lodashGet(props.personalDetails, [ownerEmail, 'firstName'], '') || Str.removeSMSDomain(ownerEmail);
const managerAvatar = lodashGet(props.personalDetails, [managerEmail, 'avatar'], '');
const ownerAvatar = lodashGet(props.personalDetails, [ownerEmail, 'avatar'], '');
const cachedTotal = props.iouReport.cachedTotal ? props.iouReport.cachedTotal.replace(/[()]/g, '') : '';
const cachedTotal = props.iouReport.total && props.iouReport.currency
? props.numberFormat(
props.iouReport.total / 100,
{style: 'currency', currency: props.iouReport.currency},
) : '';
return (
<TouchableWithoutFeedback onPress={props.onPreviewPressed}>
<View style={[styles.iouPreviewBox, ...props.containerStyles]}>
Expand Down
29 changes: 27 additions & 2 deletions src/components/withLocalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import ONYXKEYS from '../ONYXKEYS';
import * as Localize from '../libs/Localize';
import DateUtils from '../libs/DateUtils';
import * as LocalePhoneNumber from '../libs/LocalePhoneNumber';
import numberFormat from '../libs/numberFormat';
import * as NumberFormatUtils from '../libs/NumberFormatUtils';
import * as LocaleDigitUtils from '../libs/LocaleDigitUtils';
import CONST from '../CONST';

const LocaleContext = createContext(null);
Expand All @@ -29,6 +30,12 @@ const withLocalizePropTypes = {

/** Returns an internationally converted phone number with the country code */
fromLocalPhone: PropTypes.func.isRequired,

/** Gets the standard digit corresponding to a locale digit */
fromLocaleDigit: PropTypes.func.isRequired,

/** Gets the locale digit corresponding to a standard digit */
toLocaleDigit: PropTypes.func.isRequired,
};

const localeProviderPropTypes = {
Expand Down Expand Up @@ -56,6 +63,8 @@ class LocaleContextProvider extends React.Component {
timestampToDateTime: this.timestampToDateTime.bind(this),
fromLocalPhone: this.fromLocalPhone.bind(this),
toLocalPhone: this.toLocalPhone.bind(this),
fromLocaleDigit: this.fromLocaleDigit.bind(this),
toLocaleDigit: this.toLocaleDigit.bind(this),
preferredLocale: this.props.preferredLocale,
};
}
Expand All @@ -75,7 +84,7 @@ class LocaleContextProvider extends React.Component {
* @returns {String}
*/
numberFormat(number, options) {
return numberFormat(this.props.preferredLocale, number, options);
return NumberFormatUtils.format(this.props.preferredLocale, number, options);
}

/**
Expand Down Expand Up @@ -115,6 +124,22 @@ class LocaleContextProvider extends React.Component {
return LocalePhoneNumber.fromLocalPhone(this.props.preferredLocale, number);
}

/**
* @param {String} digit
* @returns {String}
*/
toLocaleDigit(digit) {
return LocaleDigitUtils.toLocaleDigit(this.props.preferredLocale, digit);
}

/**
* @param {String} localeDigit
* @returns {String}
*/
fromLocaleDigit(localeDigit) {
return LocaleDigitUtils.fromLocaleDigit(this.props.preferredLocale, localeDigit);
}

render() {
return (
<LocaleContext.Provider value={this.getContextValue()}>
Expand Down
73 changes: 73 additions & 0 deletions src/libs/LocaleDigitUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import _ from 'underscore';

import * as NumberFormatUtils from './NumberFormatUtils';

const STANDARD_DIGITS = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '.', '-', ','];

const INDEX_DECIMAL = 10;
const INDEX_MINUS_SIGN = 11;
const INDEX_GROUP = 12;

const getLocaleDigits = _.memoize((locale) => {
const localeDigits = _.clone(STANDARD_DIGITS);
for (let i = 0; i <= 9; i++) {
localeDigits[i] = NumberFormatUtils.format(locale, i);
}
_.forEach(NumberFormatUtils.formatToParts(locale, 1000000.5), (part) => {
switch (part.type) {
case 'decimal':
localeDigits[INDEX_DECIMAL] = part.value;
break;
case 'minusSign':
localeDigits[INDEX_MINUS_SIGN] = part.value;
break;
case 'group':
localeDigits[INDEX_GROUP] = part.value;
break;
default:
break;
}
});
return localeDigits;
});

/**
* Gets the locale digit corresponding to a standard digit.
*
* @param {String} locale
* @param {String} digit - Character of a single standard digit . It may be "0" ~ "9" (digits),
* "," (group separator), "." (decimal separator) or "-" (minus sign).
* @returns {String}
*
* @throws If `digit` is not a valid standard digit.
*/
function toLocaleDigit(locale, digit) {
const index = _.indexOf(STANDARD_DIGITS, digit);
if (index < 0) {
throw new Error(`"${digit}" must be in ${JSON.stringify(STANDARD_DIGITS)}`);
}
return getLocaleDigits(locale)[index];
}

/**
* Gets the standard digit corresponding to a locale digit.
*
* @param {String} locale
* @param {String} localeDigit - Character of a single locale digit. It may be **the localized version** of
* "0" ~ "9" (digits), "," (group separator), "." (decimal separator) or "-" (minus sign).
* @returns {String}
*
* @throws If `localeDigit` is not a valid locale digit.
*/
function fromLocaleDigit(locale, localeDigit) {
const index = _.indexOf(getLocaleDigits(locale), localeDigit);
if (index < 0) {
throw new Error(`"${localeDigit}" must be in ${JSON.stringify(getLocaleDigits(locale))}`);
}
return STANDARD_DIGITS[index];
}

export {
toLocaleDigit,
fromLocaleDigit,
};
12 changes: 12 additions & 0 deletions src/libs/NumberFormatUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
function format(locale, number, options) {
return new Intl.NumberFormat(locale, options).format(number);
}

function formatToParts(locale, number, options) {
return new Intl.NumberFormat(locale, options).formatToParts(number);
}

export {
format,
formatToParts,
};
5 changes: 0 additions & 5 deletions src/libs/numberFormat/index.js

This file was deleted.

15 changes: 0 additions & 15 deletions src/libs/numberFormat/index.native.js

This file was deleted.

40 changes: 32 additions & 8 deletions src/pages/iou/steps/IOUAmountPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import lodashGet from 'lodash/get';
import _ from 'underscore';
import ONYXKEYS from '../../../ONYXKEYS';
import styles from '../../../styles/styles';
import BigNumberPad from '../../../components/BigNumberPad';
Expand Down Expand Up @@ -175,17 +176,40 @@ class IOUAmountPage extends React.Component {
* Update amount on amount change
* Validate new amount with decimal number regex up to 6 digits and 2 decimal digit
*
* @param {String} amount
* @param {String} text - Changed text from user input
*/
updateAmount(amount) {
if (!this.validateAmount(amount)) {
return;
}
updateAmount(text) {
this.setState((prevState) => {
const amount = this.replaceAllDigits(text, this.props.fromLocaleDigit);
return this.validateAmount(amount)
? {amount: this.stripCommaFromAmount(amount)}
: prevState;
});
}

this.setState({amount: this.stripCommaFromAmount(amount)});
/**
* Replaces each character by calling `convertFn`. If `convertFn` throws an error, then
* the original character will be preserved.
*
* @param {String} text
* @param {Function} convertFn - `this.props.fromLocaleDigit` or `this.props.toLocaleDigit`
* @returns {String}
*/
replaceAllDigits(text, convertFn) {
return _.chain([...text])
.map((char) => {
try {
return convertFn(char);
} catch {
return char;
}
})
.join('')
.value();
}

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

Choose a reason for hiding this comment

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

Can't we just use, numberFormat prop directly for the whole input value?

Copy link
Contributor Author

@songlang1994 songlang1994 Jan 15, 2022

Choose a reason for hiding this comment

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

Unfortunately we can't. Because the amount might not be a valid number yet while inputing (e.g. "12.").

If we do need to use numberFormat the code will be

get formattedInputingAmount() {
    if (!this.state.amount) {
        return '';
    }

    const endsWithDecimalPoint = this.state.amount.endsWith('.');
    if (endsWithDecimalPoint) {
        const localized = this.props.numberFormat(this.state.amount, {useGrouping: false, minimumFractionDigits: 1});
        return localized.slice(0, -1);
    }

    const fraction = this.state.amount.split('.')[1];
    const fractionDigits = fraction ? fraction.length : 0;
    return this.props.numberFormat(this.state.amount, {useGrouping: false, minimumFractionDigits: fractionDigits});
}

Copy link
Member

Choose a reason for hiding this comment

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

Ok. What is more performant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no performance issue here since the max length of amount is just 8. But replaceAllDigits here is more readable.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds, good. I will give it a go.

return (
<>
<View style={[
Expand All @@ -209,8 +233,8 @@ class IOUAmountPage extends React.Component {
textStyle={styles.iouAmountText}
onChangeText={this.updateAmount}
ref={el => this.textInput = el}
value={this.state.amount}
placeholder="0"
value={formattedAmount}
placeholder={this.props.numberFormat(0)}
songlang1994 marked this conversation as resolved.
Show resolved Hide resolved
keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
/>
</View>
Expand Down
10 changes: 10 additions & 0 deletions src/setup/platformSetup/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@ import PushNotification from '../../libs/Notification/PushNotification';
import * as Report from '../../libs/actions/Report';
import Performance from '../../libs/Performance';

// we only need polyfills for Mobile.
import '@formatjs/intl-getcanonicallocales/polyfill';
import '@formatjs/intl-locale/polyfill';
import '@formatjs/intl-pluralrules/polyfill';
import '@formatjs/intl-numberformat/polyfill';

// Load en & es Locale data
import '@formatjs/intl-numberformat/locale-data/en';
import '@formatjs/intl-numberformat/locale-data/es';

export default function () {
/*
* Register callbacks for push notifications.
Expand Down