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.

Sign up for GitHub

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

[CP Staging] fix: store selected currency in URL query temporarily #32851

Merged
merged 5 commits into from
Dec 12, 2023
Merged

[CP Staging] fix: store selected currency in URL query temporarily #32851

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd05a4d
fix: store selected currency in URL query temporarily
sangar-1028 Dec 11, 2023
de583c8
fix: navigate when selecting currency
sangar-1028 Dec 11, 2023
83689f9
Merge branch 'main' into fix/issue-32826
sangar-1028 Dec 11, 2023
e70f1e9
fix: use || instead of ?? operator
sangar-1028 Dec 11, 2023
f32d7d8
fix: don't store currency data only for confirm page
sangar-1028 Dec 11, 2023
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
6 changes: 4 additions & 2 deletions src/pages/iou/request/step/IOURequestStepAmount.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,17 @@ const defaultProps = {
function IOURequestStepAmount({
report,
route: {
params: {iouType, reportID, transactionID, backTo},
params: {iouType, reportID, transactionID, backTo, currency: selectedCurrency},
},
transaction,
transaction: {currency},
transaction: {currency: originalCurrency},
}) {
const {translate} = useLocalize();
const textInput = useRef(null);
const focusTimeoutRef = useRef(null);
const iouRequestType = getRequestType(transaction);
/* eslint-disable es/no-nullish-coalescing-operators */
const currency = selectedCurrency ?? originalCurrency;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should disable this lint rule

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@luacmartins Do you suggest to use || operator?

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 replaced the operator. Will you check again?


useFocusEffect(
useCallback(() => {
Expand Down
12 changes: 7 additions & 5 deletions src/pages/iou/request/step/IOURequestStepCurrency.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import useLocalize from '@hooks/useLocalize';
import compose from '@libs/compose';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import Navigation from '@libs/Navigation/Navigation';
import * as IOU from '@userActions/IOU';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES, {getUrlWithBackToParam} from '@src/ROUTES';
import IOURequestStepRoutePropTypes from './IOURequestStepRoutePropTypes';
Expand Down Expand Up @@ -59,14 +58,18 @@ function IOURequestStepCurrency({
const [searchValue, setSearchValue] = useState('');
const optionsSelectorRef = useRef();

const navigateBack = () => {
const navigateBack = (selectedCurrency = undefined) => {
// If the currency selection was done from the confirmation step (eg. + > request money > manual > confirm > amount > currency)
// then the user needs taken back to the confirmation page instead of the initial amount page. This is because the route params
// are only able to handle one backTo param at a time and the user needs to go back to the amount page before going back
// to the confirmation page
if (pageIndex === 'confirm') {
const routeToAmountPageWithConfirmationAsBackTo = getUrlWithBackToParam(backTo, `/${ROUTES.MONEY_REQUEST_STEP_CONFIRMATION.getRoute(iouType, transactionID, reportID)}`);
Navigation.goBack(routeToAmountPageWithConfirmationAsBackTo);
if (selectedCurrency) {
Navigation.navigate(`${routeToAmountPageWithConfirmationAsBackTo}&currency=${selectedCurrency}`);
} else {
Navigation.goBack(routeToAmountPageWithConfirmationAsBackTo);
}
return;
}
Navigation.goBack(backTo || ROUTES.HOME);
Expand All @@ -78,8 +81,7 @@ function IOURequestStepCurrency({
*/
const confirmCurrencySelection = (option) => {
Keyboard.dismiss();
IOU.setMoneyRequestCurrency_temporaryForRefactor(transactionID, option.currencyCode);
navigateBack();
navigateBack(option.currencyCode);
Copy link
Contributor

Choose a reason for hiding this comment

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

A minor UI issue here. As we navigate back without ensuring that the Onyx has indeed updated the new currency, the new currency does not immediately get displayed in the previous screen. This resulted in issue #41515

};

const {sections, headerMessage, initiallyFocusedOptionKey} = useMemo(() => {
Expand Down
Loading