App: [HOLD for payment 2024-04-15] [$750] mWeb - Click on back button from country picker page redirects to wrong page

@puneetlath The discussions mainly revolved around the Datepicker (selecting a year for example) and basically the way the component has been written before made it complicated to select the value and return it successfully to the previous page if the datepicker was reused in different from (you got Expense date field, date of birth, bank flows etc).

Similarly we had the issue with Currency selector in amount page but @koko57 has refactor the Money request flow and made the Currency selection bit easier to reuse while having its own route.

Hence I think we can come back to working on these as its own pages now that we got a patten in place.

We would create a new country page which will take the goBack param which will be used to know where to navigate when the Country is selected or Up button is clicked. I would say we would have to make it similar as the IOUCurrencySelection page.

I will update the OP to correcly describe the problem:

Problem

The country picker in Personal Details > Home Address is made of a component which is controlled in state and it does not have its own route. This was made to simplify reusing the component in various places in the app as its easier to just plug it into some page and let its open/ closed state be controlled from within the parent component as well as take the selected value and save it in the parent form.

This however leads to unexpected navigation behaviour with the back button in browser or in android, because the component is not its own page hence its not a screen in the browser history either.

Now that we got a good solution pattern in place with the IOUCurrencySelection component reused for 1. creating money request, 2. Confirming creation of money request and 3. Editing money request amount, all done using the same component and having its own path which allows for correct and predictable navigation.

Solution

Lets build the Country picker page as we did IOUCurrencySelection using its own route accepting goBack as url param, and making sure the AddressPage will consume the country picked from the ?country url param same as we do with ?currency in the other case.

Well, both proposals are not very detailed. so I am going to pick one on the basis who proposed the required solution first.

@ygshbht’s proposal looks good to me. They proposed this as well. The main technique is already outlined in the issue details and there are many examples in the App to follow.

🎀 👀 🎀 C+ reviewed

Assigning @WoLewicki per team discussion

Payment Summary

Upwork Job

• Reviewer: @parasharrajat owed $250 via NewDot
• Contributor: @WoLewicki is from an agency-contributor and not due payment
• ROLE: @ygshbht paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@greg-schroeder)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1697712669744525312/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

I think we should add a test for this but one should be enough.

I would say lets use the For Personal Address:

@greg-schroeder would you be able to add this one? thanks!

Perfect thanks!

Yes, it’s better. I’ll have the PR ready in a day

I wouldn’t consider that as a regression as https://github.com/Expensify/App/issues/28743 is completely an edge case. However, it should be fixed to prevent crashes. @mountiny What do you think?

Anyway, we are still waiting on @mountiny to comment on https://github.com/Expensify/App/issues/23725#issuecomment-1738794394 so that we can start on the second PR for this issue.

Thus, we can handle https://github.com/Expensify/App/issues/28743 in the next PR.

Increasing the price to 750 lets do the same thing for statePicker in the address page in a new pr

@parasharrajat I think it would be great to fix the StatePicker for sure.

Year picker can be a separate issue

Lets see based on the actual PR, to some extent we can just copy paste the IOU page approach

Yeah, doing it now.

Yep, the decision might not be very starightforward. Anyway, I have added more details to the proposal for him to review

Proposal

Updated

We had bunch of discussions before when we had it its own page and the problem was reusability, so I think this just boils down to what we prefer more. I agree that there is not really a simple solution as we cannot mess with the browser history.

I think we could explore taking a fresh look with Callstack on how to make these forms its own pages so that the navigation works as expected.

Isn’t it the same for web when clicking browser back button @kbecciv ?

@MariaHCD I will post this in SWM channel to let the guys know an dI think they will take it

posting NewFeature as I think its more true than bug

@MariaHCD This is not a blocker. This is same for Year picker and similar parts of the form. The Year page or in this case the State or Country picker are not its own route/ page in the stack hence when you click back it takes you to the personal details.

I think its something we might have to look at as a general solution as I think we would like this fixed for consistency, but its not a blocker.

I will demote this and add it to the navigation follow ups tracking project.