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

fix: Reopen QR modal in safe creation #2294

Merged
merged 2 commits into from
Aug 1, 2023
Merged

fix: Reopen QR modal in safe creation #2294

merged 2 commits into from
Aug 1, 2023

Conversation

usame-algan
Copy link
Member

@usame-algan usame-algan commented Jul 14, 2023

What it solves

Fix for #2280

How this PR fixes it

  • Only runs the validation if the value has changed

How to test it

  1. Open Safe creation
  2. Add an owner
  3. Press the QR button and select an address
  4. Press it again and observe it opens again

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

@github-actions
Copy link

github-actions bot commented Jul 14, 2023

Branch preview

✅ Deploy successful!

https://fix_qr--walletweb.review-wallet-web.5afe.dev

@github-actions
Copy link

github-actions bot commented Jul 14, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

Copy link
Member

@iamacook iamacook left a comment

Choose a reason for hiding this comment

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

There's a test that needs to be fixed but otherwise looks good to me!

@usame-algan usame-algan marked this pull request as draft July 14, 2023 14:48
@usame-algan
Copy link
Member Author

Looks like the validation now doesn't run anymore in certain cases. Will have to take a look at this again. Ideally we can find a way to not trigger the validation when pressing the QR Code button.

@francovenica
Copy link
Contributor

francovenica commented Jul 14, 2023

Looks like the validation now doesn't run anymore in certain cases. Will have to take a look at this again. Ideally we can find a way to not trigger the validation when pressing the QR Code button.

Yeah, In the address book test it seems that is failing becaus the "Save" button becomes clickable as soon as you write anything, and not when a valid address is in the input.
In the load safe is failing because a random string of text is not triggering the error in the address input

@usame-algan usame-algan marked this pull request as ready for review August 1, 2023 08:24
@usame-algan
Copy link
Member Author

I've switched to using RHF native isValidating flag to solve this issue.

@francovenica
Copy link
Contributor

Looks good now. The validations work fine as well.

@usame-algan usame-algan merged commit e78930a into dev Aug 1, 2023
7 checks passed
@usame-algan usame-algan deleted the fix-qr branch August 1, 2023 13:36
@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants