From 3f4b7eac738ea6ad5e06bc28d53a0a163bc79441 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 18 Jun 2026 13:46:27 -0400 Subject: [PATCH] fix(17): IN-02 useCallback handleClose, drop exhaustive-deps disables --- apps/pwa/src/components/CredentialSheet.tsx | 29 ++++++++++++--------- apps/pwa/src/components/SettingsSheet.tsx | 21 ++++++++------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/apps/pwa/src/components/CredentialSheet.tsx b/apps/pwa/src/components/CredentialSheet.tsx index d640c8e..52220b1 100644 --- a/apps/pwa/src/components/CredentialSheet.tsx +++ b/apps/pwa/src/components/CredentialSheet.tsx @@ -22,7 +22,7 @@ * T-10-16: autoComplete="new-password" prevents autofill of stored credential */ -import { useState, useEffect, useRef } from 'react'; +import { useState, useEffect, useRef, useCallback } from 'react'; import { useMutation, useQueryClient } from '@tanstack/react-query'; import { Loader2 } from 'lucide-react'; import { @@ -88,6 +88,20 @@ export function CredentialSheet({ const dialogRef = useRef(null); const handleDialogKeyDown = useFocusTrap(dialogRef); + // IN-02: useCallback so handleClose can be a real dependency of the Escape + // effect (no blanket exhaustive-deps disable), matching the LinkOidc/Reset + // sheets' pattern. + const handleClose = useCallback(() => { + setPassword(''); + setEmail(''); + setValidationError(null); + onClose(); + // Return focus to trigger element (a11y) + if (triggerRef?.current) { + triggerRef.current.focus(); + } + }, [onClose, triggerRef]); + // Escape key closes the sheet (SettingsSheet pattern) useEffect(() => { if (!isOpen) return; @@ -98,7 +112,7 @@ export function CredentialSheet({ }; document.addEventListener('keydown', onKeyDown); return () => document.removeEventListener('keydown', onKeyDown); - }, [isOpen]); // eslint-disable-line react-hooks/exhaustive-deps + }, [isOpen, handleClose]); // Focus heading on open (a11y) useEffect(() => { @@ -107,17 +121,6 @@ export function CredentialSheet({ } }, [isOpen]); - function handleClose() { - setPassword(''); - setEmail(''); - setValidationError(null); - onClose(); - // Return focus to trigger element (a11y) - if (triggerRef?.current) { - triggerRef.current.focus(); - } - } - const credentialMutation = useMutation({ mutationFn: async () => { if (mode === 'self-service') { diff --git a/apps/pwa/src/components/SettingsSheet.tsx b/apps/pwa/src/components/SettingsSheet.tsx index f5dfc52..5aa63c7 100644 --- a/apps/pwa/src/components/SettingsSheet.tsx +++ b/apps/pwa/src/components/SettingsSheet.tsx @@ -21,7 +21,7 @@ * Security: T-05-24 — all copy is plain-text JSX children, no dangerouslySetInnerHTML. */ -import { useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { X, Bell, AlertCircle, Loader2, LogOut } from 'lucide-react'; import { useQuery, useMutation } from '@tanstack/react-query'; import { useNavigate } from 'react-router'; @@ -606,6 +606,15 @@ function ChangePasswordSheet({ isOpen, onClose }: ChangePasswordSheetProps) { const dialogRef = useRef(null); const handleDialogKeyDown = useFocusTrap(dialogRef); + // IN-02: useCallback so handleClose is a real dependency of the Escape effect. + const handleClose = useCallback(() => { + setCurrentPassword(''); + setNewPassword(''); + setConfirmPassword(''); + setError(null); + onClose(); + }, [onClose]); + useEffect(() => { if (!isOpen) return; const onKeyDown = (e: KeyboardEvent) => { @@ -613,7 +622,7 @@ function ChangePasswordSheet({ isOpen, onClose }: ChangePasswordSheetProps) { }; document.addEventListener('keydown', onKeyDown); return () => document.removeEventListener('keydown', onKeyDown); - }, [isOpen]); // eslint-disable-line react-hooks/exhaustive-deps + }, [isOpen, handleClose]); useEffect(() => { if (isOpen && headingRef.current) { @@ -621,14 +630,6 @@ function ChangePasswordSheet({ isOpen, onClose }: ChangePasswordSheetProps) { } }, [isOpen]); - function handleClose() { - setCurrentPassword(''); - setNewPassword(''); - setConfirmPassword(''); - setError(null); - onClose(); - } - const changeMutation = useMutation({ mutationFn: async () => { if (newPassword !== confirmPassword) throw new Error('mismatch');