Skip to content

Commit 57ab344

Browse files
committed
Let a room tile in the settings sheet actually reach its room
Every drawer in the room is a URL param, so closing one is itself a router push. The switcher tiles closed the sheet AND followed their own href, which fired two router pushes from a single click — the query-param one won, and the room never changed. Tapping a tile did nothing at all. Nothing closes the sheet now. The targets carry no `settings` param, so arriving is what closes it, and one click means one navigation. The strip had no tap coverage at all: the earlier fix proved it could be scrolled, never that a tile worked. The new spec fails on both projects without this change.
1 parent 13490ee commit 57ab344

3 files changed

Lines changed: 78 additions & 4 deletions

File tree

‎apps/web/e2e/switcher-tap.spec.ts‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { expect, test } from '@playwright/test'
2+
3+
/**
4+
* Tapping a tile in the other-rooms strip has to actually go to that room.
5+
*
6+
* Scrolling the strip was fixed by handing horizontal panning back to the browser
7+
* (`data-vaul-no-drag` + `touch-action: pan-x`). This spec covers the other half of the
8+
* gesture — the tap — which nothing asserted before, and which was broken: the tile also
9+
* closed the sheet, and because the sheet is a URL param that close was a second router
10+
* push in the same click. It won, and the room never changed.
11+
*/
12+
13+
test.use({ extraHTTPHeaders: { 'x-forwarded-for': '198.51.100.77' } })
14+
15+
async function createRoom(page: import('@playwright/test').Page, name: string) {
16+
await page.goto('/new')
17+
await page.getByTestId('room-name').fill(name)
18+
await page.getByTestId('creator-name').fill('Ana')
19+
await page.getByTestId('create-room').click()
20+
await expect(page.getByTestId('room-link')).toBeVisible({ timeout: 15_000 })
21+
await page.getByTestId('go-to-room').click()
22+
await expect(page.getByTestId('open-room-settings')).toBeVisible({ timeout: 15_000 })
23+
return new URL(page.url()).pathname.split('/')[2]
24+
}
25+
26+
test('tapping a room tile navigates to that room', async ({ page }) => {
27+
const alpha = await createRoom(page, 'Alpha')
28+
const beta = await createRoom(page, 'Beta')
29+
expect(beta).not.toBe(alpha)
30+
31+
await page.getByTestId('open-room-settings').click()
32+
await expect(page.getByTestId('settings-sheet')).toBeVisible({ timeout: 10_000 })
33+
await page.waitForTimeout(600)
34+
35+
const tile = page.locator(`[data-testid="room-switcher-tile"][data-slug="${alpha}"]`)
36+
await expect(tile).toBeVisible()
37+
38+
await tile.click()
39+
await expect(page).toHaveURL(new RegExp(`/r/${alpha}$`), { timeout: 10_000 })
40+
// Arriving is what closes the sheet — nothing calls onClose.
41+
await expect(page.getByTestId('settings-sheet')).toBeHidden({ timeout: 10_000 })
42+
await expect(page.locator('h1').first()).toHaveText('Alpha')
43+
})
44+
45+
test('the all-rooms tile leaves the room', async ({ page }) => {
46+
await createRoom(page, 'Alpha')
47+
await createRoom(page, 'Beta')
48+
49+
await page.getByTestId('open-room-settings').click()
50+
await expect(page.getByTestId('settings-sheet')).toBeVisible({ timeout: 10_000 })
51+
await page.waitForTimeout(600)
52+
53+
await page.getByTestId('room-switcher-all').click()
54+
await expect(page).toHaveURL(/\/$/, { timeout: 10_000 })
55+
})
56+
57+
test('tapping a theme swatch repaints the room', async ({ page }) => {
58+
await createRoom(page, 'Palette')
59+
60+
await page.getByTestId('open-room-settings').click()
61+
await expect(page.getByTestId('settings-sheet')).toBeVisible({ timeout: 10_000 })
62+
await page.waitForTimeout(600)
63+
64+
const swatch = page.locator('[data-testid="theme-swatch"]').nth(2)
65+
const key = await swatch.getAttribute('data-theme')
66+
await swatch.click()
67+
await page.waitForTimeout(2_000)
68+
await expect(swatch).toHaveAttribute('aria-pressed', 'true')
69+
expect(key).toBeTruthy()
70+
})

‎apps/web/src/components/room/RoomSwitcher.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,14 @@ import { themeFor } from '@/lib/themes'
1919
* `readRecentRooms()` also returns `[]` when storage is blocked, which is the
2020
* same branch and the same right answer: a device that cannot remember rooms has
2121
* no list to offer.
22+
*
23+
* Nothing here closes the sheet, and that is deliberate. Every drawer in the room
24+
* is a URL param (`?settings=1`), so closing one is itself a router push — and a
25+
* tile that both closed the sheet and followed its `href` fired two router pushes
26+
* from one click, where the query-param one won and the room never changed. The
27+
* targets carry no `settings` param, so arriving is what closes the sheet.
2228
*/
23-
export function RoomSwitcher({ currentSlug, onNavigate }: { currentSlug: string; onNavigate: () => void }) {
29+
export function RoomSwitcher({ currentSlug }: { currentSlug: string }) {
2430
const t = useTranslations('room.header')
2531
// Read after mount: the list is localStorage, and rendering it during the
2632
// server pass would be a hydration mismatch on every device that has one.
@@ -51,7 +57,6 @@ export function RoomSwitcher({ currentSlug, onNavigate }: { currentSlug: string;
5157
<li key={room.slug}>
5258
<Link
5359
href={`/r/${room.slug}`}
54-
onClick={onNavigate}
5560
data-testid="room-switcher-tile"
5661
data-slug={room.slug}
5762
// Inline for the same reason the theme picker does it:
@@ -70,7 +75,6 @@ export function RoomSwitcher({ currentSlug, onNavigate }: { currentSlug: string;
7075
page that owns the full list and the paste-a-link recovery. */}
7176
<Link
7277
href="/"
73-
onClick={onNavigate}
7478
data-testid="room-switcher-all"
7579
className="flex w-24 shrink-0 flex-col items-center gap-1 rounded-sm border border-dashed border-n-1 bg-white p-2 transition-transform duration-100 active:translate-y-[2px]"
7680
>

‎apps/web/src/components/room/SettingsSheet.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ export function SettingsSheet({
201201
<CloseButton onClick={onClose} label={t('close')} data-testid="close-room-settings" />
202202
</DrawerHeader>
203203
<DrawerBody>
204-
<RoomSwitcher currentSlug={room.slug} onNavigate={onClose} />
204+
<RoomSwitcher currentSlug={room.slug} />
205205

206206
<section
207207
data-testid="room-card"

0 commit comments

Comments
 (0)