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.

Sign up for GitHub

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

Jump to bottom

feat: add desktop download URL and instructions #1233

Merged
merged 24 commits into from
Oct 30, 2023
Merged

feat: add desktop download URL and instructions #1233

merged 24 commits into from
Oct 30, 2023

Conversation

hlopes-ledger
Copy link
Contributor

@hlopes-ledger hlopes-ledger commented May 11, 2023 •
edited
Loading

  • add a downloadUrls.desktop property
  • add a wallet.desktop.instructions property
  • set these on the Ledger connector to present desktop and mobile app download buttons for Ledger Live

demo here: https://645e04523bc76170fcab5872--ledger-rainbowkit-demo.netlify.app

testing:

  • choose the Ledger Live wallet
  • refuse to open Ledger Live when prompted
  • press "GET"
  • there should be app install buttons for desktop and mobile
  • after pressing "Get the desktop app" it should show the instructions
  • pressing "Connect" should take you back to the QR code screen and prompt you to open Ledger Live Desktop

image
image

@hlopes-ledger hlopes-ledger requested a review from a team as a code owner May 11, 2023 16:31
@vercel
Copy link

vercel bot commented May 11, 2023

@hlopes-ledger is attempting to deploy a commit to the rainbowdotme Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented May 12, 2023 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
rainbowkit-example ❌ Failed (Inspect) Mar 19, 2024 3:29pm
rainbowkit-site ❌ Failed (Inspect) Mar 19, 2024 3:29pm

@hlopes-ledger hlopes-ledger force-pushed the add-desktop-app-download branch 2 times, most recently from 2992f6a to 71c1c47 Compare May 12, 2023 17:05
This was referenced May 12, 2023
Closed
Closed
@DanielSinclair
Copy link
Contributor

@hlopes-ledger Thanks for working on this flow. Will try to get reviewed and merged this week!

@DanielSinclair DanielSinclair added the enhancement New feature or request label May 22, 2023
@hlopes-ledger
Copy link
Contributor Author

Thanks @DanielSinclair! #1237 builds on this one to add support for the Ledger Extension.

@DanielSinclair DanielSinclair self-requested a review July 5, 2023 05:31
@DanielSinclair
Copy link
Contributor

After testing, am seeing an issue on the Ledger Live Desktop flow where the UI doesn't properly route before the the desktop deep link is fired.
Before:
Screenshot 2023-07-06 at 3 41 24 PM
After:
Screenshot 2023-07-06 at 3 41 16 PM

@DanielSinclair DanielSinclair self-requested a review July 6, 2023 19:43
@socket-security
Copy link

socket-security bot commented Jul 7, 2023 •
edited
Loading

No top level dependency changes detected. Learn more about Socket for GitHub ↗︎

hlopes-ledger and others added 6 commits July 7, 2023 00:59
@hlopes-ledger @DanielSinclair
feat: add desktop download URL and instructions
48070a3 
- add a downloadUrls.desktop property
- add a wallet.desktop.instructions property
- set these on the Ledger connector to present desktop and mobile
  app download buttons for Ledger Live
@DanielSinclair
fix: getWalletConnectUri in qrCode getUri
e52a08e
@DanielSinclair
feat: platform detection util
664800c
@DanielSinclair
feat: platforms util type enum
35d054f
@DanielSinclair
feat: platform specific desktop download urls
0aeb56b
@DanielSinclair
feat: ledger connector download urls and learn mores
96653f8
@DanielSinclair
Copy link
Contributor

@hlopes-ledger I see. Thanks for the heads up. Will still move forward with this PR and the name change, and would be happy to pick back up on the extension provider support if that moves forward in the future!

@tjulien-ledger
Copy link

Hi @DanielSinclair is there anything we can do to help move forward with merging this PR please?

@ComradeAERGO
Copy link

Hi @DanielSinclair
I'm taking over as @hlopes-ledger no longer works on this project. Please tell us if there's anything we an do to help you merge this PR

@DanielSinclair
Copy link
Contributor

@ComradeAERGO @tjulien-ledger Merging this today after taking care of changes and QA on our end, but is your team actually referring to and hoping for the #1237 PR that Hugo had closed? This PR only adds an additional download button to the Ledger flow to cleanup the Ledger Live onboarding UX on Desktop.

magiziz
magiziz approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@magiziz magiziz left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@ComradeAERGO
Copy link

@ComradeAERGO @tjulien-ledger Merging this today after taking care of changes and QA on our end, but is your team actually referring to and hoping for the #1237 PR that Hugo had closed? This PR only adds an additional download button to the Ledger flow to cleanup the Ledger Live onboarding UX on Desktop.

Thanks a lot. We just want to make sure this our flows are optimal across libraries.

@DanielSinclair DanielSinclair merged commit ef64a22 into rainbow-me:main Oct 30, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielSinclair DanielSinclair DanielSinclair approved these changes

@magiziz magiziz magiziz approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@hlopes-ledger @DanielSinclair @tjulien-ledger @ComradeAERGO @magiziz