feat(qr): add new QR library SCodes to scan qr codes#19811
Conversation
Jenkins BuildsClick to see older builds (366)
|
8fbff18 to
9c6904d
Compare
83aa0ed to
a70b581
Compare
micieslak
left a comment
There was a problem hiding this comment.
I like the new lib, nice replacement for outdated zxing.
Few things:
- I think it's good practice to make for of the lib and refer to that fork. It gives us better control and prevent surprises (like external lib removed and history changed in master)
- please consider using
CMake'sFetchContent. Then config is very simple and limited to single place inCMakeLists.txt, there are some usages already in that file, e.g.:
FetchContent_Declare(
QtModelsToolkit
GIT_REPOSITORY https://github.com/status-im/QtModelsToolkit.git
GIT_TAG af59b81bc10bc37359c2839ae202c160c8321f1a
)
FetchContent_MakeAvailable(QtModelsToolkit)
caybro
left a comment
There was a problem hiding this comment.
LGTM
Minus what Michal said; let's use a fork with a FetchContent
9c6904d to
28b8e62
Compare
8039b14 to
76ce2f9
Compare
28b8e62 to
cc5b2a8
Compare
90018a9 to
9dc41d9
Compare
cc5b2a8 to
869d9f7
Compare
fb4106c to
7dae4da
Compare
|
Alright, I finally fixed all the builds. You guys can review |
869d9f7 to
f09775d
Compare
ee915fa to
312b447
Compare
f09775d to
e5ef90a
Compare
312b447 to
277afd7
Compare
cbdc365 to
e8ea87c
Compare
8179992 to
c42daf3
Compare
|
Tested build 35
|
9051b22 to
aa19aed
Compare
c42daf3 to
ae0a275
Compare
aa19aed to
50d22e2
Compare
2fd3d90 to
17b0ef0
Compare
|
@sunleos thanks for the tests.
@sunleos you can test again once the build is ready |
|
re 5.: it seems like it was a regression on master. Alex fixed it here: #19940 |
Unfortunately, it looks that one second loader is not enough. Let's increase it a bit more if it is reasonable. Screen_Recording_20260212_234350_Status_PR.mp4 |
Ah, sorry about making you change the copy. If it works the way you just described in this line above, then, yes, the copy "cannot" is more appropriate there and works better than "could not" from my perspective as a user. Let's revert the copy back |
Got it. Makes sense. Yeah, we can implement it later if needed. |
just tested this PR. Yes, confirming that issue 5 is fixed there :) |
17b0ef0 to
1c115b0
Compare
1c115b0 to
475ddbc
Compare
|
@sunleos nice finds. I fixed all the issues you found. I increased the loader to two seconds before showing an error. |
Hey @jrainville, Tested APK build 44 All issues are fixed. Great job!!! 👍 :) Just two small comments:
|












What does the PR do
Part of #19678
Based on top of #19766
Adds a new library to scan QR codes.
The reason is that the one we are using is very old and doesn't support more modern QR codes that are rounded, like the wallet connect ones.
This new lib does support scanning WC codes, though I'll create a new PR to add just the WC support code to the QML.
Affected areas
New lib
Architecture compliance
My PR is consistent with this document: QML Architecture Guidelines
Screencapture of the functionality
Still works:
new-scan-lib.webm
Impact on end user
None, though scanning QR codes is probably gonna be faster
How to test
Risk
Low-Medium