Skip to content
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
4 changes: 4 additions & 0 deletions src/components/Nav/MobileMenu/MobileMenuClient.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { cn } from "@/lib/utils/cn"
import HamburgerButton from "./HamburgerButton"

import { useCloseOnNavigate } from "@/hooks/useCloseOnNavigate"
import { useTranslation } from "@/hooks/useTranslation"

type MobileMenuClientProps = {
className?: string
Expand All @@ -22,6 +23,7 @@ const MobileMenuClient = ({
side,
children,
}: MobileMenuClientProps) => {
const { t } = useTranslation("common")
const [open, setOpen] = useCloseOnNavigate()
const triggerRef = React.useRef<HTMLButtonElement>(null)

Expand All @@ -41,6 +43,8 @@ const MobileMenuClient = ({
className="flex flex-col"
onOpenChange={setOpen}
triggerRef={triggerRef}
aria-label={t("site-title")}
data-testid="mobile-menu-dialog"
>
{children}
</PersistentPanel>
Expand Down
8 changes: 8 additions & 0 deletions src/components/ui/persistent-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ interface PersistentPanelProps {
onOpenChange?: (open: boolean) => void
/** Ref to the trigger element - focus returns here on close */
triggerRef?: React.RefObject<HTMLElement | null>
/** Accessible label for the dialog */
"aria-label"?: string
/** Test ID for the dialog */
"data-testid"?: string
}

/**
Expand All @@ -34,6 +38,8 @@ const PersistentPanel = ({
children,
onOpenChange,
triggerRef,
"aria-label": ariaLabel,
"data-testid": dataTestId,
}: PersistentPanelProps) => {
// Track if component should be in DOM (lazy mount, stays mounted after first open)
const [isMounted, setIsMounted] = React.useState(false)
Expand Down Expand Up @@ -161,6 +167,8 @@ const PersistentPanel = ({
className={contentClasses}
role="dialog"
aria-modal="true"
aria-label={ariaLabel}
data-testid={dataTestId}
>
{children}
</div>
Expand Down
4 changes: 3 additions & 1 deletion tests/e2e/pages/BasePage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class BasePage {
this.mobileMenuButton = this.primaryNav.getByRole("button", {
name: /toggle menu button/i,
})
this.mobileSidebar = page.getByRole("dialog", { name: /ethereum.org/i })
this.mobileSidebar = page.getByRole("dialog")
}

/**
Expand Down Expand Up @@ -118,6 +118,8 @@ export class BasePage {
*/
async openLanguagePickerMobile(): Promise<void> {
await this.mobileMenuButton.click()
// Wait for the dialog to be visible before trying to find elements inside it
await expect(this.mobileSidebar).toBeVisible()
await this.mobileSidebar.getByTestId("mobile-menu-language-picker").click()
}

Expand Down
24 changes: 13 additions & 11 deletions tests/e2e/pages/FindWalletPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,12 @@ export class FindWalletPage extends BasePage {
const desktopCheckbox = desktopLabelParent.locator("button")
await desktopCheckbox.click()

const itemContainer = desktopLabelParent.locator("..")
const osOptionLabels = itemContainer.locator("label span.select-none")
await expect(osOptionLabels.first()).toBeVisible()

// Get OS options
const osOptions = await desktopLabelParent
.locator("..")
.locator("label span.select-none")
.allTextContents()
const osOptions = await osOptionLabels.allTextContents()

return { initialCount, osOptions }
}
Expand Down Expand Up @@ -168,11 +169,12 @@ export class FindWalletPage extends BasePage {
const mobileCheckbox = mobileLabelParent.locator("button")
await mobileCheckbox.click()

const itemContainer = mobileLabelParent.locator("..")
const osOptionLabels = itemContainer.locator("label span.select-none")
await expect(osOptionLabels.first()).toBeVisible()

// Get OS options
const osOptions = await mobileLabelParent
.locator("..")
.locator("label span.select-none")
.allTextContents()
const osOptions = await osOptionLabels.allTextContents()

// Close filters
await this.mobileFiltersSubmitButton.click()
Expand Down Expand Up @@ -219,9 +221,9 @@ export class FindWalletPage extends BasePage {
* Verify the row counter displays the provided expected count
*/
async verifyRowCounterEquals(expectedCount: number) {
const counterText = await this.rowCounter.textContent()

expect(counterText).toBe(`Showing all wallets (${expectedCount})`)
await expect(this.rowCounter).toHaveText(
`Showing all wallets (${expectedCount})`
)
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/start.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ test.describe("Start Page", () => {
const walletModal = page.getByRole("dialog")
await expect(walletModal.getByText("Connect a Wallet")).toBeVisible()
await expect(walletModal.getByText("MetaMask")).toBeVisible()
await expect(walletModal.getByText("Coinbase Wallet")).toBeVisible()
await expect(walletModal.getByText("Coinbase")).toBeVisible()
Copy link
Member

Choose a reason for hiding this comment

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

@pettinarip Hm, what am I missing? Looks like the wallet is named "Coinbase Wallet" in our wallet data

image image

Also, looks like we should probably update that separately: https://www.coinbase.com/wallet ("Coinbase Wallet is now Base app")

Copy link
Member Author

Choose a reason for hiding this comment

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

@wackerow this is for the /start page. The test is matching a few wallet names that appear in the connect modal.

await expect(walletModal.getByText("Rainbow")).toBeVisible()
})
})