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

hotfix: remove coinbase from default screen #3310

Merged
merged 33 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
9a128d5
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Sep 19, 2024
71f4058
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Sep 20, 2024
bc7b24e
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Sep 23, 2024
2b9a3db
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Sep 24, 2024
3bdbaec
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Sep 26, 2024
b60643a
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Sep 27, 2024
7db0f42
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Oct 2, 2024
34d19b4
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Oct 9, 2024
9ac3b69
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 13, 2024
e5d5f33
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 14, 2024
ecab080
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 14, 2024
2c7cd61
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 15, 2024
1065e16
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 15, 2024
9164df0
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 18, 2024
672b9e8
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 18, 2024
7715f95
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 19, 2024
3c67cb1
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 19, 2024
05646dd
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 20, 2024
7193992
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 20, 2024
7151a56
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 21, 2024
775803c
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 21, 2024
8df8a61
Merge branch 'main' of https://github.com/reown-com/appkit
svenvoskamp Nov 22, 2024
6f1fdac
ignore apps in changesets config
svenvoskamp Nov 22, 2024
2acbf3e
ignore manual
svenvoskamp Nov 22, 2024
cf27756
merge main
svenvoskamp Nov 22, 2024
76b7083
remove coinbase from default screen
svenvoskamp Nov 22, 2024
fb69fbf
redo config.json
svenvoskamp Nov 22, 2024
2b5062b
Merge branch 'main' of https://github.com/reown-com/appkit into hotfi…
svenvoskamp Nov 22, 2024
92af621
fix configparams
svenvoskamp Nov 22, 2024
e95bb77
fix test
svenvoskamp Nov 22, 2024
3c04c97
Merge branch 'main' into hotfix/remove-coinbase-default
svenvoskamp Nov 25, 2024
b9d45d0
add tests
svenvoskamp Nov 25, 2024
ee17cad
Merge branch 'main' into hotfix/remove-coinbase-default
svenvoskamp Nov 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/chilled-dingos-exercise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
'@reown/appkit-adapter-ethers5': patch
'@reown/appkit-adapter-ethers': patch
'@reown/appkit-scaffold-ui': patch
'@apps/demo': patch
'@apps/gallery': patch
'@reown/appkit-adapter-polkadot': patch
'@reown/appkit-adapter-solana': patch
'@reown/appkit-adapter-wagmi': patch
'@reown/appkit': patch
'@reown/appkit-utils': patch
'@reown/appkit-cdn': patch
'@reown/appkit-common': patch
'@reown/appkit-core': patch
'@reown/appkit-experimental': patch
'@reown/appkit-polyfills': patch
'@reown/appkit-siwe': patch
'@reown/appkit-siwx': patch
'@reown/appkit-ui': patch
'@reown/appkit-wallet': patch
---

Remove coinbase from default screen
1 change: 1 addition & 0 deletions apps/laboratory/src/pages/library/external.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const wagmiAdapter = new WagmiAdapter({
const modal = createAppKit({
adapters: [wagmiAdapter],
networks,
featuredWalletIds: ['fd20dc426fb37566d803205b19bbc1d4096b248ac04548e3cfb6b3a38bd033aa'],
defaultNetwork: mainnet,
projectId: ConstantsUtil.ProjectId,
features: {
Expand Down
16 changes: 16 additions & 0 deletions apps/laboratory/tests/basic-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ basicTest('Should show socials enabled by default', async () => {
await modalPage.closeModal()
})

basicTest('Should not show Coinbase by default', async () => {
await modalPage.page.getByTestId('connect-button').click()
await modalValidator.expectCoinbaseNotVisible()
await modalPage.closeModal()
})

basicTest('Should show external connectors', async ({ library }) => {
if (library !== 'wagmi') {
return
Expand All @@ -61,3 +67,13 @@ basicTest('Should show external connectors', async ({ library }) => {
await modalPage.page.getByTestId('connect-button').click()
await modalValidator.expectExternalVisible()
})

basicTest('Should show Coinbase as featured wallet', async ({ library }) => {
if (library !== 'wagmi') {
return
}

await modalPage.page.goto(`${BASE_URL}/library/external/`)
await modalPage.page.getByTestId('connect-button').click()
await modalValidator.expectCoinbaseVisible()
})
16 changes: 15 additions & 1 deletion apps/laboratory/tests/shared/validators/ModalValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,24 @@ export class ModalValidator {
}

async expectExternalVisible() {
const externalConnector = this.page.getByTestId(/^wallet-selector-external/u)
const externalConnector = this.page.getByTestId(
/^wallet-selector-external-externalTestConnector/u
)
await expect(externalConnector).toBeVisible()
}

async expectCoinbaseNotVisible() {
const coinbaseConnector = this.page.getByTestId(/^wallet-selector-external-coinbaseWalletSDK/u)
await expect(coinbaseConnector).not.toBeVisible()
}

async expectCoinbaseVisible() {
const coinbaseConnector = this.page.getByTestId(
/^wallet-selector-featured-fd20dc426fb37566d803205b19bbc1d4096b248ac04548e3cfb6b3a38bd033aa/u
)
await expect(coinbaseConnector).toBeVisible()
}

async expectMultipleAccounts() {
await this.page.waitForTimeout(500)
await expect(this.page.getByText('Switch Address')).toBeVisible({
Expand Down
11 changes: 2 additions & 9 deletions packages/adapters/ethers/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ export class EthersAdapter extends AdapterBlueprint {

if (this.namespace) {
this.addConnector({
id: connector,
id: key,
explorerId: PresetsUtil.ConnectorExplorerIds[key],
imageUrl: options?.connectorImages?.[key],
name: PresetsUtil.ConnectorNamesMap[key],
Expand Down Expand Up @@ -305,15 +305,8 @@ export class EthersAdapter extends AdapterBlueprint {
if (event.detail) {
const { info, provider } = event.detail
const existingConnector = this.connectors?.find(c => c.name === info?.name)
const coinbaseConnector = this.connectors?.find(
c => c.id === ConstantsUtil.COINBASE_SDK_CONNECTOR_ID
)
const isCoinbaseDuplicated =
coinbaseConnector &&
event.detail.info?.rdns ===
ConstantsUtil.CONNECTOR_RDNS_MAP[ConstantsUtil.COINBASE_SDK_CONNECTOR_ID]

if (!existingConnector && !isCoinbaseDuplicated) {
if (!existingConnector) {
const type = PresetsUtil.ConnectorTypesMap[ConstantsUtil.EIP6963_CONNECTOR_ID]

if (type && this.namespace) {
Expand Down
11 changes: 2 additions & 9 deletions packages/adapters/ethers5/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ export class Ethers5Adapter extends AdapterBlueprint {

if (this.namespace) {
this.addConnector({
id: connector,
id: key,
explorerId: PresetsUtil.ConnectorExplorerIds[key],
imageUrl: options?.connectorImages?.[key],
name: PresetsUtil.ConnectorNamesMap[key],
Expand Down Expand Up @@ -306,15 +306,8 @@ export class Ethers5Adapter extends AdapterBlueprint {
if (event.detail) {
const { info, provider } = event.detail
const existingConnector = this.connectors?.find(c => c.name === info?.name)
const coinbaseConnector = this.connectors?.find(
c => c.id === ConstantsUtil.COINBASE_SDK_CONNECTOR_ID
)
const isCoinbaseDuplicated =
coinbaseConnector &&
event.detail.info?.rdns ===
ConstantsUtil.CONNECTOR_RDNS_MAP[ConstantsUtil.COINBASE_SDK_CONNECTOR_ID]

if (!existingConnector && !isCoinbaseDuplicated) {
if (!existingConnector) {
const type = PresetsUtil.ConnectorTypesMap[ConstantsUtil.EIP6963_CONNECTOR_ID]

if (type && this.namespace) {
Expand Down
1 change: 1 addition & 0 deletions packages/adapters/wagmi/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export class WagmiAdapter extends AdapterBlueprint {
})
this.namespace = CommonConstantsUtil.CHAIN.EVM
this.createConfig({
...configParams,
Copy link
Contributor

Choose a reason for hiding this comment

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

Very crucial 🙏🏽

Copy link
Contributor

@enesozturk enesozturk Nov 25, 2024

Choose a reason for hiding this comment

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

If no go for short time can we separate this to another PR and merge asap?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I'm doing here #3324

networks: CaipNetworksUtil.extendCaipNetworks(configParams.networks, {
projectId: configParams.projectId,
customNetworkImageUrls: {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,19 @@ export class W3mConnectExternalWidget extends LitElement {
// -- Render -------------------------------------------- //
public override render() {
const externalConnectors = this.connectors.filter(connector => connector.type === 'EXTERNAL')
const filteredOutCoinbaseConnectors = externalConnectors.filter(
connector => connector.id !== 'coinbaseWalletSDK'
)
Comment on lines +32 to +34
Copy link
Collaborator

Choose a reason for hiding this comment

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

have you tried what would happen if it were to be one of the first results of the list / featured?

In that case its valid to be on first screen

I think it would have this same ID and get filtered but not completely sure

Copy link
Contributor

Choose a reason for hiding this comment

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

+different E2E test variations of featuredWalletIds use cases, would be really beneficial

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wouldn't be filtered out if it is added with the featuredWalletIds list, because those wallets will be rendered in the connect-featured-widget, but I will add some tests


if (!externalConnectors?.length) {
if (!filteredOutCoinbaseConnectors?.length) {
this.style.cssText = `display: none`

return null
}

return html`
<wui-flex flexDirection="column" gap="xs">
${externalConnectors.map(
${filteredOutCoinbaseConnectors.map(
connector => html`
<wui-list-wallet
imageSrc=${ifDefined(AssetUtil.getConnectorImage(connector))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export class W3mConnectFeaturedWidget extends LitElement {
${wallets.map(
wallet => html`
<wui-list-wallet
data-testid=${`wallet-selector-featured-${wallet.id}`}
imageSrc=${ifDefined(AssetUtil.getWalletImage(wallet))}
name=${wallet.name ?? 'Unknown'}
@click=${() => this.onConnectWallet(wallet)}
Expand Down
Loading