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

Add error messages when wrong pin is entered #195

Merged
merged 9 commits into from
Dec 8, 2018
Merged

Conversation

Tbaut
Copy link
Contributor

@Tbaut Tbaut commented Nov 29, 2018

Screenshots:
image
image
image

@Tbaut Tbaut requested a review from lexfrl November 29, 2018 13:39
@lexfrl
Copy link
Contributor

lexfrl commented Nov 29, 2018

cool! Could you attach some screenshots also?

Copy link
Contributor

@lexfrl lexfrl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Small grumbles though

src/screens/AccountUnlock.js Outdated Show resolved Hide resolved
src/screens/AccountUnlock.js Outdated Show resolved Hide resolved
src/screens/AccountUnlock.js Outdated Show resolved Hide resolved
src/screens/AccountUnlock.js Outdated Show resolved Hide resolved
src/screens/AccountUnlock.js Outdated Show resolved Hide resolved
src/screens/AccountUnlock.js Outdated Show resolved Hide resolved
src/screens/AccountUnlock.js Outdated Show resolved Hide resolved
src/stores/AccountsStore.js Outdated Show resolved Hide resolved
src/stores/AccountsStore.js Outdated Show resolved Hide resolved
src/stores/ScannerStore.js Outdated Show resolved Hide resolved
@Tbaut
Copy link
Contributor Author

Tbaut commented Nov 29, 2018

Did some cleanup for unused components or functions (sometime I just commented it, will remove it if it's ok, along with the console.log)

^ the description of the issue :) I'll remove those console.log of course, it's a WIP requesting feedback in general.

@Tbaut
Copy link
Contributor Author

Tbaut commented Dec 5, 2018

It should be good to go if you want to have a look :)

@Tbaut Tbaut changed the title [WIP] Add error messages when wrong pin is entered Add error messages when wrong pin is entered Dec 5, 2018
@lexfrl
Copy link
Contributor

lexfrl commented Dec 5, 2018

As we discussed before, we's better to not change the current onChange behavior. What we decided to do is to just show user the current "unlocking" status.

@Tbaut
Copy link
Contributor Author

Tbaut commented Dec 7, 2018

Here we go, I've moved the error message to the state.
As we discussed, I don't think it's possible to leave the onChange function unchanged as I need to pass the information if the pin is wrong or right. If should now work well on iphone as well.

Let me know your thoughts.

Copy link
Contributor

@lexfrl lexfrl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect! One small grumble.

this.setState({ pin });
if (pin.length < 1) {
onChangeText={async (pin) => {
this.setState({ pin: pin })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be leaved as just this.setState({ pin });

@Tbaut Tbaut deleted the tbaut-error-message-pin branch September 17, 2019 09:43
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.

[UX] Account creation, give feedback for PIN typing and confirmation
2 participants