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: update uniwallet wc link #7450

Merged
merged 2 commits into from
Nov 6, 2023
Merged

fix: update uniwallet wc link #7450

merged 2 commits into from
Nov 6, 2023

Conversation

just-toby
Copy link
Contributor

@just-toby just-toby commented Oct 11, 2023

Description

!! this should not be merged until uniwallet v1.15 is released !!

just updates the WalletConnect URI schema as requested by the mobile team. see the slack message below for details. according to that message, this new schema will result in the behavior:

  • If the user doesn't have the wallet app, it opens the URL in the browser, which has the link to download the app.
  • If the user has the wallet installed and reads the QR code with the camera app, it'll launch the app and show the WalletConnect sheet (after the fix above is live).
  • If the user reads from the wallet's scan screen, it'll show the WalletConnect sheet (after the fix above is live).

Linear ticket: https://linear.app/uniswap/issue/WEB-2982/update-the-url-for-wc-scan
Slack thread: https://uniswapteam.slack.com/archives/C03JG5E8N4B/p1696948339018599?thread_ts=1695999340.625809&cid=C03JG5E8N4B

Test plan

QA (ie manual testing)

  • @brnunes tested this URL schema and ensured it behaves as outlined above

Devices

  • macbook + iPhone 13 to test the above flows

@vercel
Copy link

vercel bot commented Oct 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
interface ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 6, 2023 7:21pm

@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #7450 (83c3d94) into main (9eaa22f) will not change coverage.
Report is 2 commits behind head on main.
The diff coverage is 0.00%.

Flag Coverage Δ
cloud-tests 83.60% <ø> (ø)
unit-tests 43.14% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@cypress
Copy link

cypress bot commented Oct 11, 2023

Passing run #15350 ↗︎

0 125 0 0 Flakiness 0

Details:

fix: update uniwallet wc link
Project: Uniswap Interface Commit: 83c3d94cfe
Status: Passed Duration: 03:15 💡
Started: Nov 6, 2023 7:21 PM Ended: Nov 6, 2023 7:24 PM

Review all test suite changes for PR #7450 ↗︎

@just-toby just-toby requested a review from brnunes October 11, 2023 16:53
@just-toby just-toby marked this pull request as ready for review October 11, 2023 16:53
@just-toby just-toby requested review from a team and natew October 11, 2023 16:53
@cbachmeier
Copy link
Contributor

Can you leave this as a draft until uniwallet v1.15 is released?

@just-toby just-toby marked this pull request as draft October 16, 2023 21:20
@just-toby just-toby marked this pull request as ready for review November 6, 2023 16:40
Copy link
Contributor

@cbachmeier cbachmeier left a comment

Choose a reason for hiding this comment

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

I have version 1.15 of the app however when I click on the link from the preview, I don't get the WalletConnect sheet
IMG_0438

RPReplay_Final1699291153.MP4

@just-toby
Copy link
Contributor Author

@cbachmeier @cartcrom good points, we don't want to change the redirect URL ! just the QR code one. i just updated and tested the 3 cases in the description to verify it's working as expected

@just-toby just-toby merged commit 098c7b9 into main Nov 6, 2023
24 of 25 checks passed
@just-toby just-toby deleted the fix/new-wc-uniwallet-url branch November 6, 2023 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants