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

Deprecating wallet picker #1039

Merged
merged 7 commits into from
Nov 6, 2023
Merged

Deprecating wallet picker #1039

merged 7 commits into from
Nov 6, 2023

Conversation

bangtoven
Copy link
Contributor

Summary

How did you test your changes?

@bangtoven bangtoven changed the title Jungho/picker Deprecating wallet picker Oct 28, 2023
bangtoven
bangtoven commented Oct 28, 2023
View reviewed changes
packages/wallet-sdk/src/components/ConnectContent/ConnectContent.tsx
@@ -8,12 +8,10 @@ import { useCallback, useState } from 'preact/hooks';
import { createQrUrl } from '../../util';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

before
Screenshot 2023-10-27 at 4 59 15 PM

after
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fyi, we are working with design for new UI, removing the white space and making it responsive to screensize.

packages/wallet-sdk/src/components/icons/QRCodeIcon.tsx
<path d="M8.89922 7.39912H7.99922V5.40112H5.44922L5.44922 9.79912H6.34922L6.34922 6.30112H7.09922V8.29912H9.79922V5.40112H8.89922V7.39912Z" />
<path d="M7.99912 8.89917H7.09912V9.79917H7.99912V8.89917Z" />
<path d="M9.79917 8.89917H8.89917V9.79917H9.79917V8.89917Z" />
<svg width="18" height="18" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" {...props}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

before
image

after
image

packages/wallet-sdk/src/relay/MobileRelay.ts
location.href = `https://www.coinbase.com/connect-dapp?uri=${encodeURIComponent(
location.href
)}`;
location.href = `https://go.cb-w.com/dapp?cb_url=${encodeURIComponent(location.href)}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverting
image

dapp is also a correct path

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need any walletlink changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no. this is handled by branch.io

@bangtoven bangtoven marked this pull request as ready for review October 28, 2023 00:09
@bangtoven bangtoven enabled auto-merge (squash) October 28, 2023 00:21
cb-jake
cb-jake reviewed Oct 30, 2023
View reviewed changes
Copy link
Contributor

@cb-jake cb-jake left a comment

Choose a reason for hiding this comment

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

Wonder if we should also remove the setAppSrc? It was added to support switching between cb and cbw

cb-jake
cb-jake reviewed Oct 30, 2023
View reviewed changes
Copy link
Contributor

@cb-jake cb-jake left a comment

Choose a reason for hiding this comment

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

if we're only supporting a single wallet moving forward we can clean up the render on ConnectContent by removing the mapping of the defined wallets and just rending the CoinbaseWalletSteps directly with a single ConnectItem component.

@bangtoven
Copy link
Contributor Author

thanks @cb-jake for the review!

  • no walletlink changes needed for this to work.
  • will take a look at setAppSrc
  • gotcha. let me see if there's any more clean up we could do. fyi, actually we asked design/product support on this to make the UI look better with single wallet.

@bangtoven bangtoven marked this pull request as draft November 1, 2023 18:42
auto-merge was automatically disabled November 1, 2023 18:42

Pull request was converted to draft

bangtoven
bangtoven commented Nov 4, 2023
View reviewed changes
packages/wallet-sdk/src/components/Snackbar/Snackbar.tsx
@@ -7,18 +7,9 @@ import { useEffect, useState } from 'preact/hooks';

import css from './Snackbar-css';

const cblogo = `data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMzIiIGhlaWdodD0iMzIiIGZpbGw9Im5vbmUiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyI+PHBhdGggZD0iTTEuNDkyIDEwLjQxOWE4LjkzIDguOTMgMCAwMTguOTMtOC45M2gxMS4xNjNhOC45MyA4LjkzIDAgMDE4LjkzIDguOTN2MTEuMTYzYTguOTMgOC45MyAwIDAxLTguOTMgOC45M0gxMC40MjJhOC45MyA4LjkzIDAgMDEtOC45My04LjkzVjEwLjQxOXoiIGZpbGw9IiMxNjUyRjAiLz48cGF0aCBmaWxsLXJ1bGU9ImV2ZW5vZGQiIGNsaXAtcnVsZT0iZXZlbm9kZCIgZD0iTTEwLjQxOSAwSDIxLjU4QzI3LjMzNSAwIDMyIDQuNjY1IDMyIDEwLjQxOVYyMS41OEMzMiAyNy4zMzUgMjcuMzM1IDMyIDIxLjU4MSAzMkgxMC40MkM0LjY2NSAzMiAwIDI3LjMzNSAwIDIxLjU4MVYxMC40MkMwIDQuNjY1IDQuNjY1IDAgMTAuNDE5IDB6bTAgMS40ODhhOC45MyA4LjkzIDAgMDAtOC45MyA4LjkzdjExLjE2M2E4LjkzIDguOTMgMCAwMDguOTMgOC45M0gyMS41OGE4LjkzIDguOTMgMCAwMDguOTMtOC45M1YxMC40MmE4LjkzIDguOTMgMCAwMC04LjkzLTguOTNIMTAuNDJ6IiBmaWxsPSIjZmZmIi8+PHBhdGggZmlsbC1ydWxlPSJldmVub2RkIiBjbGlwLXJ1bGU9ImV2ZW5vZGQiIGQ9Ik0xNS45OTggMjYuMDQ5Yy01LjU0OSAwLTEwLjA0Ny00LjQ5OC0xMC4wNDctMTAuMDQ3IDAtNS41NDggNC40OTgtMTAuMDQ2IDEwLjA0Ny0xMC4wNDYgNS41NDggMCAxMC4wNDYgNC40OTggMTAuMDQ2IDEwLjA0NiAwIDUuNTQ5LTQuNDk4IDEwLjA0Ny0xMC4wNDYgMTAuMDQ3eiIgZmlsbD0iI2ZmZiIvPjxwYXRoIGQ9Ik0xMi43NjIgMTQuMjU0YzAtLjgyMi42NjctMS40ODkgMS40ODktMS40ODloMy40OTdjLjgyMiAwIDEuNDg4LjY2NiAxLjQ4OCAxLjQ4OXYzLjQ5N2MwIC44MjItLjY2NiAxLjQ4OC0xLjQ4OCAxLjQ4OGgtMy40OTdhMS40ODggMS40ODggMCAwMS0xLjQ4OS0xLjQ4OHYtMy40OTh6IiBmaWxsPSIjMTY1MkYwIi8+PC9zdmc+`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverting previous change b0c9041#diff-5963d849ba8821a176ba59be3d5c0ce674b78312019d36635730546fe81b296a

@bangtoven bangtoven marked this pull request as ready for review November 4, 2023 00:45
@bangtoven bangtoven merged commit 9a5ebab into master Nov 6, 2023
14 checks passed
@bangtoven bangtoven deleted the jungho/picker branch November 6, 2023 18:06
@bangtoven bangtoven mentioned this pull request Dec 8, 2023
Merged
@Brandylee24 Brandylee24 mentioned this pull request Jan 30, 2024
Open
bangtoven added a commit that referenced this pull request Feb 29, 2024
@bangtoven
Deprecating wallet picker (#1039)
38ea099 
* remove coinbase retail option

* Revert "remove coinbase retail option"

This reverts commit a8c5453.

* Revert "Revert "remove coinbase retail option""

This reverts commit 5b593da.

* update icons

* revert appSrc stuff

* cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cb-jake cb-jake cb-jake approved these changes

@fan-zhang-sv fan-zhang-sv Awaiting requested review from fan-zhang-sv

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bangtoven @cb-jake