240 lines
16 KiB
Markdown
240 lines
16 KiB
Markdown
# FN-398 — Subscription Page Mobile Remainder (P1)
|
||
|
||
## Task
|
||
- **ID:** FN-398
|
||
- **Priority:** P1 — bottom-of-funnel, revenue-critical
|
||
- **Single source file:** `apps/web/src/routes/dashboard/subscription/index.tsx`
|
||
- **Single test folder:** `apps/web/src/routes/dashboard/subscription/__tests__/`
|
||
- Existing files to extend: `cls-regression.test.tsx`, `grid-parity.test.tsx`
|
||
- **Dependencies:** FN-395 (P1-A shortlist; informational only — no code dependency).
|
||
|
||
This task fixes three specific defects (one CLS, one dialog overflow, one safe-area padding) on the subscription / plan-selection page. All requirements below are stated inline; you do **not** need to read any external audit document to execute.
|
||
|
||
---
|
||
|
||
## In-scope defects (verbatim requirements — inline, authoritative)
|
||
|
||
### Defect A — Skeleton grid CLS parity (audit §6)
|
||
|
||
**Symptom:** When the subscription page is loading, the skeleton placeholders use a different grid than the real plan-cards grid. When the real data arrives, the layout jumps (Cumulative Layout Shift).
|
||
|
||
**Real grid (already correct in source, currently `index.tsx` line ~989):**
|
||
```tsx
|
||
<div className="grid gap-4 sm:grid-cols-2 lg:grid-cols-4">
|
||
{plans.map((plan) => /* plan card */)}
|
||
</div>
|
||
```
|
||
- `plans` is the exported array of plan definitions in `index.tsx`. `plans.length === 4`.
|
||
|
||
**Required skeleton (final state) at the `isLoading` early-return (currently `index.tsx` ~503–512):**
|
||
```tsx
|
||
<div className="grid gap-4 sm:grid-cols-2 lg:grid-cols-4">
|
||
{plans.map((_, i) => (
|
||
<Skeleton key={i} className="h-48 w-full rounded-2xl" />
|
||
))}
|
||
</div>
|
||
```
|
||
|
||
**Mechanical acceptance:**
|
||
- Skeleton grid `className` **string-equals** `"grid gap-4 sm:grid-cols-2 lg:grid-cols-4"`.
|
||
- Skeleton grid renders exactly `plans.length` (i.e. 4) children, and the count is derived from `plans` — **not** a literal `[0, 1, 2, 3]`.
|
||
- Each child has `className="h-48 w-full rounded-2xl"`.
|
||
- The real grid `className` is unchanged.
|
||
|
||
**Why `plans.map` and not a literal:** future plan additions (e.g. a 5th tier) must keep skeleton/real-grid item counts in sync automatically. Hard-coded `[0,1,2,3]` would silently drift.
|
||
|
||
---
|
||
|
||
### Defect B — Dialog overflow on mobile (audit §9)
|
||
|
||
**Symptom:** On screens ≤ 375px wide, the downgrade-offer dialog and the cancel-confirm dialog (rendered from `index.tsx`) overflow horizontally and/or push their footer buttons off-screen vertically when content is tall (e.g. a long brand list inside the downgrade dialog).
|
||
|
||
**Affected elements:** every `<DialogContent>` JSX node rendered from `apps/web/src/routes/dashboard/subscription/index.tsx`. As of baseline there is at least one at `index.tsx` line ~1669; there is also a cancel-confirm dialog rendered later in the same file.
|
||
|
||
**Required final state for every `<DialogContent>` in this file:**
|
||
```tsx
|
||
<DialogContent className="max-w-[calc(100vw-2rem)] sm:max-w-md max-h-[calc(100dvh-2rem)] overflow-hidden flex flex-col">
|
||
<DialogHeader>…</DialogHeader>
|
||
<div className="overflow-y-auto -mx-6 px-6">
|
||
{/* body content that was previously a direct child of DialogContent */}
|
||
</div>
|
||
<DialogFooter>…</DialogFooter>
|
||
</DialogContent>
|
||
```
|
||
|
||
**Mechanical acceptance:**
|
||
1. Every `<DialogContent>` in `index.tsx` has `max-w-[calc(100vw-2rem)]` and `sm:max-w-md` in its `className`.
|
||
2. Every `<DialogContent>` has `max-h-[calc(100dvh-2rem)]` and `overflow-hidden flex flex-col`.
|
||
3. Between `<DialogHeader>` and `<DialogFooter>` there is exactly one wrapper `<div>` with `overflow-y-auto` containing the dialog body. (If the existing layout already has a body wrapper, add `overflow-y-auto` to it rather than nesting another `<div>`.)
|
||
4. Existing dialog body content, props, handlers, and copy are unchanged.
|
||
|
||
**Why `100dvh`:** dynamic viewport height accounts for mobile browser chrome (URL bar collapse). The arbitrary value `max-h-[calc(100dvh-2rem)]` already works in Tailwind ≥ 3.4, which this repo uses.
|
||
|
||
---
|
||
|
||
### Defect C — Sticky checkout bar safe-area padding (audit §10)
|
||
|
||
**Symptom:** The fixed-bottom sticky checkout bar overlaps the iOS home indicator on notched devices.
|
||
|
||
**Affected element:** the outer `<div>` of the sticky checkout bar (`index.tsx` ~line 1206). It currently looks like:
|
||
```tsx
|
||
<div className="fixed inset-x-0 bottom-0 z-40 animate-fade-in-up border-t border-border bg-background/95 backdrop-blur supports-[backdrop-filter]:bg-background/80 pb-[env(safe-area-inset-bottom,0px)]">
|
||
```
|
||
|
||
**Required final state:** the outermost fixed container of the sticky checkout bar contains the Tailwind arbitrary-value token `pb-[env(safe-area-inset-bottom,0px)]`. Inner spacing classes (`py-3`, `px-4`, etc.) must remain — the safe-area padding is **additive**, not a replacement.
|
||
|
||
**Mechanical acceptance:**
|
||
- `rg -n "fixed inset-x-0 bottom-0" apps/web/src/routes/dashboard/subscription/index.tsx` → the matched element's `className` contains `pb-[env(safe-area-inset-bottom,0px)]`.
|
||
- No inner content wrapper has duplicate `env(safe-area-inset-bottom)` padding.
|
||
|
||
---
|
||
|
||
## Pre-implementation step (mandatory, no-op-friendly)
|
||
|
||
The audit is dated 2026-05-13. Memory note `2026-05-13.md`:
|
||
> *"FN-319 (CLS regression) verified as no-op: Skeleton grid at line 527 already matches real grid at line 1076. The audit doc post-p0-subscription-audit.md section 6 is stale — this was already fixed in FN-199."*
|
||
|
||
Therefore, before editing **any** of A/B/C:
|
||
|
||
1. `cd` to repo root. Locate the canonical subscription page:
|
||
```
|
||
rg -l "checkout_started" apps/web/src/routes
|
||
```
|
||
If the path differs from `apps/web/src/routes/dashboard/subscription/index.tsx`, update all references in this prompt to the canonical path before continuing.
|
||
2. For each of Defect A / B / C, open the file and **verify the current source against the "Required final state" snippets above**:
|
||
- If **already correct** → do not edit the source. Skip to the Testing section and add/strengthen the regression test for that invariant.
|
||
- If **partially correct** (e.g. dialog has `max-w-[calc(100vw-2rem)]` but no `max-h` or no scroll wrapper) → implement only the missing piece.
|
||
- If **absent** → implement the full required final state.
|
||
3. Record findings in the PR description as a 3-row table: `Defect | Pre-state | Action taken`.
|
||
|
||
---
|
||
|
||
## Implementation steps
|
||
|
||
Execute in order. Each step is a single concrete change.
|
||
|
||
### Step 1 — Skeleton grid (Defect A)
|
||
- Open `apps/web/src/routes/dashboard/subscription/index.tsx`.
|
||
- Find the `if (isLoading) { return (…) }` block (≈ line 503–512).
|
||
- Replace the skeleton grid `<div>` and its children with the required final state (Defect A above), using `plans.map((_, i) => …)` for the children.
|
||
- If `plans` is not in scope at the early-return, hoist its export to top-level of the file (it already is exported per the regression test `grid-parity.test.tsx` which does `import { PlanGrid, plans } from "@/routes/dashboard/subscription/index"`). Do **not** change any other code.
|
||
|
||
### Step 2 — Dialog overflow (Defect B)
|
||
- Find every `<DialogContent>` in the file. As of baseline there are 2 (downgrade offer ≈ line 1669 and cancel confirm later in the file).
|
||
- For each, apply the required final state above:
|
||
- Edit the `className` of `<DialogContent>` to: `max-w-[calc(100vw-2rem)] sm:max-w-md max-h-[calc(100dvh-2rem)] overflow-hidden flex flex-col`.
|
||
- Wrap the JSX between `<DialogHeader>` and `<DialogFooter>` in `<div className="overflow-y-auto -mx-6 px-6">`. If only one of those siblings exists, wrap whatever sits between the header (or top of `DialogContent`) and the footer (or bottom of `DialogContent`).
|
||
- Do not change dialog body content, copy, props, handlers, or the cancel/confirm logic.
|
||
|
||
### Step 3 — Sticky bar safe-area (Defect C)
|
||
- Find the sticky checkout bar (`<div className="fixed inset-x-0 bottom-0 …`) ≈ line 1206.
|
||
- Ensure its `className` contains `pb-[env(safe-area-inset-bottom,0px)]`. If absent, append it. Keep all other classes.
|
||
- Verify no inner wrapper duplicates this padding.
|
||
|
||
### Step 4 — Typecheck and lint
|
||
```
|
||
pnpm --filter @sase/web typecheck
|
||
pnpm --filter @sase/web lint apps/web/src/routes/dashboard/subscription/
|
||
```
|
||
Both must exit 0. Resolve any errors strictly within the scope above.
|
||
|
||
---
|
||
|
||
## Tests (mandatory, extend only — do not create new test files)
|
||
|
||
All test edits land in:
|
||
- `apps/web/src/routes/dashboard/subscription/__tests__/cls-regression.test.tsx`
|
||
- `apps/web/src/routes/dashboard/subscription/__tests__/grid-parity.test.tsx`
|
||
|
||
Existing structure (from current source):
|
||
- `cls-regression.test.tsx` already has 5 tests asserting skeleton grid class string, item count, and parity with `PlanGrid`. It imports `{ PlanGrid, plans }` from the source.
|
||
- `grid-parity.test.tsx` exists alongside it (assertion shape parallel; extend as needed).
|
||
|
||
### Add the following named tests:
|
||
|
||
**T-A1 — `skeleton grid item count equals plans.length (not a hard-coded literal)`**
|
||
- Render the page in `isLoading` state (mock the subscription query to be `isLoading: true`).
|
||
- Query the skeleton grid container, assert it has exactly `plans.length` direct children, and `plans.length === 4`.
|
||
- Assertion target: `expect(grid.children).toHaveLength(plans.length)` and `expect(plans).toHaveLength(4)`.
|
||
|
||
**T-A2 — `skeleton grid className is string-equal to real grid className`**
|
||
- (Already covered by existing test 2 in `cls-regression.test.tsx` — keep green; do not duplicate.)
|
||
|
||
**T-B1 — `every DialogContent has mobile-safe width and height classes`**
|
||
- Render the page in a state that mounts a dialog (set subscription `status: "active"` and ensure a lower-tier plan exists so the downgrade dialog can open; then click "İptal Et" / similar to open the dialog; or render dialog open via prop).
|
||
- Query `screen.getByRole("dialog")` and assert its className contains all of: `max-w-[calc(100vw-2rem)]`, `sm:max-w-md`, `max-h-[calc(100dvh-2rem)]`.
|
||
- Repeat for the cancel-confirm dialog (open it via the same flow).
|
||
|
||
**T-B2 — `dialog body wrapper is scrollable`**
|
||
- Within an open dialog, assert that there exists a descendant element of the dialog (between `DialogHeader` and `DialogFooter`) whose className contains `overflow-y-auto`.
|
||
|
||
**T-C1 — `sticky checkout bar has safe-area bottom padding`**
|
||
- Render the page in a state where `stickyBarVisible && selectedPlanKey` is truthy (set initial state, or call the page in a way that toggles `stickyBarVisible` — e.g. mock IntersectionObserver to fire with `isIntersecting: false`).
|
||
- Query the sticky bar by its class signature `fixed inset-x-0 bottom-0` and assert its className contains the substring `pb-[env(safe-area-inset-bottom,0px)]`.
|
||
|
||
### Test verification protocol
|
||
|
||
For each of T-A1, T-B1, T-B2, T-C1:
|
||
1. Run `pnpm --filter @sase/web test cls-regression grid-parity` — confirm green.
|
||
2. Temporarily revert the source change for that defect (e.g. delete the `pb-[env(...)]` token).
|
||
3. Re-run; confirm the corresponding test fails.
|
||
4. Restore the source; confirm green again.
|
||
5. Record one line per test in the PR description: `T-A1: fail→pass verified`.
|
||
|
||
Test placement: **prefer extending `cls-regression.test.tsx`** for T-A1, and **prefer `grid-parity.test.tsx`** for T-B1, T-B2, T-C1 (dialog + sticky bar are not CLS). Do not create a new test file.
|
||
|
||
---
|
||
|
||
## Acceptance criteria (binary, mechanical — all must pass)
|
||
|
||
A reviewer running these commands at repo root after the change must see the indicated results.
|
||
|
||
1. `rg -n "grid gap-4 sm:grid-cols-2 lg:grid-cols-4" apps/web/src/routes/dashboard/subscription/index.tsx` → **≥ 2 matches** (skeleton + real grid).
|
||
2. `rg -n "plans\.map" apps/web/src/routes/dashboard/subscription/index.tsx` → **≥ 1 match inside the `isLoading` branch** (skeleton derives count from `plans`).
|
||
3. Number of `<DialogContent` occurrences in `index.tsx` equals the number of `max-w-\[calc\(100vw-2rem\)\]` matches in the same file.
|
||
- Run both: `rg -c "<DialogContent" apps/web/src/routes/dashboard/subscription/index.tsx` and `rg -c "max-w-\[calc\(100vw-2rem\)\]" apps/web/src/routes/dashboard/subscription/index.tsx`. Counts must be equal and both ≥ 1.
|
||
4. Same count equality for `max-h-\[calc\(100dvh-2rem\)\]` vs `<DialogContent`.
|
||
5. `rg -n "overflow-y-auto" apps/web/src/routes/dashboard/subscription/index.tsx` → at least one match inside each `<DialogContent>` (visually confirm or use `rg -A 50 "<DialogContent" | rg "overflow-y-auto"`).
|
||
6. `rg -n "pb-\[env\(safe-area-inset-bottom" apps/web/src/routes/dashboard/subscription/index.tsx` → at least one match; the match resides on a line whose `className` also contains `fixed inset-x-0 bottom-0`.
|
||
7. `pnpm --filter @sase/web typecheck` exits 0.
|
||
8. `pnpm --filter @sase/web lint apps/web/src/routes/dashboard/subscription/` exits 0.
|
||
9. `pnpm --filter @sase/web test cls-regression grid-parity` exits 0 with all pre-existing tests plus T-A1, T-B1, T-B2, T-C1 passing. (Total ≥ 5 + 4 = 9 named tests across the two files.)
|
||
10. **Manual mobile check** at iPhone-SE (375×667) and iPhone 14 Pro (393×852) viewports in Chrome DevTools device-mode or Playwright headed:
|
||
- **CLS:** with Network throttled to "Slow 3G" and CPU 4× slowdown, load `/dashboard/subscription`. The plan-cards region must not change height or column layout between skeleton and real content. Visually verify no jump; numerically the column count is 2 at 640–1023px and 4 at ≥1024px in both states.
|
||
- **Dialog:** open the downgrade dialog; confirm the dialog horizontal edges are ≥ 1rem from the viewport edges (no clipping), and that when content overflows vertically the inner body scrolls while header/footer remain pinned.
|
||
- **Sticky bar:** in iOS Safari (or iOS simulator) at the subscription page, with a plan selected and the page scrolled so the sticky bar appears, confirm the bar's CTA button is fully above the home indicator (≥ env(safe-area-inset-bottom) visible padding below the button).
|
||
- If Playwright/iOS simulator is unavailable in the execution environment (system libs missing — see project memory), document the gap explicitly in the PR with the exact reason and which manual checks were skipped. Replace skipped checks with: (i) a screenshot of Chrome DevTools device-mode at 375×667 showing each state, and (ii) a DOM-snapshot proving the `pb-[env(safe-area-inset-bottom,0px)]` class is on the rendered sticky bar.
|
||
|
||
---
|
||
|
||
## Out of scope (explicit — do not touch)
|
||
|
||
- PostHog event taxonomy, event names, properties, payloads, or call sites.
|
||
- Pricing logic, plan-array contents, brand limits, billing-period math.
|
||
- i18n keys, copy text, message bundle files (`apps/web/src/messages/*.json`).
|
||
- Any file outside `apps/web/src/routes/dashboard/subscription/`.
|
||
- The `@sase/ui/dialog` component implementation (only the consumer site).
|
||
- Adding new Tailwind plugins (e.g. safe-area plugin). Use only the arbitrary-value class `pb-[env(safe-area-inset-bottom,0px)]` already present in the codebase.
|
||
- Visual redesign, color, typography, or component restructuring beyond what the three defect fixes require.
|
||
- Card brand SVGs, trust copy aria-labels, dashboard footer copy (those are other audit items, not in this task).
|
||
|
||
---
|
||
|
||
## Risk + rollback
|
||
|
||
- All edits are CSS-class-level inside a single file plus additive tests. Revenue-critical surface, so:
|
||
- Land on the standard release branch; no feature flag is required (changes are visual-parity / additive padding only).
|
||
- Rollback = `git revert <commit>` of this PR's single commit.
|
||
- After merge, verify the FN-348 post-deploy verification harness (`qa/post-deploy/fn348-verify.mjs`) still passes; in particular the P0-8 check ("Bundle accessible — FN-345 CLS fix deployed").
|
||
|
||
---
|
||
|
||
## Deliverables
|
||
|
||
1. Updated `apps/web/src/routes/dashboard/subscription/index.tsx` — only the regions covering Defects A/B/C, and only where pre-state did not already satisfy the required final state.
|
||
2. Extended tests in `apps/web/src/routes/dashboard/subscription/__tests__/cls-regression.test.tsx` and `__tests__/grid-parity.test.tsx`.
|
||
3. PR description containing:
|
||
- The 3-row pre-implementation table (Defect | Pre-state | Action taken).
|
||
- Per-test fail→pass evidence lines (T-A1, T-B1, T-B2, T-C1).
|
||
- Screenshot or DOM-snippet evidence for the iPhone-SE manual check (or explicit documentation of any environment-skipped check).
|