Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC906e51b5a12c: [Cashtab] Update ScanQRCode.js

Summary

T3387

Upgrade to latest available zxing. Update component logic to work with latest zxing.

Test Plan

npm test

Navigate to https://cashtab-local-dev.netlify.app/ which deploys this diff
Scan a QR code of an ecash address
Scan a QR code of a bip21 string, try entering ecash:qp89xgjhcqdnzzemts0aj378nfe2mhu9yvxj9nhgg6?amount=17&op_return_raw=04007461622263617368746162206D6573736167652077697468206F705F72657475726E5F726177 at https://www.qr-code-generator.com/
Scan a QR code of a URL and verify you get a validation error

Unfortunately I'm not able to mock an error thrown by the camera. Manually test that the camera modal goes away and the error msg is rendered by

change line 47 of ScanQRCode.js from if (error instanceof NotFoundException) { to if (!(error instanceof NotFoundException)) {
npm start

screenshot rel:

Diff Detail

Repository

rABC Bitcoin ABC

Branch

qr-code-improvements

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 26593
Build 52756: Build Diff	cashtab-tests
Build 52755: arc lint + arc unit

Event Timeline

bytesofman created this revision.Jan 20 2024, 23:35

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 20 2024, 23:35

Harbormaster completed remote builds in B26518: Diff 44400.Jan 20 2024, 23:35

lint

Harbormaster completed remote builds in B26519: Diff 44401.Jan 20 2024, 23:38

working implementation

Harbormaster completed remote builds in B26526: Diff 44413.Jan 22 2024, 00:06

TODO

test on iOS safari
test on non-safari iOS
add integration tests
Do you need the error and setError? at the moment it isn't doing anything, you need to know which errors need to be caught

add integration tests

bytesofman edited the test plan for this revision. (Show Details)Jan 22 2024, 19:26

Harbormaster completed remote builds in B26559: Diff 44453.Jan 22 2024, 19:27

bytesofman published this revision for review.Jan 22 2024, 19:36

bytesofman edited the test plan for this revision. (Show Details)

bytesofman added inline comments.

cashtab/src/components/Common/ScanQRCode.js
136 ↗	(On Diff #44453)	Apple opened up this API. I've asked John to test on iOS safari and non-safari to confirm camera comes up.

Fabien requested changes to this revision.Jan 23 2024, 08:34

Fabien added a subscriber: Fabien.

Fabien added inline comments.

cashtab/src/components/Common/ScanQRCode.js
59 ↗	(On Diff #44453)	How does this work ? This branch will be executed if there is no error, but you will log an error, display an error and stop scanning. My guess is that the error is immediately hidden so you didn't notice.

This revision now requires changes to proceed.Jan 23 2024, 08:34

patch error handling issue

bytesofman marked an inline comment as done.Jan 23 2024, 17:18

bytesofman added inline comments.

cashtab/src/components/Common/ScanQRCode.js
59 ↗	(On Diff #44453)	good catch

Harbormaster completed remote builds in B26585: Diff 44490.Jan 23 2024, 17:21

timing bug, error raised often on frequent scans

Also ignore FormatException error

Harbormaster completed remote builds in B26593: Diff 44503.Jan 23 2024, 22:53

Fabien accepted this revision.Jan 24 2024, 08:59

This revision is now accepted and ready to land.Jan 24 2024, 08:59

Fabien requested changes to this revision.Jan 24 2024, 09:00

Fabien added inline comments.

cashtab/src/components/Common/ScanQRCode.js
69	You should return here, if there is an error you don't want to process the result

This revision now requires changes to proceed.Jan 24 2024, 09:00

bytesofman marked an inline comment as done.Jan 24 2024, 18:12

bytesofman added inline comments.

cashtab/src/components/Common/ScanQRCode.js
69	handled also in testing I found another error to ignore. It's the 3 of 3 of subclasses of the ReaderException, so that should be all of them (the types of errors that come from the reader getting a bad scan of the barcode and thinking it is not a QR code, or has a bad checksum, or there is no qr code in the image ... in these cases we want to just keep scanning until we find a good qr code)