fix(05-review): CR-04 pre-fetch VAPID key into state; no await before pushManager.subscribe
This commit is contained in:
@@ -25,6 +25,24 @@ import { useEffect, useRef, useState } from 'react'
|
||||
import { X, Bell, AlertCircle, Loader2 } from 'lucide-react'
|
||||
import { usePushSubscription } from '../hooks/usePushSubscription.js'
|
||||
|
||||
// CR-04: fetch VAPID key (from sessionStorage cache if available) for the
|
||||
// tap-gated subscribe() path. Same logic as PushPermissionPrompt.
|
||||
async function fetchVapidKeyForSettings(): Promise<string | null> {
|
||||
try {
|
||||
const cached = sessionStorage.getItem('vapidPublicKey')
|
||||
if (cached) return cached
|
||||
const res = await fetch('/api/push/vapid-public-key', { credentials: 'include' })
|
||||
if (!res.ok) return null
|
||||
const data = (await res.json()) as { publicKey: string }
|
||||
if (data.publicKey) {
|
||||
sessionStorage.setItem('vapidPublicKey', data.publicKey)
|
||||
}
|
||||
return data.publicKey ?? null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
interface SettingsSheetProps {
|
||||
isOpen: boolean
|
||||
onClose: () => void
|
||||
@@ -33,6 +51,9 @@ interface SettingsSheetProps {
|
||||
export function SettingsSheet({ isOpen, onClose }: SettingsSheetProps) {
|
||||
const { subscribe, permission, isSubscribed, setEnabled } = usePushSubscription()
|
||||
const [isTogglingOn, setIsTogglingOn] = useState(false)
|
||||
// CR-04: pre-fetch the VAPID key into state so the toggle tap handler can call
|
||||
// subscribe(registration, vapidKey) without any network await before pushManager.subscribe().
|
||||
const [vapidKey, setVapidKey] = useState<string | null>(null)
|
||||
const closeButtonRef = useRef<HTMLButtonElement>(null)
|
||||
|
||||
// Compute initial toggle on/off state per UI-SPEC toggle initial state rule:
|
||||
@@ -56,6 +77,16 @@ export function SettingsSheet({ isOpen, onClose }: SettingsSheetProps) {
|
||||
}
|
||||
}, [isOpen])
|
||||
|
||||
// CR-04: Pre-fetch the VAPID key while the sheet is open so the OS-dialog path
|
||||
// (permission === 'default') has the key ready before the user taps.
|
||||
useEffect(() => {
|
||||
if (!isOpen) return
|
||||
if (permission === 'denied') return
|
||||
void fetchVapidKeyForSettings().then((key) => {
|
||||
if (key) setVapidKey(key)
|
||||
})
|
||||
}, [isOpen, permission])
|
||||
|
||||
if (!isOpen) return null
|
||||
|
||||
const handleToggle = async () => {
|
||||
@@ -75,11 +106,15 @@ export function SettingsSheet({ isOpen, onClose }: SettingsSheetProps) {
|
||||
} else {
|
||||
// off → on, permission 'default': needs tap-gated subscribe with OS dialog
|
||||
// The toggle click IS the tap gesture — call subscribe() directly here.
|
||||
// CR-04: vapidKey was pre-fetched into state (useEffect above); pass it directly
|
||||
// so there is no await-fetch between the tap and pushManager.subscribe().
|
||||
if (!vapidKey) return // key not ready — this should be rare; button should show spinner
|
||||
const resolvedVapidKey = vapidKey
|
||||
setIsTogglingOn(true)
|
||||
try {
|
||||
const registration = await navigator.serviceWorker?.ready
|
||||
if (registration) {
|
||||
await subscribe(registration)
|
||||
await subscribe(registration, resolvedVapidKey)
|
||||
}
|
||||
} catch {
|
||||
// Permission denied by OS or error — permission state will update reactively
|
||||
|
||||
Reference in New Issue
Block a user