You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Wallet - Left side bar is LHN instead of Settings after adding a bank account #39827
Version Number: 1.4.61-0 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs:https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Slack conversation:
👋 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:
Identify the pull request that introduced this issue and revert it.
@rlinoz FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
Hmm I don't think anyone would directly access the page called /add-bank-account-refactor - that might just be something temporary as we work through all of the styles here? Definitely curious for @mountiny 's take but I have a feeling we can close this one out?
Activity
melvin-bot commentedon Apr 8, 2024
Triggered auto assignment to @rlinoz (
DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.github-actions commentedon Apr 8, 2024
👋 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:
lanitochka17 commentedon Apr 8, 2024
@rlinoz FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
rlinoz commentedon Apr 8, 2024
Looking at the videos in the original PR it looks like it is working as intended, maybe @mountiny or @shawnborton can confirm?
shawnborton commentedon Apr 8, 2024
Hmm I don't think anyone would directly access the page called
/add-bank-account-refactor- that might just be something temporary as we work through all of the styles here? Definitely curious for @mountiny 's take but I have a feeling we can close this one out?c3024 commentedon Apr 8, 2024
I think we map the central pane to RHP in these cases for deeplink cases. In this case by adding
here
App/src/libs/Navigation/linkingConfig/CENTRAL_PANE_TO_RHP_MAPPING.ts
Line 35 in dd30d13
Also, this route being sub-part of wallet shouldn't this be in
WALLEThereApp/src/SCREENS.ts
Line 70 in dd30d13
instead of directly in
SETTINGShereApp/src/SCREENS.ts
Line 35 in dd30d13
allgandalf commentedon Apr 8, 2024
Proposal
Please re-state the problem that we are trying to solve in this issue.
LHN changes when for wallet routes
What is the root cause of that problem?
We have not mapped all routes related to add bank account/ add debit card for wallet pages:
App/src/libs/Navigation/linkingConfig/CENTRAL_PANE_TO_RHP_MAPPING.ts
Lines 24 to 36 in dd30d13
What changes do you think we should make in order to solve the problem?
Map all the routes related to adding bank account/ debit card for wallet page;
App/src/ROUTES.ts
Lines 110 to 113 in dd30d13
What alternative solutions did you explore? (Optional)
N/A
mountiny commentedon Apr 8, 2024
@rlinoz thanks for the ping
This is not used flow yet and its middle of development hence the special route. We can close this
@koko57 will address this in the future work, the page needs to be added to the correct mapping
koko57 commentedon Apr 8, 2024
yes, it's temporary as the refactor is being worked on - I'll take care of that in one of the following PRs
cc @mountiny @shawnborton