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.

Sign up for GitHub

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

Jump to bottom

[Wallet] Secure Send Navigation Bug Fix #4489

Merged
merged 6 commits into from Jul 22, 2020
Merged

[Wallet] Secure Send Navigation Bug Fix #4489

merged 6 commits into from Jul 22, 2020

Conversation

tarikbellamine
Copy link
Contributor

@tarikbellamine tarikbellamine commented Jul 17, 2020 •
edited

Description

Fixes a bug where repeatedly cycling through "edit" for secure send would cause navigation to stall. See tagged issue for context.

Other changes

Minor refactoring to further consolidate secure send properties into the mapping object.

Tested

Wrote new tests.

Related issues

Backwards compatibility

Yes

jeanregisser
jeanregisser suggested changes Jul 20, 2020
View reviewed changes
Copy link
Contributor

@jeanregisser jeanregisser left a comment

Choose a reason for hiding this comment

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

Looking almost good, see comments below.

packages/mobile/src/navigator/Screens.tsx Show resolved Hide resolved
packages/mobile/src/send/ValidateRecipientAccount.tsx Outdated Show resolved Hide resolved
packages/mobile/test/schemas.ts Show resolved Hide resolved
jmrossy
jmrossy reviewed Jul 20, 2020
View reviewed changes
Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

Looks okay to me, please see Jean's comments

jeanregisser
jeanregisser approved these changes Jul 22, 2020
View reviewed changes
Copy link
Contributor

@jeanregisser jeanregisser left a comment

Choose a reason for hiding this comment

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

Awesome 🎉

@jeanregisser jeanregisser added the automerge Have PR merge automatically when checks pass label Jul 22, 2020
@mergify mergify bot merged commit 937f70b into master Jul 22, 2020
@mergify mergify bot deleted the tarikbellamine/secure-send-edit-bug branch July 22, 2020 10:10
@tarikbellamine tarikbellamine mentioned this pull request Sep 16, 2020
Closed
@ValoraQA
Copy link

ValoraQA commented Mar 4, 2021

Hey @tarikbellamine I have verified this issue using latest Android Internal Build V1.11.0(1004294337) & Test Flight Build V1.11.0(47) & observed that issue is still reproduced. User not able to proceed by submitting last 4 digit codes again from Send secure flow screen.
Verified on devices: Samsung Galaxy A5(7.0),iPhone SE (13.4.1)
Attachment: NotProceedBySubmitting4CodesAgain.mp4

@jeanregisser
Copy link
Contributor

Thanks, could you open a new bug for this?

@ValoraQA
Copy link

ValoraQA commented Mar 9, 2021

Sure @jeanregisser we will raise a new issue for this, Thanks!

@ValoraQA
Copy link

ValoraQA commented Mar 9, 2021

Hi @jeanregisser We have raised new issue valora-inc/wallet#106.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmrossy jmrossy jmrossy left review comments

@jeanregisser jeanregisser jeanregisser approved these changes

@cmcewen cmcewen Awaiting requested review from cmcewen

Assignees
No one assigned
Labels
automerge Have PR merge automatically when checks pass wallet
Projects
None yet
Milestone
No milestone
4 participants
@tarikbellamine @ValoraQA @jeanregisser @jmrossy