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

[Enhancement] Specific ScanView child classes to limit acceptable QR types #333

Merged
merged 9 commits into from
Aug 6, 2023

Conversation

kdmukai
Copy link
Contributor

@kdmukai kdmukai commented Feb 20, 2023

Aside from the MainMenuView's "Scan" option, all other ScanView calls are expecting a specific type of QR code (e.g. "Scan a PSBT") but nothing is stopping ScanView from accepting an unexpected but supported QR type.

This PR:

  • adds specific ScanView child classes that display an error screen if the user scans an unexpected QR type.

  • displays a friendlier error screen when an invalid or unrecognized QR format is scanned (the current approach just raises an Exception and makes it look like a bug in SeedSigner).

  • promotes "Verify address" to the Tools menu. Previously it was a kind of hidden feature that would be activated if a bitcoin address was scanned by ScanView.

Testing notes

If a user scans the wrong kind of QR, verify that they get another chance to resume their task and scan again.

@newtonick newtonick added the enhancement New feature or request label Feb 28, 2023
@newtonick newtonick added this to the 0.6.1 milestone Feb 28, 2023
@jdlcdl
Copy link

jdlcdl commented Feb 28, 2023

Concept ACK
ACK tested

I like: flexibility of being able to enter generic scan view without expectations (as currently) and additionally being able to enter a specific scan view when navigation has been specific... and having better instructions as well as better error messages.

I also like having a new ErrorView to direct flow when things go "a little" wrong instead of the dire-warning-ish "raise an exception" for errors that are common user mistakes.... reserving exceptions to truly exceptional cases.

I will test this.
I have tested:

  • scans seed/address/descriptor/psbt types from main menu, just as before.
  • when scanning specific types: offers better instructions, and better errors with "expected" and "received" information for user.

@newtonick newtonick added the merge conflicts has merge conflicts that need resolution label Aug 1, 2023
* Promotes single address verification to a top-level Tool.
* Promotes single address verification to a top-level Tool.
* Promotes single address verification to a top-level Tool.
@newtonick newtonick removed the merge conflicts has merge conflicts that need resolution label Aug 5, 2023
@newtonick
Copy link
Collaborator

ACK Tested and reviewed

@newtonick newtonick merged commit 93e90ce into SeedSigner:dev Aug 6, 2023
@kdmukai kdmukai deleted the scan_qrtype_checks branch September 2, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants