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

Flip updated dApp connections flag #3492

Merged
merged 13 commits into from
Jul 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .env.defaults
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ SUPPORT_SWAP_QUOTE_REFRESH=false
SUPPORT_ACHIEVEMENTS_BANNER=false
SUPPORT_NFT_SEND=false
USE_MAINNET_FORK=false
ENABLE_UPDATED_DAPP_CONNECTIONS=false
ENABLE_UPDATED_DAPP_CONNECTIONS=true
108 changes: 88 additions & 20 deletions e2e-tests/dapp-connect.spec.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,96 @@
import { test } from "./utils"
import { test, expect } from "./utils"

test("dapp connect", async ({ page, context, walletPageHelper }) => {
await walletPageHelper.onboarding.addReadOnlyAccount("testertesting.eth")
test.describe("dApp Connections", () => {
test("should display an informational popup for Taho as default on first connection", async ({
context,
walletPageHelper,
}) => {
await walletPageHelper.onboarding.addReadOnlyAccount("testertesting.eth")

const dappPage = await context.newPage()
await dappPage.goto("https://swap.cow.fi/")
await dappPage
.locator("#swap-button")
.getByRole("button", { name: "Connect Wallet" })
.click()
const dappPage = await context.newPage()
await dappPage.goto("https://swap.cow.fi/")
await dappPage
.locator("#swap-button")
.getByRole("button", { name: "Connect Wallet" })
.click()

// Get page after a specific action (e.g. clicking a link)
const [popupPage] = await Promise.all([
context.waitForEvent("page"),
await dappPage.locator("text=Injected").click(), // Opens a new tab
])
await popupPage.waitForLoadState()
// Get page after a specific action (e.g. clicking a link)
const [popupPage] = await Promise.all([
context.waitForEvent("page"),
await dappPage.locator("text=Injected").click(), // Opens a new tab
])
await popupPage.waitForLoadState()

await popupPage.locator("button", { hasText: "Connect" }).click()
// Clear the one-time informational popup, if present.
const connectingPopupTitle = popupPage.locator("h3", {
hasText: "Connecting with Taho",
})

await walletPageHelper.goToStartPage()
expect(await connectingPopupTitle.count()).toBe(1)
await expect(connectingPopupTitle).toBeVisible()

await page.locator('text="Settings"').click()
await page.locator("text=Connected websites").click()
// Clear the popover.
const popupCloseLocator = popupPage.getByRole("button", {
name: "Background close",
})

await page.locator('xpath=//li[contains(., "CoW Swap")]//button').click()
await popupCloseLocator.click()
await popupCloseLocator.waitFor({ state: "detached", timeout: 1000 })

await popupPage.locator("button", { hasText: "Reject" }).click()

await dappPage.close()

const dappPage2 = await context.newPage()
await dappPage2.goto("https://swap.cow.fi/")
await dappPage2
.locator("#swap-button")
.getByRole("button", { name: "Connect Wallet" })
.click()

// Get page after a specific action (e.g. clicking a link)
const [popupPage2] = await Promise.all([
context.waitForEvent("page"),
await dappPage2.locator("text=Injected").click(), // Opens a new tab
])
await popupPage2.waitForLoadState()

// Check that the popup is no longer displayed.
const connectingPopupTitle2 = popupPage2.locator("h3", {
hasText: "Connecting with Taho",
})
expect(await connectingPopupTitle2.count()).toBe(0)
})

test("should work and add an entry to the connected websites list", async ({
page,
context,
walletPageHelper,
}) => {
await walletPageHelper.onboarding.addReadOnlyAccount("testertesting.eth")
await walletPageHelper.hideDappConnectPopup()

const dappPage = await context.newPage()
await dappPage.goto("https://swap.cow.fi/")
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably worth running one small dApp along hardhat for some standard tests of this kind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would really love to avoid more stuff we have to maintain tbh, but agree this is a pretty kludgey way to do it.

await dappPage
.locator("#swap-button")
.getByRole("button", { name: "Connect Wallet" })
.click()

// Get page after a specific action (e.g. clicking a link)
const [popupPage] = await Promise.all([
context.waitForEvent("page"),
await dappPage.locator("text=Injected").click(), // Opens a new tab
])
await popupPage.waitForLoadState()

await popupPage.locator("button", { hasText: "Connect" }).click()

await walletPageHelper.goToStartPage()

await page.locator('text="Settings"').click()
await page.locator("text=Connected websites").click()

await page.locator('xpath=//li[contains(., "CoW Swap")]//button').click()
})
})
1 change: 1 addition & 0 deletions e2e-tests/signing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ test.describe("Signing", () => {
walletPageHelper,
}) => {
await walletPageHelper.onboarding.addNewWallet()
await walletPageHelper.hideDappConnectPopup()

const siwe = await context.newPage()
await siwe.goto("https://login.xyz")
Expand Down
35 changes: 35 additions & 0 deletions e2e-tests/utils/walletPageHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,39 @@ export default class WalletPageHelper {
this.popup.getByTestId("top_menu_network_switcher").last()
).toHaveText(network)
}

/**
* Hides the dApp Connection "use Taho as default" informational popup so
* tests can proceed assuming dApp connection will be available without
* additional interactions.
*/
async hideDappConnectPopup(): Promise<void> {
const dappPage = await this.context.newPage()
await dappPage.goto("https://swap.cow.fi/")
await dappPage
.locator("#swap-button")
.getByRole("button", { name: "Connect Wallet" })
.click()

const [popupPage] = await Promise.all([
this.context.waitForEvent("page"),
await dappPage.locator("text=Injected").click(), // Opens a new tab
])
await popupPage.waitForLoadState()

// Clear the one-time informational popup, if present.
const connectingPopupTitle = popupPage.locator("h3", {
hasText: "Connecting with Taho",
})
if ((await connectingPopupTitle.count()) > 0) {
await expect(connectingPopupTitle).toBeVisible()
const bgLocator = popupPage.locator(".bg")

await bgLocator.click()
await bgLocator.waitFor({ state: "detached", timeout: 1000 })
}

await popupPage.close()
await dappPage.close()
}
}
2 changes: 1 addition & 1 deletion env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ interface Window {
shouldSetTally: boolean,
shouldReload?: boolean
) => void
routeToNewDefault: (request: unknown) => Promise<unknown>
routeToNewNonTahoDefault: (request: unknown) => Promise<unknown>
getProviderInfo: (
provider: WalletProvider
) => WalletProvider["providerInfo"]
Expand Down
4 changes: 2 additions & 2 deletions src/popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ setTimeout(() => {
}, 1000)

const POPUP_WIDTH = 384
// const POPUP_HEIGHT = 600
const POPUP_HEIGHT = 600

/**
* Because browser.windows.create(...) takes a height and width that includes
Expand All @@ -48,7 +48,7 @@ const POPUP_WIDTH = 384
*/
const missingWidth = POPUP_WIDTH - window.innerWidth
// TODO: Change this to use POPUP_HEIGHT and test signing and other popups
const missingHeight = window.outerHeight - window.innerHeight
const missingHeight = POPUP_HEIGHT - window.innerHeight

window.resizeBy(missingWidth, missingHeight)

Expand Down
19 changes: 15 additions & 4 deletions src/window-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,18 @@ if (!window.walletRouter) {
}, 1000)
}
},
routeToNewDefault(request: Required<RequestArgument>): Promise<unknown> {
routeToNewNonTahoDefault(
request: Required<RequestArgument>
): Promise<unknown> {
// Don't route to a new default if it's Taho. This avoids situations
// where Taho is default, then default is turned off, but no other
// provider is installed, so that we don't try to reroute back to Taho
// as the only other provider.
if (this.currentProvider === this.tallyProvider) {
return Promise.reject(
new Error("Only the Taho provider is installed.")
)
}
return this.currentProvider.request(request)
},
getProviderInfo(provider: WalletProvider) {
Expand Down Expand Up @@ -150,9 +161,9 @@ Object.defineProperty(window, "ethereum", {
// Always proxy to the current provider, even if it has changed. This
// allows changes in the current provider, particularly when the user
// changes their default wallet, to take effect immediately. Combined
// with walletRouter.routeToNewDefault, this allows Taho to effect a
// change in provider without a page reload or even a second attempt
// at connecting.
// with walletRouter.routeToNewNonTahoDefault, this allows Taho to
// effect a change in provider without a page reload or even a second
// attempt at connecting.
window.walletRouter?.currentProvider ?? target,
prop,
receiver
Expand Down
12 changes: 6 additions & 6 deletions ui/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@
"viewAll": "View all"
},
"topMenu": {
"showCurrentDappConnection": "Show current dApp connection",
"showCurrentDappConnection": "Show current website connection",
"connectToWebsiteUsing": "Connect to website using:",
"setTahoAsDefault": "Set Taho as default",
"setOtherAsDefault": "Set MetaMask/other wallet as default",
Expand All @@ -825,11 +825,11 @@
},
"connectedDappInfo": {
"dAppTitle": "Account connected to",
"dappConnections": "Dapp connections",
"dappConnections": "Website connections",
"guideline": {
"title": "How to connect to dapps",
"step1": "Set Taho as default",
"step2": "Click “connect to wallet” in a dapp",
"title": "How to connect to websites",
"step1": "Connect using Taho",
"step2": "Click “connect” on a website",
"step3": "Select one similar to these"
},
"walletConnectInfo": "You can now connect to any dapp that supports Wallet Connect by selecting Taho from desktop tab. <url>Learn more</url>",
Expand Down Expand Up @@ -891,7 +891,7 @@
"snackbar": "You can change this in Settings later",
"isDefault": "is now your default wallet",
"notDefault": "is not your default wallet",
"tooltip": "Setting Taho as your default wallet means that every time you connect to a dApp, Taho will open instead of MetaMask or other wallets."
"tooltip": "Setting Taho as your default wallet means that every time you connect to a website, Taho will open instead of MetaMask or other wallets."
},
"analyticsNotification": {
"title": "Analytics are enabled",
Expand Down
9 changes: 8 additions & 1 deletion ui/components/TopMenu/TopMenuConnectedDAppInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { FeatureFlags, isEnabled } from "@tallyho/tally-background/features"
import classNames from "classnames"
import React, { ReactElement, useCallback, useState } from "react"
import { Trans, useTranslation } from "react-i18next"
import DAppConnectionDefaultToggle from "../DAppConnection/DAppConnectionDefaultToggle"
import SharedAccordion from "../Shared/SharedAccordion"
import SharedLink from "../Shared/SharedLink"
import SharedPanelSwitcher from "../Shared/SharedPanelSwitcher"
Expand Down Expand Up @@ -80,7 +81,13 @@ function ConnectionDAppGuideline({
<li>
<span className="wallet_toggle_wrap">
{t("guideline.step1")}
<WalletDefaultToggle />
{isEnabled(
FeatureFlags.ENABLE_UPDATED_DAPP_CONNECTIONS
) ? (
<DAppConnectionDefaultToggle alwaysForceSelection="taho" />
) : (
<WalletDefaultToggle />
)}
</span>
</li>
<li>{t("guideline.step2")}</li>
Expand Down
22 changes: 20 additions & 2 deletions ui/pages/DAppConnect/DAppConnectPage.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { ReactElement, useEffect } from "react"
import React, { ReactElement, useEffect, useState } from "react"
import { AccountTotal } from "@tallyho/tally-background/redux-slices/selectors"
import { PermissionRequest } from "@tallyho/provider-bridge-shared"
import { useTranslation } from "react-i18next"
Expand Down Expand Up @@ -34,17 +34,35 @@ export default function DAppConnectPage({
const { title, origin, faviconUrl, accountAddress } = permission

const isDefaultWallet = useBackgroundSelector(selectDefaultWallet)
const [wasDefaultWallet, setWasDefaultWallet] = useState(isDefaultWallet)

useEffect(() => {
if (
isEnabled(FeatureFlags.ENABLE_UPDATED_DAPP_CONNECTIONS) &&
wasDefaultWallet &&
!isDefaultWallet
) {
// When default wallet is toggled off, wait for the toggle animation to
// complete and close the window out.
setTimeout(() => window.close(), 300)
}
}, [isDefaultWallet])
}, [wasDefaultWallet, isDefaultWallet])

useEffect(() => {
if (
isEnabled(FeatureFlags.ENABLE_UPDATED_DAPP_CONNECTIONS) &&
!wasDefaultWallet &&
isDefaultWallet
) {
// When default wallet is toggled on, make sure wasDefaultWallet is
// updated. This handles the situation where the wallet is not default,
// then a connection popup appears, then the user toggles it to default,
// then they toggle it back off---in that scenario, we want the same
// behavior as if the popup had been set as default and then it was
// turned off.
setWasDefaultWallet(true)
}
}, [wasDefaultWallet, isDefaultWallet])

return (
<>
Expand Down
2 changes: 1 addition & 1 deletion ui/pages/DAppConnectRequest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export default function DAppConnectRequest(): ReactElement {
.page {
background-color: var(--green-95);
height: 100vh;
width: 100vw;
width: 100%;
z-index: 1000;
}
`}</style>
Expand Down
1 change: 1 addition & 0 deletions ui/public/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ html,
body,
#tally-root {
height: 100%;
width: 100%;
}

body.popup {
Expand Down
2 changes: 1 addition & 1 deletion window-provider/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export default class TallyWindowProvider extends EventEmitter {
.sort(([id], [id2]) => Number(BigInt(id2) - BigInt(id)))
.forEach(([, { sendData, reject, resolve }]) => {
window.walletRouter
?.routeToNewDefault(sendData.request)
?.routeToNewNonTahoDefault(sendData.request)
// On success or error, call the original reject/resolve
// functions to notify the requestor of the new wallet's
// response.
Expand Down