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

Wallet - Confirm button is not shown on the currency selector page #55678

Closed
1 of 8 tasks
lanitochka17 opened this issue Jan 23, 2025 · 17 comments
Closed
1 of 8 tasks

Wallet - Confirm button is not shown on the currency selector page #55678

lanitochka17 opened this issue Jan 23, 2025 · 17 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@lanitochka17
Copy link

lanitochka17 commented Jan 23, 2025

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 9.0.89-2
Reproducible in staging?: Y
Reproducible in production?: No
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: #54798
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team
Component: User Settings

Action Performed:

  1. Login to NewDot.
  2. Go to Settings > Wallet > Add bank account
  3. Click Add bank account
  4. Select any county other than United States(or select India)
  5. Click Next
  6. Click the currency push row

Expected Result:

You see the Confirm button at the bottom

Actual Result:

Confirm button is not shown

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Bug6722172_1737666570946.Screen_Recording_2025-01-24_at_12.00.48_at_night.mp4

View all open jobs on GitHub

@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Jan 23, 2025
Copy link

melvin-bot bot commented Jan 23, 2025

Triggered auto assignment to @yuwenmemon (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link

melvin-bot bot commented Jan 23, 2025

💬 A slack conversation has been started in #expensify-open-source

Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@mkzie2
Copy link
Contributor

mkzie2 commented Jan 24, 2025

🚨 Edited by proposal-police: This proposal was edited at 2025-01-24 02:04:56 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Confirm button is not shown

What is the root cause of that problem?

We do not show confirm button on selection list here

What changes do you think we should make in order to solve the problem?

If we just want to add Confirm button to the currency list in add bank account flow, Introduce showConfirmButton to CurrencySelectionList and CurrencyPicker. Then enable it in BankAccountDetails.

In CurrencyPicker, define SelectionList's onConfirm as:

onConfirm={(e, item) => updateInput(item)}

In order to do that, we need to update updateInput to accept optional item parameter.

const updateInput = (item: ValuePickerItem) => {

The confirm button here simply just navigate back when no thing new was selected.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

NA

What alternative solutions did you explore? (Optional)

Or we can modify the currency flow to work like CountrySelection where you need to select the country first and press Confirm to close the picker and save the selection. Currently in CurrencyPicker, press any currency row will automatically save it and close the picker.

@yuwenmemon
Copy link
Contributor

@MonilBhavsar @shubham1206agra Seems like this is also related to #54798?

@shubham1206agra
Copy link
Contributor

@yuwenmemon This was decided to be as expected. There will be confirm button in currency selection page. You can close this issue.

@Beamanator Beamanator added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Jan 24, 2025
@Beamanator
Copy link
Contributor

Nice, at least marking NAB for now, @yuwenmemon can close

@yuwenmemon
Copy link
Contributor

Well, it sounds like we need to update the regression steps at least. @MonilBhavsar, since this is your project, are you able to take care of that?

@MonilBhavsar
Copy link
Contributor

MonilBhavsar commented Jan 24, 2025

I will, thanks!

@melvin-bot melvin-bot bot added the Overdue label Jan 27, 2025
@joekaufmanexpensify
Copy link
Contributor

This was decided to be as expected. There will be confirm button in currency selection page. You can close this issue.

@shubham1206agra Do you mind linking me to thread about this? I know we had a lot of discussions about the feature in Slack, but I don't remember cutting the confirm button on this page. And all the design doc mockups show it/discuss it being there.

I totally might be misremembering, but just want to make sure I am following so I can update the tests if we talked about cutting this.

@joekaufmanexpensify joekaufmanexpensify self-assigned this Jan 27, 2025
@melvin-bot melvin-bot bot removed the Overdue label Jan 27, 2025
@joekaufmanexpensify joekaufmanexpensify added the Bug Something is broken. Auto assigns a BugZero manager. label Jan 27, 2025
Copy link

melvin-bot bot commented Jan 27, 2025

Current assignee @joekaufmanexpensify is eligible for the Bug assigner, not assigning anyone new.

@shubham1206agra
Copy link
Contributor

@garrettmknight garrettmknight moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Jan 28, 2025
@garrettmknight garrettmknight added the Internal Requires API changes or must be handled by Expensify staff label Jan 28, 2025
@joekaufmanexpensify
Copy link
Contributor

Got it, thanks! In the future, could you please tag me and @MonilBhavsar if we're contemplating changes to the design agreed on in the design doc? I don't think we were aware that we were removing this button.

@joekaufmanexpensify
Copy link
Contributor

After doing some more research in the app, I think it's fine to leave this as is though, especially if it would be a lot of work to refactor this page to add the button. I'll update the regression test and then we can close this.

@joekaufmanexpensify
Copy link
Contributor

Updated tests to clarify this.

@joekaufmanexpensify
Copy link
Contributor

Closing as this is all set!

@github-project-automation github-project-automation bot moved this from Bugs and Follow Up Issues to Done in [#whatsnext] #expense Jan 28, 2025
@MonilBhavsar
Copy link
Contributor

Thank you Joe!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
Status: Done
Development

No branches or pull requests

8 participants