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: handle forward-delete properly in the amount text input #25815

Merged
merged 10 commits into from
Aug 29, 2023
5 changes: 5 additions & 0 deletions src/components/AmountTextInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,16 @@ const propTypes = {

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

/** Function to call to handle key presses in the text input */
onKeyPress: PropTypes.func,
};

const defaultProps = {
forwardedRef: undefined,
selection: undefined,
onSelectionChange: () => {},
onKeyPress: () => {},
};

function AmountTextInput(props) {
Expand All @@ -50,6 +54,7 @@ function AmountTextInput(props) {
selection={props.selection}
onSelectionChange={props.onSelectionChange}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.TEXT}
onKeyPress={props.onKeyPress}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,47 +1,10 @@
import React, {useState, useEffect} from 'react';
import PropTypes from 'prop-types';
import AmountTextInput from './AmountTextInput';
import CurrencySymbolButton from './CurrencySymbolButton';
import * as CurrencyUtils from '../libs/CurrencyUtils';
import useLocalize from '../hooks/useLocalize';
import * as MoneyRequestUtils from '../libs/MoneyRequestUtils';

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,

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

/** Selection Object */
selection: PropTypes.shape({
start: PropTypes.number,
end: PropTypes.number,
}),

/** Function to call when selection in text input is changed */
onSelectionChange: PropTypes.func,
};

const defaultProps = {
forwardedRef: undefined,
onChangeAmount: () => {},
onCurrencyButtonPress: () => {},
selection: undefined,
onSelectionChange: () => {},
};
import AmountTextInput from '../AmountTextInput';
import CurrencySymbolButton from '../CurrencySymbolButton';
import * as CurrencyUtils from '../../libs/CurrencyUtils';
import useLocalize from '../../hooks/useLocalize';
import * as MoneyRequestUtils from '../../libs/MoneyRequestUtils';
import * as textInputWithCurrencySymbolPropTypes from './textInputWithCurrencySymbolPropTypes';

function TextInputWithCurrencySymbol(props) {
const {fromLocaleDigit} = useLocalize();
Expand Down Expand Up @@ -85,6 +48,7 @@ function TextInputWithCurrencySymbol(props) {
}
props.onSelectionChange(e);
}}
onKeyPress={props.onKeyPress}
/>
);

Expand All @@ -105,8 +69,8 @@ function TextInputWithCurrencySymbol(props) {
);
}

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

export default React.forwardRef((props, ref) => (
Expand Down
72 changes: 72 additions & 0 deletions src/components/TextInputWithCurrencySymbol/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import React from 'react';
import AmountTextInput from '../AmountTextInput';
import CurrencySymbolButton from '../CurrencySymbolButton';
import * as CurrencyUtils from '../../libs/CurrencyUtils';
import useLocalize from '../../hooks/useLocalize';
import * as MoneyRequestUtils from '../../libs/MoneyRequestUtils';
import * as textInputWithCurrencySymbolPropTypes from './textInputWithCurrencySymbolPropTypes';

function TextInputWithCurrencySymbol(props) {
const {fromLocaleDigit} = useLocalize();
const currencySymbol = CurrencyUtils.getLocalizedCurrencySymbol(props.selectedCurrencyCode);
const isCurrencySymbolLTR = CurrencyUtils.isCurrencySymbolLTR(props.selectedCurrencyCode);

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

/**
* Set a new amount value properly formatted
*
* @param {String} text - Changed text from user input
*/
const setFormattedAmount = (text) => {
const newAmount = MoneyRequestUtils.addLeadingZero(MoneyRequestUtils.replaceAllDigits(text, fromLocaleDigit));
props.onChangeAmount(newAmount);
};

const amountTextInput = (
<AmountTextInput
formattedAmount={props.formattedAmount}
onChangeAmount={setFormattedAmount}
placeholder={props.placeholder}
ref={props.forwardedRef}
selection={props.selection}
onSelectionChange={(e) => {
props.onSelectionChange(e);
}}
onKeyPress={props.onKeyPress}
/>
);

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

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

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

export default React.forwardRef((props, ref) => (
<TextInputWithCurrencySymbol
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
forwardedRef={ref}
/>
));
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import React from 'react';
import PropTypes from 'prop-types';

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,

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

/** Selection Object */
selection: PropTypes.shape({
start: PropTypes.number,
end: PropTypes.number,
}),

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

/** Function to call to handle key presses in the text input */
onKeyPress: PropTypes.func,
};

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

export {propTypes, defaultProps};
24 changes: 23 additions & 1 deletion src/pages/iou/steps/MoneyRequestAmountForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import * as DeviceCapabilities from '../../../libs/DeviceCapabilities';
import TextInputWithCurrencySymbol from '../../../components/TextInputWithCurrencySymbol';
import useLocalize from '../../../hooks/useLocalize';
import CONST from '../../../CONST';
import getOperatingSystem from '../../../libs/getOperatingSystem';
import * as Browser from '../../../libs/Browser';

const propTypes = {
/** IOU amount saved in Onyx */
Expand Down Expand Up @@ -72,6 +74,8 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu
end: selectedAmountAsString.length,
});

const forwardDeletePressedRef = useRef(false);

/**
* Event occurs when a user presses a mouse button over an DOM element.
*
Expand Down Expand Up @@ -121,7 +125,8 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu
return;
}
setCurrentAmount((prevAmount) => {
setSelection((prevSelection) => getNewSelection(prevSelection, prevAmount.length, newAmountWithoutSpaces.length));
const isForwardDelete = prevAmount.length > newAmountWithoutSpaces.length && forwardDeletePressedRef.current;
setSelection((prevSelection) => getNewSelection(prevSelection, isForwardDelete ? newAmountWithoutSpaces.length : prevAmount.length, newAmountWithoutSpaces.length));
return MoneyRequestUtils.stripCommaFromAmount(newAmountWithoutSpaces);
});
};
Expand Down Expand Up @@ -171,6 +176,22 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu
onSubmitButtonPress(currentAmount);
}, [onSubmitButtonPress, currentAmount]);

/**
* Input handler to check for a forward-delete key (or keyboard shortcut) press.
*/
const textInputKeyPress = ({nativeEvent}) => {
const key = nativeEvent.key.toLowerCase();
if (Browser.isMobileSafari() && key === 'control') {
Copy link
Contributor

Choose a reason for hiding this comment

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

'control' -> CONST.PLATFORM_SPECIFIC_KEYS.CTRL.DEFAULT ?

Copy link
Contributor Author

@akinwale akinwale Aug 25, 2023

Choose a reason for hiding this comment

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

Makes sense. I've seen these key constants in the past, but then I forgot all about them. It's fixed now.

// 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.
forwardDeletePressedRef.current = key === 'delete' || (_.contains([CONST.OS.MAC_OS, CONST.OS.IOS], getOperatingSystem()) && nativeEvent.ctrlKey && key === 'd');
};

const formattedAmount = MoneyRequestUtils.replaceAllDigits(currentAmount, toLocaleDigit);
const buttonText = isEditing ? translate('common.save') : translate('common.next');

Expand Down Expand Up @@ -203,6 +224,7 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu
}
setSelection(e.nativeEvent.selection);
}}
onKeyPress={textInputKeyPress}
/>
</View>
<View
Expand Down