docs(05-08): complete opt-out + reliability surface plan
This commit is contained in:
@@ -0,0 +1,193 @@
|
||||
---
|
||||
phase: 05-web-push-notifications
|
||||
plan: 08
|
||||
subsystem: pwa/hooks, pwa/components
|
||||
tags: [web-push, settings, permission-denied, toggle, reliability, D-09, D-10]
|
||||
dependency_graph:
|
||||
requires: [05-04]
|
||||
provides: [SettingsSheet (master toggle D-09), PermissionDeniedBanner (D-10), usePushSubscription setEnabled/isSubscribed, silent re-subscribe health-check (D-10)]
|
||||
affects:
|
||||
- apps/pwa/src/hooks/usePushSubscription.ts
|
||||
- apps/pwa/src/components/SettingsSheet.tsx
|
||||
- apps/pwa/src/components/PermissionDeniedBanner.tsx
|
||||
- apps/pwa/src/components/AppNav.tsx
|
||||
- apps/pwa/src/components/CalendarShell.tsx
|
||||
- apps/pwa/src/App.tsx
|
||||
- apps/pwa/src/styles/tokens.css
|
||||
tech_stack:
|
||||
added: []
|
||||
patterns:
|
||||
- usePushSubscription setEnabled master toggle (D-09)
|
||||
- Silent dead-subscription recovery on mount (D-10)
|
||||
- PermissionDeniedBanner role=alert, OS-revoked-only gate
|
||||
- SettingsSheet bottom sheet (role=dialog, z:301, Escape+backdrop close)
|
||||
- AppNav avatar promoted to button with onOpenSettings prop chain
|
||||
key_files:
|
||||
created:
|
||||
- apps/pwa/src/components/SettingsSheet.tsx
|
||||
- apps/pwa/src/components/PermissionDeniedBanner.tsx
|
||||
modified:
|
||||
- apps/pwa/src/hooks/usePushSubscription.ts
|
||||
- apps/pwa/src/components/AppNav.tsx
|
||||
- apps/pwa/src/components/CalendarShell.tsx
|
||||
- apps/pwa/src/App.tsx
|
||||
- apps/pwa/src/styles/tokens.css
|
||||
decisions:
|
||||
- "setEnabled(true) + permission=default: no-op; caller must tap-gated subscribe() — iOS user-gesture requirement"
|
||||
- "readNotificationsDisabled() guards health-check re-subscribe: skip if notificationsEnabled=0 (explicit user off)"
|
||||
- "persistNotificationsEnabled(false) now writes '0' instead of removing key — allows banner to detect prior-enabled state"
|
||||
- "onOpenSettings threaded through App → CalendarShell → AppNav (not hoisted to global store) — keeps settings state local to App.tsx"
|
||||
- "@keyframes spin added to tokens.css — shared by SettingsSheet Loader2 and SyncStateToast spinners"
|
||||
metrics:
|
||||
duration: 9
|
||||
completed_date: "2026-06-10"
|
||||
tasks_completed: 3
|
||||
files_changed: 7
|
||||
---
|
||||
|
||||
# Phase 05 Plan 08: Opt-Out + Reliability Surface Summary
|
||||
|
||||
Single master notifications toggle (D-09) in an avatar-opened Settings sheet, silent dead-subscription recovery on app open (D-10), and a persistent permission-denied banner for the OS-revoked case (D-10) — completing the user-facing half of the mandatory iOS health-check.
|
||||
|
||||
## Tasks Executed
|
||||
|
||||
### Task 1: usePushSubscription health-check + permission state (D-10)
|
||||
**Status:** Completed. Commit: `458d6e4`
|
||||
|
||||
Extended `apps/pwa/src/hooks/usePushSubscription.ts`:
|
||||
- Added `isSubscribed: boolean` state (true when pushManager has active subscription)
|
||||
- Added `setEnabled(on: boolean)` master toggle: off → unsubscribe + persist '0'; on + permission granted → silent subscribe; on + permission default/denied → no-op
|
||||
- Health-check now calls `readNotificationsDisabled()` — skips silent re-subscribe if user explicitly turned notifications off (notificationsEnabled=0). Prevents re-subscribing against the user's will.
|
||||
- Changed `persistNotificationsEnabled(false)` to write '0' instead of removing the key — PermissionDeniedBanner needs to detect "was previously enabled" state
|
||||
- Exported `readNotificationsEnabled` for PermissionDeniedBanner and SettingsSheet initial-state logic
|
||||
- Removed dead local usage of `readNotificationsEnabled` (was defined but not in returned interface — the lint hint from the plan)
|
||||
|
||||
### Task 2: SettingsSheet + AppNav avatar button
|
||||
**Status:** Completed. Commit: `1de4aa5`
|
||||
|
||||
Created `apps/pwa/src/components/SettingsSheet.tsx`:
|
||||
- Bottom sheet (role="dialog", aria-modal, aria-label="Settings", borderRadius 12px 12px 0 0, zIndex 301, backdrop 300 click-to-close, Escape closes)
|
||||
- Heading "Settings" + X close button (44px, aria-label="Close settings")
|
||||
- Section label "NOTIFICATIONS" (uppercase, muted, letter-spacing 0.06em)
|
||||
- Bell icon + toggle row: "FamilySync Notifications" / "Reminders, event changes, list updates"
|
||||
- Toggle switch: role="switch", aria-checked, aria-label (on/off variants), 44px touch target
|
||||
- On: track var(--color-member-0) #4A90D9, thumb white
|
||||
- Off: track var(--color-border), thumb white
|
||||
- Disabled (permission denied): opacity 0.5, no pointer events
|
||||
- Loader2 spinner replaces toggle while subscribing
|
||||
- Permission-denied hint: AlertCircle + "Notifications are blocked in your browser settings." + "How to enable" link (shown only when permission === 'denied')
|
||||
- Toggle is wired to `usePushSubscription` — `setEnabled` called on click; initial state from `isSubscribed + permission`
|
||||
|
||||
Modified `apps/pwa/src/components/AppNav.tsx`:
|
||||
- PhoneNav avatar `div` promoted to `<button>` with `onClick={onOpenSettings}` and `aria-label="${displayName} — open settings"` (44px target)
|
||||
- DesktopNav gains avatar button at bottom of sidebar with same aria-label pattern
|
||||
- `onOpenSettings` prop threaded through `AppNavProps` → both `PhoneNav` and `DesktopNav`
|
||||
|
||||
Modified `apps/pwa/src/styles/tokens.css`:
|
||||
- Added `@keyframes spin` (0deg → 360deg) — missing keyframe used by SettingsSheet Loader2 and existing SyncStateToast spinner
|
||||
|
||||
### Task 3: PermissionDeniedBanner + App mount + desktop verification
|
||||
**Status:** Completed. Commit: `010a69c`
|
||||
|
||||
Created `apps/pwa/src/components/PermissionDeniedBanner.tsx`:
|
||||
- role="alert" (assertive — permission loss is high-priority)
|
||||
- Condition: `Notification.permission === 'denied'` AND `readNotificationsEnabled() === true`
|
||||
- AlertCircle 24px (var(--color-destructive)) + "Notifications blocked" heading + "Re-enable in your browser settings." + "How to enable" inline button
|
||||
- "How to enable" opens `InstructionSheet` — WalkthroughSheet-style bottom sheet (zIndex 1000) with OS-specific 4-step instructions (iOS or Android/Chrome); platform detected via `isIOS()` UA check
|
||||
- All 4 iOS steps + all 4 Android steps verbatim from UI-SPEC Copywriting Contract
|
||||
- No dismiss button — persistent until OS permission restored
|
||||
|
||||
Modified `apps/pwa/src/App.tsx`:
|
||||
- Added `useState(false)` for `settingsOpen`
|
||||
- Mounted `<PermissionDeniedBanner />` above `<Routes>` (below AppNav, above content — per UI-SPEC)
|
||||
- Mounted `<SettingsSheet isOpen={settingsOpen} onClose={...} />` as portal-level sibling
|
||||
- CalendarShell receives `onOpenSettings={() => setSettingsOpen(true)}`
|
||||
|
||||
Modified `apps/pwa/src/components/CalendarShell.tsx`:
|
||||
- Added optional `onOpenSettings?: () => void` prop
|
||||
- Threaded to both AppNav usages (phone and desktop layout paths)
|
||||
|
||||
**playwright-cli verification results (desktop Chromium):**
|
||||
- Banner renders with exact UI-SPEC copy ("Notifications blocked", "Re-enable in your browser settings.", "How to enable") when `Notification.permission==='denied'` AND `notificationsEnabled=1`
|
||||
- Banner is absent when `Notification.permission==='granted'`
|
||||
- "How to enable" opens Android/Chrome instruction sheet with all 4 verbatim steps
|
||||
- SettingsSheet opens from avatar click (`button "Lucas — open settings"`); shows toggle + permission-denied hint when denied
|
||||
- `pnpm --filter @familysync/pwa build` green throughout
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
**1. [Rule 2 - Missing Critical Functionality] @keyframes spin missing from tokens.css**
|
||||
- **Found during:** Task 2 SettingsSheet implementation
|
||||
- **Issue:** SettingsSheet uses `animation: 'spin 1s linear infinite'` on Loader2, but `@keyframes spin` was not defined in `tokens.css`. SyncStateToast already uses the same animation name — the missing keyframe was a pre-existing gap.
|
||||
- **Fix:** Added `@keyframes spin { from { transform: rotate(0deg) } to { transform: rotate(360deg) } }` to `apps/pwa/src/styles/tokens.css`
|
||||
- **Files modified:** `apps/pwa/src/styles/tokens.css`
|
||||
- **Commit:** `1de4aa5`
|
||||
|
||||
**2. [Rule 1 - Bug] persistNotificationsEnabled(false) removed key instead of writing '0'**
|
||||
- **Found during:** Task 1 — PermissionDeniedBanner needs to detect "was previously enabled" (notificationsEnabled !== null AND !== '0')**
|
||||
- **Issue:** Original implementation called `localStorage.removeItem('notificationsEnabled')` on disable. After a user disables notifications, the key disappears. The PermissionDeniedBanner condition `readNotificationsEnabled() === true` (checks for '1') would never be true — banner would never show. More importantly, the health-check guard `readNotificationsDisabled()` (checks for '0') also wouldn't trigger — health-check would re-subscribe even after explicit user disable.
|
||||
- **Fix:** Changed `persistNotificationsEnabled(false)` to write `'0'` explicitly. Now '0' = explicitly disabled, '1' = explicitly enabled, absent = never configured.
|
||||
- **Files modified:** `apps/pwa/src/hooks/usePushSubscription.ts`
|
||||
- **Commit:** `458d6e4`
|
||||
|
||||
**3. [Rule 2 - Missing Prop Thread] CalendarShell required onOpenSettings thread**
|
||||
- **Found during:** Task 3 App.tsx mount
|
||||
- **Issue:** Plan said "App.tsx mounts SettingsSheet; AppNav receives onOpenSettings" but CalendarShell is the intermediary between App.tsx and AppNav — it didn't accept or forward `onOpenSettings`. Without threading it, the avatar click had no handler.
|
||||
- **Fix:** Added optional `onOpenSettings?: () => void` prop to CalendarShell, forwarded to both AppNav instances (phone and desktop layout).
|
||||
- **Files modified:** `apps/pwa/src/components/CalendarShell.tsx`
|
||||
- **Commit:** `010a69c`
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None. All three surfaces are fully wired end-to-end:
|
||||
- Toggle calls real `setEnabled` which calls real subscribe/unsubscribe paths
|
||||
- Banner reads real `Notification.permission` and `localStorage.notificationsEnabled`
|
||||
- Instruction sheet has verbatim copy for both iOS and Android
|
||||
|
||||
## Deferred (iOS Device-Only Checks)
|
||||
|
||||
The following items cannot be driven by playwright-cli and remain on the Phase 5 human gate (device-only, Gate 2):
|
||||
|
||||
1. **iOS Safari standalone — toggle subscribe**: `setEnabled(true)` on iOS requires the user gesture to be the original tap; this works in desktop Chromium but needs iOS device validation
|
||||
2. **iOS standalone — banner instruction sheet**: iOS steps (Settings → Safari → Notifications) need on-device validation; only Android steps shown on desktop
|
||||
3. **iOS push delivery after toggle on/off**: VAPID subscription round-trip on APNs requires real iOS device
|
||||
4. **Permission polling**: On iOS, `Notification.permission` can change externally (user changes Settings app); banner disappears on next check — needs device validation
|
||||
|
||||
## Threat Flags
|
||||
|
||||
No new threat surface beyond the plan's threat model. All three threats mitigated:
|
||||
|
||||
| Threat | Status |
|
||||
|--------|--------|
|
||||
| T-05-23: Silent re-subscribe without permission | Mitigated — health-check only runs when `Notification.permission==='granted'` |
|
||||
| T-05-24: XSS via copy | Mitigated — all copy is plain-text JSX children (no dangerouslySetInnerHTML) |
|
||||
| T-05-25: Toggle off leaves stale server subscription | Mitigated — `setEnabled(false)` calls `unsubscribe()` which calls `DELETE /api/push/subscription` |
|
||||
|
||||
## Self-Check
|
||||
|
||||
**Files created/verified:**
|
||||
- [x] apps/pwa/src/components/SettingsSheet.tsx — exists
|
||||
- [x] apps/pwa/src/components/PermissionDeniedBanner.tsx — exists
|
||||
- [x] apps/pwa/src/hooks/usePushSubscription.ts — modified, exists
|
||||
- [x] apps/pwa/src/components/AppNav.tsx — modified, exists
|
||||
- [x] apps/pwa/src/components/CalendarShell.tsx — modified, exists
|
||||
- [x] apps/pwa/src/App.tsx — modified, exists
|
||||
- [x] apps/pwa/src/styles/tokens.css — modified, exists
|
||||
|
||||
**Commits verified:**
|
||||
- 458d6e4: feat(05-08): extend usePushSubscription with isSubscribed, setEnabled, permission state (D-10)
|
||||
- 1de4aa5: feat(05-08): SettingsSheet (master toggle D-09) + AppNav avatar promoted to button
|
||||
- 010a69c: feat(05-08): PermissionDeniedBanner + App mount + CalendarShell onOpenSettings wiring
|
||||
|
||||
**Build:** `pnpm --filter @familysync/pwa build` green (precache 7 entries, dist/sw.js produced)
|
||||
|
||||
**playwright-cli evidence:**
|
||||
- Banner renders correctly at http://localhost:4175/ with mocked API + denied permission + notificationsEnabled=1
|
||||
- Banner absent when permission=granted
|
||||
- "How to enable" opens Android instruction sheet (correct for Chromium)
|
||||
- SettingsSheet opens from avatar click, shows "Settings" / "NOTIFICATIONS" / toggle / permission-denied hint
|
||||
- Screenshot captured: `/tmp/settings-denied.png` (banner + sheet both visible simultaneously)
|
||||
|
||||
## Self-Check: PASSED
|
||||
Reference in New Issue
Block a user