Add universal qr scanner #11014
Merged
Add universal qr scanner #11014
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (31)
|
Ferossgp
force-pushed
the
feature/add-universal-qr-scanner
branch
4 times, most recently
from
d608f7d
to
f8ec3f6
Compare
Ferossgp
force-pushed
the
feature/add-universal-qr-scanner
branch
from
f8ec3f6
to
26baf8b
Compare
98% of end-end tests have passedFailed tests (2)Click to expand
Passed tests (95)Click to expand |
|
@Ferossgp can you pleae rebase it? thanks! |
Ferossgp
force-pushed
the
feature/add-universal-qr-scanner
branch
from
26baf8b
to
497de1b
Compare
ISSUE 1: on scanning chat keys or universal links app is redirected to profile view with profile of another userAnd to see again your profile you need to go back. Expected result: Actual result:
ISSUE 2: unable to scan valid QR after scanning invalid ENS nameSteps:
Expected result: ISSUE 3: on scanning deep link with public chat - null is not an object errorSteps:
Expected result: Actual result: ISSUE 4: doesn't support universal links to dapps with full URLsSteps:
Expected result: Actual result: ISSUE 5: error at attempt to scan EIP linkSteps:
Expected result: Actual result: |
Cannot replicate, maybe there was some context before that action, or can you please share INFO logs? All other issues fixed |
Ferossgp
force-pushed
the
feature/add-universal-qr-scanner
branch
2 times, most recently
from
04a7513
to
d576e72
Compare
Ferossgp
force-pushed
the
feature/add-universal-qr-scanner
branch
from
d576e72
to
872ccb5
Compare
|
@Ferossgp thanks for fixes! About other issues: ISSUE 5: EIP links are still not fully supported (for tokens, ENS names, payment links)More context of supported params is here: #10280 Links for testing:
Test case is here: https://ethstatus.testrail.net/index.php?/cases/view/6282 Also in case of scanning valid wallet address as it mentioned in #10764 should open If users scans wallet address open Wallet > Send > To field includes address. ISSUE 6: after scanning another user profile scanning your profile key redirects to another user profileSteps:
Expected result: Actual result: |
|
@churik fixed |
flexsurfer
reviewed
Aug 10, 2020
| (navigation/navigate-to-cofx :tabs {:screen :wallet}))) | ||
|
|
||
| (fx/defn match-scan | ||
| {:events [::match-scanned-value]} |
There was a problem hiding this comment.
could you please elaborate why handlers are here and not in the router ns ?thanks
There was a problem hiding this comment.
should it be handled different if we scan it or if we open universal link?
There was a problem hiding this comment.
Handlers are almost similar, the only difference is on errors strings. Also in some places we do not show error in Universal Links, while in QR we say that we can't scan this code
Ferossgp
force-pushed
the
feature/add-universal-qr-scanner
branch
from
29a35bd
to
e4c00ee
Compare
|
@churik fixed wallet scanner |
flexsurfer
approved these changes
Aug 10, 2020
90% of end-end tests have passedFailed tests (10)Click to expand
Passed tests (87)Click to expand
|
|
Tested universal code scanner + existing QR code scanners from chat view, wallet view, bootnode, mailserver QR code scanner on IOS 13 and Android 10 Awesome work! |
|
@Ferossgp new issue with redirect to user profile from universal link: Thanks for the clue! |
|
Fixed and verified. Ready to be merged! |
Rename events Add router to handle all links Use router in add new chat Unify universal link and universal qr with router Add icon for universal scanner Update tests Now routing is tested in routing PR lint Cleanup QA fixes Scan own profile Handle more EIP Fix wallet scanner Fix stack for view profile in UL Signed-off-by: Gheorghe Pinzaru <feross95@gmail.com>
Ferossgp
force-pushed
the
feature/add-universal-qr-scanner
branch
from
f720518
to
7935798
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #10764
Added bidi to handle routes instead of regexp. Also unified the QR and Universal links, to use same routing as they use the same links.