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

Reusable and stylable money request amount input #40390

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: 2 additions & 0 deletions src/components/AmountForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ function AmountForm(
}}
onKeyPress={textInputKeyPress}
isCurrencyPressable={isCurrencyPressable}
style={[styles.iouAmountTextInput]}
containerStyle={[styles.iouAmountTextInputContainer]}
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
/>
Expand Down
11 changes: 6 additions & 5 deletions src/components/AmountTextInput.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import type {ForwardedRef} from 'react';
import type {NativeSyntheticEvent, StyleProp, TextInputKeyPressEventData, TextInputSelectionChangeEventData, TextStyle, ViewStyle} from 'react-native';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
import type {TextSelection} from './Composer/types';
import TextInput from './TextInput';
Expand Down Expand Up @@ -31,21 +30,23 @@ type AmountTextInputProps = {

/** Function to call to handle key presses in the text input */
onKeyPress?: (event: NativeSyntheticEvent<KeyboardEvent>) => void;

/** Style for the TextInput container */
containerStyle?: StyleProp<ViewStyle>;
} & Pick<BaseTextInputProps, 'autoFocus'>;

function AmountTextInput(
{formattedAmount, onChangeAmount, placeholder, selection, onSelectionChange, style, touchableInputWrapperStyle, onKeyPress, ...rest}: AmountTextInputProps,
{formattedAmount, onChangeAmount, placeholder, selection, onSelectionChange, style, touchableInputWrapperStyle, onKeyPress, containerStyle, ...rest}: AmountTextInputProps,
ref: ForwardedRef<BaseTextInputRef>,
) {
const styles = useThemeStyles();
return (
<TextInput
disableKeyboard
autoGrow
hideFocusedState
shouldInterceptSwipe
inputStyle={[styles.iouAmountTextInput, styles.p0, styles.noLeftBorderRadius, styles.noRightBorderRadius, style]}
textInputContainerStyles={[styles.borderNone, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
inputStyle={style}
textInputContainerStyles={containerStyle}
onChangeText={onChangeAmount}
ref={ref}
value={formattedAmount}
Expand Down
251 changes: 251 additions & 0 deletions src/components/MoneyRequestAmountInput.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
import type {ForwardedRef} from 'react';
import React, {useCallback, useEffect, useImperativeHandle, useRef, useState} from 'react';
import type {NativeSyntheticEvent, StyleProp, TextInputSelectionChangeEventData, TextStyle, ViewStyle} from 'react-native';
import useLocalize from '@hooks/useLocalize';
import * as Browser from '@libs/Browser';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import getOperatingSystem from '@libs/getOperatingSystem';
import * as MoneyRequestUtils from '@libs/MoneyRequestUtils';
import CONST from '@src/CONST';
import type {BaseTextInputRef} from './TextInput/BaseTextInput/types';
import TextInputWithCurrencySymbol from './TextInputWithCurrencySymbol';

type CurrentMoney = {amount: string; currency: string};

type MoneyRequestAmountInputRef = {
setNewAmount: (amountValue: string) => void;
changeSelection: (newSelection: Selection) => void;
changeAmount: (newAmount: string) => void;
getAmount: () => string;
getSelection: () => Selection;
};

type MoneyRequestAmountInputProps = {
/** IOU amount saved in Onyx */
amount?: number;

/** Currency chosen by user or saved in Onyx */
currency?: string;

/** Whether the currency symbol is pressable */
isCurrencyPressable?: boolean;

/** Fired when back button pressed, navigates to currency selection page */
onCurrencyButtonPress?: () => void;

/** Function to call when the amount changes */
onAmountChange?: (amount: string) => void;

/** Whether to update the selection */
shouldUpdateSelection?: boolean;

/** Style for the input */
inputStyle?: StyleProp<TextStyle>;

/** Style for the container */
ZhenjaHorbach marked this conversation as resolved.
Show resolved Hide resolved
containerStyle?: StyleProp<ViewStyle>;

/** Reference to moneyRequestAmountInputRef */
moneyRequestAmountInputRef?: ForwardedRef<MoneyRequestAmountInputRef>;

/** Character to be shown before the amount */
prefixCharacter?: string;

/** Whether to hide the currency symbol */
hideCurrencySymbol?: boolean;

/** Style for the prefix */
prefixStyle?: StyleProp<TextStyle>;

/** Style for the prefix container */
prefixContainerStyle?: StyleProp<ViewStyle>;

/** Style for the touchable input wrapper */
touchableInputWrapperStyle?: StyleProp<ViewStyle>;
};

type Selection = {
start: number;
end: number;
};

/**
* Returns the new selection object based on the updated amount's length
*/
const getNewSelection = (oldSelection: Selection, prevLength: number, newLength: number): Selection => {
const cursorPosition = oldSelection.end + (newLength - prevLength);
return {start: cursorPosition, end: cursorPosition};
};

function MoneyRequestAmountInput(
{
amount = 0,
currency = CONST.CURRENCY.USD,
isCurrencyPressable = true,
onCurrencyButtonPress,
onAmountChange,
prefixCharacter = '',
hideCurrencySymbol = false,
shouldUpdateSelection = true,
moneyRequestAmountInputRef,
...props
}: MoneyRequestAmountInputProps,
forwardedRef: ForwardedRef<BaseTextInputRef>,
) {
const {toLocaleDigit, numberFormat} = useLocalize();

const textInput = useRef<BaseTextInputRef | null>(null);

const decimals = CurrencyUtils.getCurrencyDecimals(currency);
const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : '';

const [currentAmount, setCurrentAmount] = useState(selectedAmountAsString);

const [selection, setSelection] = useState({
start: selectedAmountAsString.length,
end: selectedAmountAsString.length,
});

const forwardDeletePressedRef = useRef(false);

/**
* Sets the selection and the amount accordingly to the value passed to the input
* @param {String} newAmount - Changed amount from user input
*/
const setNewAmount = useCallback(
(newAmount: string) => {
// Remove spaces from the newAmount value because Safari on iOS adds spaces when pasting a copied value
// More info: https://github.com/Expensify/App/issues/16974
const newAmountWithoutSpaces = MoneyRequestUtils.stripSpacesFromAmount(newAmount);
// Use a shallow copy of selection to trigger setSelection
// More info: https://github.com/Expensify/App/issues/16385
if (!MoneyRequestUtils.validateAmount(newAmountWithoutSpaces, decimals)) {
setSelection((prevSelection) => ({...prevSelection}));
return;
}

// setCurrentAmount contains another setState(setSelection) making it error-prone since it is leading to setSelection being called twice for a single setCurrentAmount call. This solution introducing the hasSelectionBeenSet flag was chosen for its simplicity and lower risk of future errors https://github.com/Expensify/App/issues/23300#issuecomment-1766314724.

let hasSelectionBeenSet = false;
setCurrentAmount((prevAmount) => {
const strippedAmount = MoneyRequestUtils.stripCommaFromAmount(newAmountWithoutSpaces);
const isForwardDelete = prevAmount.length > strippedAmount.length && forwardDeletePressedRef.current;
if (!hasSelectionBeenSet) {
hasSelectionBeenSet = true;
setSelection((prevSelection) => getNewSelection(prevSelection, isForwardDelete ? strippedAmount.length : prevAmount.length, strippedAmount.length));
}
onAmountChange?.(strippedAmount);
return strippedAmount;
});
},
[decimals, onAmountChange],
);

useImperativeHandle(moneyRequestAmountInputRef, () => ({
setNewAmount(amountValue: string) {
setNewAmount(amountValue);
},
changeSelection(newSelection: Selection) {
setSelection(newSelection);
},
changeAmount(newAmount: string) {
setCurrentAmount(newAmount);
},
getAmount() {
return currentAmount;
},
getSelection() {
return selection;
},
}));

useEffect(() => {
if (!currency || typeof amount !== 'number') {
return;
}
const frontendAmount = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : '';
setCurrentAmount(frontendAmount);
setSelection({
start: frontendAmount.length,
end: frontendAmount.length,
});
// we want to re-initialize the state only when the amount changes
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [amount]);

// Modifies the amount to match the decimals for changed currency.
useEffect(() => {
// If the changed currency supports decimals, we can return
if (MoneyRequestUtils.validateAmount(currentAmount, decimals)) {
return;
}

// If the changed currency doesn't support decimals, we can strip the decimals
setNewAmount(MoneyRequestUtils.stripDecimalsFromAmount(currentAmount));

// we want to update only when decimals change (setNewAmount also changes when decimals change).
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [setNewAmount]);

/**
* Input handler to check for a forward-delete key (or keyboard shortcut) press.
*/
const textInputKeyPress = ({nativeEvent}: NativeSyntheticEvent<KeyboardEvent>) => {
const key = nativeEvent?.key.toLowerCase();
if (Browser.isMobileSafari() && key === CONST.PLATFORM_SPECIFIC_KEYS.CTRL.DEFAULT) {
// Optimistically anticipate forward-delete on iOS Safari (in cases where the Mac Accessiblity keyboard is being
// used for input). If the Control-D shortcut doesn't get sent, the ref will still be reset on the next key press.
forwardDeletePressedRef.current = true;
return;
}
// Control-D on Mac is a keyboard shortcut for forward-delete. See https://support.apple.com/en-us/HT201236 for Mac keyboard shortcuts.
// Also check for the keyboard shortcut on iOS in cases where a hardware keyboard may be connected to the device.
const operatingSystem = getOperatingSystem();
forwardDeletePressedRef.current = key === 'delete' || ((operatingSystem === CONST.OS.MAC_OS || operatingSystem === CONST.OS.IOS) && nativeEvent?.ctrlKey && key === 'd');
};

const formattedAmount = MoneyRequestUtils.replaceAllDigits(currentAmount, toLocaleDigit);

return (
<TextInputWithCurrencySymbol
formattedAmount={formattedAmount}
onChangeAmount={setNewAmount}
onCurrencyButtonPress={onCurrencyButtonPress}
placeholder={numberFormat(0)}
ref={(ref) => {
if (typeof forwardedRef === 'function') {
forwardedRef(ref);
} else if (forwardedRef?.current) {
// eslint-disable-next-line no-param-reassign
forwardedRef.current = ref;
}
textInput.current = ref;
}}
selectedCurrencyCode={currency}
selection={selection}
onSelectionChange={(e: NativeSyntheticEvent<TextInputSelectionChangeEventData>) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from this issue #41762 , we are encountering an edge case on Android. When we select all the number and then add a number, the cursor is misplaced and appears at the start of the number. The reason for this is that on Android, when we select all the text and add a number, the selection update from onSelectionChange is triggered earlier than the manual selection update. As a result, the prevSelection already contains the new selection, leading to an incorrect selection calculation more context on the proposal #41762 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, this caused the following issue: #50886
Detailed RCA can be found in the description of this PR: #51306 and in this comment: #50886 (comment)

if (!shouldUpdateSelection) {
return;
}
const maxSelection = formattedAmount.length;
const start = Math.min(e.nativeEvent.selection.start, maxSelection);
const end = Math.min(e.nativeEvent.selection.end, maxSelection);
setSelection({start, end});
}}
onKeyPress={textInputKeyPress}
hideCurrencySymbol={hideCurrencySymbol}
prefixCharacter={prefixCharacter}
isCurrencyPressable={isCurrencyPressable}
style={props.inputStyle}
containerStyle={props.containerStyle}
prefixStyle={props.prefixStyle}
prefixContainerStyle={props.prefixContainerStyle}
touchableInputWrapperStyle={props.touchableInputWrapperStyle}
/>
);
}

MoneyRequestAmountInput.displayName = 'MoneyRequestAmountInput';

export default React.forwardRef(MoneyRequestAmountInput);
export type {CurrentMoney, MoneyRequestAmountInputRef};
29 changes: 8 additions & 21 deletions src/components/TextInput/BaseTextInput/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ function BaseTextInput(
prefixCharacter = '',
inputID,
isMarkdownEnabled = false,
prefixContainerStyle = [],
prefixStyle = [],
...props
}: BaseTextInputProps,
ref: ForwardedRef<BaseTextInputRef>,
Expand Down Expand Up @@ -237,21 +239,6 @@ function BaseTextInput(
setPasswordHidden((prevPasswordHidden) => !prevPasswordHidden);
}, []);

// When adding a new prefix character, adjust this method to add expected character width.
// This is because character width isn't known before it's rendered to the screen, and once it's rendered,
// it's too late to calculate it's width because the change in padding would cause a visible jump.
// Some characters are wider than the others when rendered, e.g. '@' vs '#'. Chosen font-family and font-size
// also have an impact on the width of the character, but as long as there's only one font-family and one font-size,
// this method will produce reliable results.
const getCharacterPadding = (prefix: string): number => {
switch (prefix) {
case CONST.POLICY.ROOM_PREFIX:
return 10;
default:
throw new Error(`Prefix ${prefix} has no padding assigned.`);
}
};

const hasLabel = Boolean(label?.length);
const isReadOnly = inputProps.readOnly ?? inputProps.disabled;
// Disabling this line for safeness as nullish coalescing works only if the value is undefined or null, and errorText can be an empty string
Expand All @@ -275,6 +262,9 @@ function BaseTextInput(
role={CONST.ROLE.PRESENTATION}
onPress={onPress}
tabIndex={-1}
// When autoGrowHeight is true we calculate the width for the textInput, so it will break lines properly
// or if multiline is not supplied we calculate the textinput height, using onLayout.
onLayout={onLayout}
accessibilityLabel={label}
style={[
autoGrowHeight && styles.autoGrowHeightInputContainer(textInputHeight, variables.componentSizeLarge, typeof maxHeight === 'number' ? maxHeight : 0),
Expand All @@ -283,9 +273,6 @@ function BaseTextInput(
]}
>
<View
// When autoGrowHeight is true we calculate the width for the textInput, so it will break lines properly
// or if multiline is not supplied we calculate the textinput height, using onLayout.
onLayout={onLayout}
style={[
newTextInputContainerStyles,

Expand Down Expand Up @@ -319,10 +306,10 @@ function BaseTextInput(
</View>
)}
{!!prefixCharacter && (
<View style={styles.textInputPrefixWrapper}>
<View style={[styles.textInputPrefixWrapper, prefixContainerStyle]}>
<Text
tabIndex={-1}
style={[styles.textInputPrefix, !hasLabel && styles.pv0, styles.pointerEventsNone]}
style={[styles.textInputPrefix, !hasLabel && styles.pv0, styles.pointerEventsNone, prefixStyle]}
dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
>
{prefixCharacter}
Expand Down Expand Up @@ -352,7 +339,7 @@ function BaseTextInput(
styles.w100,
inputStyle,
(!hasLabel || isMultiline) && styles.pv0,
!!prefixCharacter && StyleUtils.getPaddingLeft(getCharacterPadding(prefixCharacter) + styles.pl1.paddingLeft),
!!prefixCharacter && StyleUtils.getPaddingLeft(StyleUtils.getCharacterPadding(prefixCharacter) + styles.pl1.paddingLeft),
inputProps.secureTextEntry && styles.secureInput,

!isMultiline && {height, lineHeight: undefined},
Expand Down
Loading
Loading