@@ -253,8 +253,8 @@ Make FamilySync configurable, administrable, and maintainable for real multi-mem
- Desktop WebKit is optional — the Apple member is already covered on mobile Safari via `iphone`; Desktop Chrome is likely sufficient for a shared/wall browser.
- Desktop WebKit is optional — the Apple member is already covered on mobile Safari via `iphone`; Desktop Chrome is likely sufficient for a shared/wall browser.
**Plans**: 1 plan
**Plans**: 1 plan
Plans:
Plans:
- [ ] 14-01-PLAN.md — Add the `desktop` Playwright project, desktop-skip the two mobile-only layout assertions (+ D-04 parity), update spec/README docs, and prove `pnpm test:e2e` is green on iphone + pixel + desktop with a blocking CI gate.
- [ ] 14-01-PLAN.md — Add the `desktop` Playwright project, desktop-skip the two mobile-only layout assertions (+ D-04 parity), update spec/README docs, and prove `pnpm test:e2e` is green on iphone + pixel + desktop with a blocking CI gate.
**UI hint**: no
**UI hint**: no
@@ -474,6 +474,7 @@ Plans:
**The footgun:** branch protection currently requires three contexts — `CI / fast-checks`, `CI / api`, `CI / harness`. The naive fix (`paths-ignore` on the workflow, or path-filtering `harness`) **deadlocks**: on a docs-only PR the required `harness`/`api` contexts never report, so the PR can never merge. A required check that never reports is worse than a slow one.
**The footgun:** branch protection currently requires three contexts — `CI / fast-checks`, `CI / api`, `CI / harness`. The naive fix (`paths-ignore` on the workflow, or path-filtering `harness`) **deadlocks**: on a docs-only PR the required `harness`/`api` contexts never report, so the PR can never merge. A required check that never reports is worse than a slow one.
**Solution (Option A — aggregate gate):**
**Solution (Option A — aggregate gate):**
- Add a cheap `changes` detector job: `git diff --name-only base...HEAD`, set `code=true` unless every changed path matches `docs/` or `*.md`.
- Add a cheap `changes` detector job: `git diff --name-only base...HEAD`, set `code=true` unless every changed path matches `docs/` or `*.md`.
- Gate the heavy jobs: `api` and `harness` get `needs: changes` + `if: needs.changes.outputs.code == 'true'`.
- Gate the heavy jobs: `api` and `harness` get `needs: changes` + `if: needs.changes.outputs.code == 'true'`.
- Add an always-running `gate` job: `needs: [fast-checks, api, harness]`, `if: always()`, passes when each dependency `result` is `success` OR `skipped`.
- Add an always-running `gate` job: `needs: [fast-checks, api, harness]`, `if: always()`, passes when each dependency `result` is `success` OR `skipped`.
**Self-analog** — read carefully; two `test.describe` blocks need desktop skip guards, one test needs a desktop parity assertion, one test passes unchanged.
#### Skip mechanism in the existing codebase
The one existing `test.skip` in the suite (from `calendar.spec.ts` lines 59–62) uses the inline conditional form:
```typescript
test.skip(
!swAvailable,
'navigator.serviceWorker is unavailable in this context (e.g. WebKit over http://localhost) — block is unobservable here',
);
```
This form — `test.skip(condition, reason)` called at the top of the test body — is the established pattern. Do **not** use `test.skip(testInfo.project.name === 'desktop', ...)` with a `testInfo` parameter; the simpler form without `testInfo` is consistent with what already exists.
For project-name gating the correct signature requires the `testInfo` fixture:
Use this signature for every mobile-only test that needs a desktop skip. Keep the reason string explicit and factual (consistent with the SW-block skip's style).
#### Tests in `layout.spec.ts` that need a desktop skip
**Test at line 62 — "BottomTabBar is fully in-viewport (Rule 3 — safe-area-inset)":**
// The bar uses env(safe-area-inset-bottom, 0px). In emulation there is no
// safe-area-inset, so the bar's bottom edge must be ≤ viewport height.
const nav = page.getByRole('navigation', { name: 'Main navigation' });
await expect(nav).toBeVisible();
const box = await nav.boundingBox();
// ...
```
On desktop `BottomTabBar.tsx:53-57` returns `null` at ≥768px, so this `nav` would resolve to DesktopNav sidebar and the safe-area-inset assertion is semantically wrong for a sidebar. **Skip on desktop.**
**Test at line 130 — "BottomTabBar is fully in-viewport on /lists (Rule 3)"** — same reason. **Skip on desktop.**
**Test at line 91 — "New Event FAB meets 56×56px touch-target minimum (Rule 1)":**
// Phone-only FAB — aria-label="New Event", fixed 56×56px (CalendarShell.tsx)
const fab = page.getByRole('button', { name: 'New Event' });
const box = await fab.boundingBox();
expect(box, 'New Event FAB bounding box must not be null').not.toBeNull();
expect(box!.width, 'New Event FAB width ≥ 56px').toBeGreaterThanOrEqual(56);
expect(box!.height, 'New Event FAB height ≥ 56px').toBeGreaterThanOrEqual(56);
});
```
On desktop `getByRole('button', { name: 'New Event' })` resolves to the **desktop toolbar button** (CalendarShell.tsx:436–457), not the 56×56px FAB. The toolbar button has `minHeight: 44px` but no 56px constraint. **Skip the FAB-geometry (56×56) assertion on desktop.** Per D-04, add a desktop parity block asserting ≥44px instead.
Desktop parity assertion to add (new test or a conditional branch in the same test):
#### Tests that pass unchanged on desktop (no modification needed)
- **"BottomTabBar navigation landmark is visible" (lines 35–41 and 108–110):** `getByRole('navigation', { name: 'Main navigation' })` resolves to DesktopNav sidebar on desktop (sole nav landmark at ≥768px). Passes as-authored.
- **"Calendar tab meets 44×44px" / "Lists tab meets 44×44px" (lines 43–60, 112–128):** Scoped to the `Main navigation` landmark; DesktopNav sidebar links have `minHeight: 44px` (AppNav.tsx:151). Pass as-authored.
- **"PhoneNav header is visible" (line 76):** `getByText('FamilySync', { exact: true })` matches the DesktopNav title text (AppNav.tsx:181). Passes on desktop. Verify no strict-mode collision (CONTEXT.md confirms the PhoneNav `<header>` returns null at ≥768px, leaving the DesktopNav title as the sole match).
- **"PhoneNav settings button meets 44×44px" (line 82):** `getByRole('button', { name: /open settings/i })` is present on both PhoneNav and DesktopNav. Passes as-authored.
- **Rule 2 overflow tests (lines 142–165):** Purely DOM measurement. Pass unchanged on desktop.
- **Harness self-validation injected-defect proofs (lines 175–256):** Use `nav[aria-label="Main navigation"]` CSS selector and body width injection. Pass unchanged on desktop.
#### `describe` block header comments to update
The jsdoc block at the top of `layout.spec.ts` (lines 1–25) currently says:
```
* Runs on both device profiles automatically (playwright.config.ts matrix):
* iphone: iPhone 14 / WebKit / 390×844
* pixel: Pixel 7 / Chromium / 412×915
```
Update to list all three profiles (same update applies to all spec file headers).
---
### `apps/pwa/e2e/calendar.spec.ts` — no structural changes needed
All tests in this file pass unchanged on desktop:
- **Auth-bypass precondition (line 26):** waits for `Main navigation` landmark — resolves to DesktopNav sidebar on desktop. Passes.
- **Populated state tests (lines 81–111):**`.sx-react-calendar-wrapper` and `getByText('Seeded Test Event')` are not viewport-dependent. Pass unchanged.
- **Error state tests (lines 116–178):**`page.route` + heading/button assertions are not viewport-dependent. Pass unchanged.
Only the file's header comment block (lines 12–15) needs updating to list the `desktop` project.
---
### `apps/pwa/e2e/lists.spec.ts` — no structural changes needed
All tests pass unchanged on desktop — no mobile-only assumptions anywhere in this file. Only the header comment (lines 12–15) needs updating to list the `desktop` project.
---
### `apps/pwa/e2e/README.md` — docs update only
**Current run commands block (lines 37–47):**
```bash
# Full suite — both iPhone (WebKit) and Pixel (Chromium) profiles
pnpm --filter @familysync/pwa test:e2e
# Single profile (faster local iteration)
pnpm --filter @familysync/pwa exec playwright test --project=pixel
# Headed (local debug — shows the browser)
pnpm --filter @familysync/pwa exec playwright test --headed
# UI mode (interactive test explorer)
pnpm --filter @familysync/pwa test:e2e:ui
```
Update the comment on the full-suite command and add a desktop-specific example:
```bash
# Full suite — iPhone (WebKit), Pixel (Chromium), Desktop Chrome profiles
pnpm --filter @familysync/pwa test:e2e
# Single profile (faster local iteration)
pnpm --filter @familysync/pwa exec playwright test --project=pixel
pnpm --filter @familysync/pwa exec playwright test --project=desktop
```
Also update the preamble sentence (line 3) which currently says "mobile-emulated (iPhone 14/WebKit + Pixel 7/Chromium)" — add "Desktop Chrome (1280×720)".
**Apply to:** any new desktop assertion that needs auth-ready confirmation
```typescript
const nav = page.getByRole('navigation', { name: 'Main navigation' });
await expect(nav).toBeVisible();
```
On mobile: resolves to `BottomTabBar` nav. On desktop: resolves to `DesktopNav` sidebar. Same locator, different element — no conditional needed.
### Desktop "New Event" button locator (CalendarShell.tsx:436–457)
**Source:** `CalendarShell.tsx` lines 424–459
The desktop toolbar button is rendered inside `{!phone && (...)}` with plain text `New Event` (no `aria-label` attribute). Playwright resolves it by accessible name from inner text:
```typescript
// Resolves to desktop toolbar button at ≥768px (has minHeight:44px per line 443)
// Resolves to phone FAB at <768px (has aria-label="New Event" per line 466)
page.getByRole('button', { name: 'New Event' })
```
At 1280px (`Desktop Chrome`) only the toolbar button renders; the FAB is in `{phone && (...)}` which is false. **No strict-mode collision.**
---
## No Analog Found
None. All files being modified are established; all new code copies directly from existing patterns in the same files.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.