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

Refactor(phone-auth-screen): issue #2812 Updated headers on phone authentication screens to more accurately reflect the type of authentication (creating account versus logging in) #2814

Merged
merged 6 commits into from
Nov 23, 2023

Conversation

MaxwellDG
Copy link
Contributor

@MaxwellDG MaxwellDG commented Nov 22, 2023

There was some unused code (unused as in it was being overridden) that looked like it was trying to make the header during login to say 'Phone number' as the header. So I followed suit and used that.

For ease of reading the PR, take note of changing the optional property 'type' (in PhoneValidationStackParamList.phoneLoginValidate) to required. There was only 1 instance of this property not being used and that was in 'earns-section.tsx'. It used to pass in undefined and then default to PhoneLoginInitiateType.CreateAccount. Now it's stated there explicitly.

…will now show 'Account set up' or 'Phone number' depending on where the user has navigated from
@MaxwellDG
Copy link
Contributor Author

Header during both 'phoneFlow' screens for login:
Phone-number-1

Header during both 'phoneFlow' screens for creating account:
phone-login-2

@nicolasburtey
Copy link
Member

  • the title of the PR is not very meaningful. release note are auto generated from it and "2812" will not really give insight about what this PR is about.
  • there is missing a dependency in useEffect as highlight by the linter

fix: Added missing dependency "screenType" to useEffect
@MaxwellDG MaxwellDG changed the title Refactor(phone-auth-screen): 2812 Refactor(phone-auth-screen): issue #2812 Updated headers on phone authentication screens to more accurately reflect the type of authentication (creating account versus logging in) Nov 23, 2023
@nicolasburtey
Copy link
Member

thanks for this. the main reason we use the same screen today is because there is no differenciation in the API between an account creation and login back in, because we use a passwordless authentication system.

I guess that is fine to have different title, but eventually the confusion might still be around if someone is logging in with the wrong account because they will have a new account with a zero balance.

@nicolasburtey nicolasburtey merged commit 869846f into GaloyMoney:main Nov 23, 2023
7 checks passed
@MaxwellDG MaxwellDG deleted the refactor/2812 branch January 3, 2024 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants