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 8 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
106 changes: 86 additions & 20 deletions e2e-tests/dapp-connect.spec.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,94 @@
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 bgLocator = popupPage.locator(".bg")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we target the "void space" button instead? Something like this should work:

        popupPage
        .getByRole("button", { name: "Background close" })
        .click()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that was my original approach and then I ran into issues that I thought were related but ultimately weren't, and then never changed it back. I'll poke at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


await page.locator('xpath=//li[contains(., "CoW Swap")]//button').click()
await bgLocator.click()
await bgLocator.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()
}
}
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
12 changes: 6 additions & 6 deletions ui/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,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 @@ -746,11 +746,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 @@ -812,7 +812,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
6 changes: 4 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,19 @@ export default function DAppConnectPage({
const { title, origin, faviconUrl, accountAddress } = permission

const isDefaultWallet = useBackgroundSelector(selectDefaultWallet)
const [wasDefaultWallet] = 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])

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