Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ function AmountField({
const [splitDraftTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`);
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
const amountInputRef = useRef<BaseTextInputRef | null>(null);
const currencyButtonRef = useRef<View>(null);
const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null);

const transactionSlice = useTransactionSelector(transactionID, amountSliceSelector);
Expand Down Expand Up @@ -347,6 +348,7 @@ function AmountField({
headerText={translate('common.selectCurrency')}
value={effectiveCurrency}
onInputChange={updateCurrency}
launcherRef={currencyButtonRef}
Comment thread
thelullabyy marked this conversation as resolved.
Resolved
/>
{!isAmountFieldDisabled ? (
<View style={[styles.mh4, styles.mv2, styles.flexRow, styles.gap2]}>
Expand All @@ -368,6 +370,7 @@ function AmountField({
shouldShowCurrencyButton={shouldShowAmountButtons}
shouldShowBigNumberPad={false}
onCurrencyButtonPress={showCurrencyPicker}
currencyButtonRef={currencyButtonRef}
onFocus={() => {
setIsAmountInputFocused(true);
}}
Expand Down
7 changes: 6 additions & 1 deletion src/components/NumberWithSymbolForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import shouldIgnoreSelectionWhenUpdatedManually from '@libs/shouldIgnoreSelectio

import CONST from '@src/CONST';

import type {ForwardedRef} from 'react';
import type {ComponentRef, ForwardedRef} from 'react';
import type {KeyboardTypeOptions, NativeSyntheticEvent, StyleProp, TextStyle, ViewStyle} from 'react-native';

import {useIsFocused} from '@react-navigation/native';
Expand Down Expand Up @@ -114,6 +114,9 @@ type NumberWithSymbolFormProps = {

onCurrencyButtonPress?: () => void;

/** Ref to the currency selection button, e.g. so a picker it opens can return focus to it. `displayAsTextInput` mode only. */
currencyButtonRef?: ForwardedRef<ComponentRef<typeof View>>;

/**
* Label on the trailing dropdown button (e.g. currency code). When set, used instead of `currency` so the same control can show a unit or other suffix.
*/
Expand Down Expand Up @@ -201,6 +204,7 @@ function NumberWithSymbolForm({
shouldShowCurrencyButton = false,
leadingRightHandSideComponent,
onCurrencyButtonPress,
currencyButtonRef,
currencyButtonLabel,
currencyButtonAccessibilityLabel,
shouldUseBorderlessButtons = false,
Expand Down Expand Up @@ -518,6 +522,7 @@ function NumberWithSymbolForm({
)}
{shouldShowCurrencyButton && !!currencyOrUnitButtonText && (
<Button
ref={currencyButtonRef}
size={CONST.BUTTON_SIZE.SMALL}
innerStyles={shouldUseBorderlessButtons ? styles.bgTransparent : undefined}
// `innerStyles` never reaches the hover style `Button` layers on top, so the pill the
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import CurrencySelectionList from '@components/CurrencySelectionList';
import type {CurrencyListItem} from '@components/CurrencySelectionList/types';
import type FocusTrapForModalProps from '@components/FocusTrap/FocusTrapForModal/FocusTrapForModalProps';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Modal from '@components/Modal';
import ScreenWrapper from '@components/ScreenWrapper';
Expand All @@ -25,9 +26,12 @@ type CurrencyPickerProps = {
onInputChange?: (value: string, key?: string) => void;

excludeCurrencies?: string[];

/** The button that opens the picker. When passed, focus returns to it after the picker closes. */
launcherRef?: FocusTrapForModalProps['launcherRef'];
};

function IOURequestStepCurrencyModal({isPickerVisible, hidePickerModal, headerText, value, excludeCurrencies, onInputChange = () => {}}: CurrencyPickerProps) {
function IOURequestStepCurrencyModal({isPickerVisible, hidePickerModal, headerText, value, excludeCurrencies, launcherRef, onInputChange = () => {}}: CurrencyPickerProps) {
const {translate} = useLocalize();
const [recentlyUsedCurrencies] = useOnyx(ONYXKEYS.RECENTLY_USED_CURRENCIES);
const styles = useThemeStyles();
Expand All @@ -44,6 +48,8 @@ function IOURequestStepCurrencyModal({isPickerVisible, hidePickerModal, headerTe
onClose={hidePickerModal}
onModalHide={hidePickerModal}
shouldEnableNewFocusManagement
shouldReturnFocus={!!launcherRef}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

src/pages/iou/request/step/IOURequestStepCurrencyModal.tsx:57

🟢 shouldReturnFocus used to be false on this modal and is now true, so focus also comes back after a mouse or touch selection. The currency button's onMouseDown calls preventDefault, so if the amount input had focus, the input is still document.activeElement when the trap activates.

The active element takes priority over launcherRef, so focus goes back to the amount input after a currency is picked. On Android mWeb, that programmatic focus can pop the soft keyboard back up right after the picker closes.

Can you check this on Android and iOS mWeb and add videos ? Every platform section in the PR is empty except MacOS.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@thelullabyy 🟠 This thread got resolved without a reply, and the Android mWeb section in the PR is still empty.

Can you add an Android mWeb video for the touch path: focus the amount input, tap the currency button, pick a currency?

  • If focus goes back to the amount input and the keyboard pops up, shouldReturnFocus needs to stay off for non-keyboard selections.
  • If it lands on the currency button with no keyboard, just reply here with the video.

👁️ Written by AI, reviewed by Kevin.

launcherRef={launcherRef}
onBackdropPress={Navigation.dismissModal}
shouldUseModalPaddingStyle={false}
shouldHandleNavigationBack
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,64 @@ import {fireEvent, render, screen} from '@testing-library/react-native';

import ConfirmationFieldsProvider from '@components/MoneyRequestConfirmationFields/Provider';
import AmountField from '@components/MoneyRequestConfirmationList/sections/AmountField';
import type {NumberWithSymbolFormProps} from '@components/NumberWithSymbolForm';

import {setMoneyRequestAmount, setMoneyRequestCurrency} from '@libs/actions/IOU/MoneyRequest';

import type IOURequestStepCurrencyModal from '@pages/iou/request/step/IOURequestStepCurrencyModal';

import CONST from '@src/CONST';

import React from 'react';

let mockCurrencyButtonRef: NumberWithSymbolFormProps['currencyButtonRef'];
let mockLauncherRef: React.ComponentProps<typeof IOURequestStepCurrencyModal>['launcherRef'];
jest.mock('@components/NumberWithSymbolForm', () => {
const {Pressable, Text} =
jest.requireActual<Record<'Pressable' | 'Text', React.ComponentType<{children?: React.ReactNode; onPress?: () => void; accessibilityRole?: 'button'}>>>('react-native');
return ({onInputChange, errorText}: {onInputChange: (amount: string) => void; errorText?: string}) => (
<>
{!!errorText && <Text>{errorText}</Text>}
<Pressable
accessibilityRole="button"
onPress={() => onInputChange('0')}
>
<Text>Enter zero</Text>
</Pressable>
<Pressable
accessibilityRole="button"
onPress={() => onInputChange('1')}
>
<Text>Enter one</Text>
</Pressable>
</>
);
return ({
onInputChange,
errorText,
currencyButtonRef,
}: {
onInputChange: (amount: string) => void;
errorText?: string;
currencyButtonRef?: NumberWithSymbolFormProps['currencyButtonRef'];
}) => {
mockCurrencyButtonRef = currencyButtonRef;
return (
<>
{!!errorText && <Text>{errorText}</Text>}
<Pressable
accessibilityRole="button"
onPress={() => onInputChange('0')}
>
<Text>Enter zero</Text>
</Pressable>
<Pressable
accessibilityRole="button"
onPress={() => onInputChange('1')}
>
<Text>Enter one</Text>
</Pressable>
</>
);
};
});
jest.mock('@pages/iou/request/step/IOURequestStepCurrencyModal', () => {
const {Pressable, Text} =
jest.requireActual<Record<'Pressable' | 'Text', React.ComponentType<{children?: React.ReactNode; onPress?: () => void; accessibilityRole?: 'button'}>>>('react-native');
return ({onInputChange}: {onInputChange: (currency: string) => void}) => (
<Pressable
accessibilityRole="button"
onPress={() => onInputChange('EUR')}
>
<Text>Pick EUR</Text>
</Pressable>
);
return ({onInputChange, launcherRef}: {onInputChange: (currency: string) => void; launcherRef?: React.ComponentProps<typeof IOURequestStepCurrencyModal>['launcherRef']}) => {
mockLauncherRef = launcherRef;
return (
<Pressable
accessibilityRole="button"
onPress={() => onInputChange('EUR')}
>
<Text>Pick EUR</Text>
</Pressable>
);
};
});
jest.mock('@components/MenuItemWithTopDescription', () => {
const {Text} = jest.requireActual<Record<'Text', React.ComponentType<{children?: React.ReactNode}>>>('react-native');
Expand Down Expand Up @@ -252,6 +271,34 @@ describe('AmountField currency change', () => {
});
});

describe('AmountField currency picker focus return', () => {
beforeEach(() => {
mockTransactionSlice = {transactionID: '1', amount: 0, currency: 'USD', isAmountMissing: false};
mockCurrencyButtonRef = undefined;
mockLauncherRef = undefined;
});

it('hands the currency button ref to the picker so focus returns to the button on close', () => {
// Given a manual expense whose amount field shows the currency button
// When the amount field renders
render(
<ConfirmationFieldsProvider
transactionID="1"
reportID="2"
action={CONST.IOU.ACTION.CREATE}
iouType={CONST.IOU.TYPE.SUBMIT}
>
<AmountField {...amountFieldProps} />
</ConfirmationFieldsProvider>,
);

// Then the picker gets the same ref the currency button is attached to, so closing the picker returns keyboard
// focus to that button instead of dropping the user at the top of the form
expect(mockLauncherRef).toBeDefined();
expect(mockLauncherRef).toBe(mockCurrencyButtonRef);
});
});

describe('AmountField while the receipt is scanning', () => {
beforeEach(() => {
mockTransactionSlice = {transactionID: '1', amount: 0, currency: 'USD', isAmountMissing: false, isAmountSet: true, isScanning: true};
Expand Down
Loading