-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Fix send code validation #47772
base: main
Are you sure you want to change the base?
Fix send code validation #47772
Conversation
Hi @c3024, can we check for the update here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Automation did not work. Commenting so that it appears on my K2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the clarification for case (2) here,
we need changes here,
Line 347 in fc849dd
Navigation.goBack(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(backTo)); |
- Navigation.goBack(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(backTo));
+ Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHOD_DETAILS.getRoute(contactMethod));
and here,
Navigation.goBack(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(backTo)); |
- Navigation.goBack(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(backTo));
+ Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(backTo));
Hi @c3024, let's check this again. |
The first change mentioned here for Here is the video showing this bug. validateChromeBug.mp4 |
Hi @c3024, I haven't added that piece yet, seeing that there was a recent code change which removed navigation completely: https://github.com/Expensify/App/pull/48320/files#r1738644645 I'm still trying to figure out why that was done and what next to do. As it stands, adding that piece gives this flickering navigation behaviour as shown in the screen record Screen.Recording.2024-09-04.at.14.26.25.mov |
90ad353
to
d137dc5
Compare
Reviewer Checklist
Screenshots/VideosAndroid: NativecodeAndroid.mp4Android: mWeb ChromecodeAndroidmWeb.mp4iOS: NativecodeiOS.mp4iOS: mWeb SafaricodeiOSmWeb.mp4MacOS: Chrome / SafaricodeChrome.mp4MacOS: DesktopcodeDesktop.mp4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Hi @shawnborton, Although I didn't make any UI changes here. I can make changes if you could propose what the UI should look like. |
Hi @c3024, what's the plan with this PR, anything more to push for? |
Details
This PR fixes issue with sending email validation link. When a user triggers email validation from profile, the magic sign-in code isn't sent immediately till you have to trigger
Didn't receive a magic code?
. This PR is raise to fix a reverted code merge #46846Fixed Issues
$: 41330
PROPOSAL: issuecomment
Tests
Expectation: The validation link is sent first time, and user is not logged out when magic code is entered.
Offline tests
Expectation: The validation link is sent first time, and user is not logged out when magic code is entered.
QA Steps
Expectation: The validation link is sent first time, and user is not logged out when magic code is entered.
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
MacOS: Chrome / Safari
Screen.Recording.2024-08-21.at.08.35.06.mov
iOS: Native
Android: Native
Android: mWeb Chrome
iOS: mWeb Safari
MacOS: Desktop