diff --git a/.github/skills/medassist-ui-consistency/SKILL.md b/.github/skills/medassist-ui-consistency/SKILL.md index 647ddd1..185edf2 100644 --- a/.github/skills/medassist-ui-consistency/SKILL.md +++ b/.github/skills/medassist-ui-consistency/SKILL.md @@ -26,6 +26,16 @@ Use `medassist-frontend-polish` only after these guardrails are satisfied. - Avoid custom inline modal/button patterns that diverge from project design. - Prefer extending existing CSS classes/styles instead of introducing parallel styling systems. +### Modal requirements (non-negotiable) + +Every modal/overlay **must** follow these rules: + +1. **Escape key**: Call `useEscapeKey(active, onClose)` from `hooks/useEscapeKey`. This registers a document-level `keydown` listener that works regardless of focus. **Never** rely on `onKeyDown` on an overlay div — it only fires when the overlay has focus, which almost never happens. +2. **Scroll lock**: Call `useScrollLock(active)` from `hooks/useScrollLock` if the modal is **not** already covered by App.tsx's centralized `useScrollLock` call. Page-local modals (e.g. `ReportModal`, `ExportModal`) must call it themselves. +3. **Click-outside close**: The overlay div gets `onClick={onClose}`, and `.modal-content` gets `onClick={(e) => e.stopPropagation()}`. +4. **Key event containment**: `.modal-content` gets `onKeyDown={(e) => { if (e.key !== "Escape") e.stopPropagation(); }}` — this prevents non-Escape keys from leaking out while still allowing Escape to propagate to the document-level handler. +5. **Nested sub-modals** (e.g. edit-stock inside MedDetailModal): Use `useEscapeKey` with `{ capture: true }` so the innermost modal intercepts Escape before the parent's handler fires. + ## Decision Heuristics 1. If an equivalent component exists, reuse it. diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 2acfd67..acd4ac4 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,5 +1,5 @@ -import { useCallback, useEffect, useState } from "react"; -import { Navigate, Route, Routes, useNavigate } from "react-router-dom"; +import { useCallback, useEffect, useRef, useState } from "react"; +import { Navigate, Route, Routes, useLocation, useNavigate } from "react-router-dom"; import { AboutModal, Lightbox, @@ -114,6 +114,7 @@ function AppRouter() { function AppContent() { const navigate = useNavigate(); + const location = useLocation(); // Get shared state from AppContext const ctx = useAppContext(); const { @@ -189,6 +190,9 @@ function AppContent() { // Local-only state (not shared across components) const [showProfile, setShowProfile] = useState(false); const [showAbout, setShowAbout] = useState(false); + const [routeTransitionMaskActive, setRouteTransitionMaskActive] = useState(false); + const routeTransitionMinEndRef = useRef(0); + const routeTransitionFallbackTimerRef = useRef(null); const closeProfile = useCallback(() => { if (showProfile) { window.history.back(); @@ -204,55 +208,6 @@ function AppContent() { // Get centralized stockThresholds from context const { stockThresholds } = ctx; - // Close modal on Escape key - useEffect(() => { - const handleEscape = (e: KeyboardEvent) => { - if (e.key === "Escape") { - // Close modals in order of priority (topmost first) - if (scheduleLightboxImage) { - closeScheduleLightbox(); - } else if (showImageLightbox) { - closeImageLightbox(); - } else if (showEditStockModal) { - closeEditStockModal(); - } else if (showRefillModal) { - closeRefillModal(); - } else if (showShareDialog) { - closeShareDialog(); - } else if (showAbout) { - closeAbout(); - } else if (showProfile) { - closeProfile(); - } else if (selectedUser) { - closeUserFilter(); - } else if (selectedMed) { - closeMedDetail(); - } - } - }; - document.addEventListener("keydown", handleEscape); - return () => document.removeEventListener("keydown", handleEscape); - }, [ - selectedMed, - showImageLightbox, - scheduleLightboxImage, - selectedUser, - showProfile, - showAbout, - showShareDialog, - showRefillModal, - showEditStockModal, - closeAbout, - closeEditStockModal, - closeImageLightbox, - closeMedDetail, - closeProfile, - closeRefillModal, - closeScheduleLightbox, - closeShareDialog, - closeUserFilter, - ]); - // Handle browser back button to close modals (in priority order) useEffect(() => { const handlePopState = () => { @@ -341,6 +296,72 @@ function AppContent() { }; }, []); + // Global Escape handling in priority order. + // This keeps behavior consistent even when child modals are mocked in tests. + useEffect(() => { + const handleEscape = (e: KeyboardEvent) => { + if (e.key !== "Escape") return; + + if (scheduleLightboxImage) { + closeScheduleLightbox(); + return; + } + if (showImageLightbox) { + closeImageLightbox(); + return; + } + if (showEditStockModal) { + closeEditStockModal(); + return; + } + if (showRefillModal) { + closeRefillModal(); + return; + } + if (showShareDialog) { + closeShareDialog(); + return; + } + if (showAbout) { + closeAbout(); + return; + } + if (showProfile) { + closeProfile(); + return; + } + if (selectedUser) { + closeUserFilter(); + return; + } + if (selectedMed) { + closeMedDetail(); + } + }; + + document.addEventListener("keydown", handleEscape); + return () => document.removeEventListener("keydown", handleEscape); + }, [ + showImageLightbox, + scheduleLightboxImage, + showEditStockModal, + showRefillModal, + showShareDialog, + showAbout, + showProfile, + selectedUser, + selectedMed, + closeImageLightbox, + closeScheduleLightbox, + closeEditStockModal, + closeRefillModal, + closeShareDialog, + closeAbout, + closeProfile, + closeUserFilter, + closeMedDetail, + ]); + // Prevent background scroll when any modal is open useScrollLock( !!( @@ -383,9 +404,57 @@ function AppContent() { await ctx.submitRefill(medId, null, () => {}, loadMeds, usePrescription); }; + useEffect(() => { + if (!routeTransitionMaskActive) return; + if (location.pathname !== "/medications") return; + + const hasEditMedIdParam = new URLSearchParams(location.search).has("editMedId"); + if (hasEditMedIdParam) return; + + const remaining = Math.max(0, routeTransitionMinEndRef.current - performance.now()); + const timer = window.setTimeout(() => setRouteTransitionMaskActive(false), remaining); + return () => window.clearTimeout(timer); + }, [location.pathname, location.search, routeTransitionMaskActive]); + + useEffect(() => { + const handleEditTransitionReady = () => { + if (!routeTransitionMaskActive) return; + const remaining = Math.max(0, routeTransitionMinEndRef.current - performance.now()); + window.setTimeout(() => { + setRouteTransitionMaskActive(false); + if (routeTransitionFallbackTimerRef.current !== null) { + window.clearTimeout(routeTransitionFallbackTimerRef.current); + routeTransitionFallbackTimerRef.current = null; + } + }, remaining); + }; + + window.addEventListener("medassist:edit-transition-ready", handleEditTransitionReady); + return () => { + window.removeEventListener("medassist:edit-transition-ready", handleEditTransitionReady); + }; + }, [routeTransitionMaskActive]); + + useEffect(() => { + return () => { + if (routeTransitionFallbackTimerRef.current !== null) { + window.clearTimeout(routeTransitionFallbackTimerRef.current); + } + }; + }, []); + const handleOpenMedicationEdit = () => { if (!selectedMed) return; const medId = selectedMed.id; + routeTransitionMinEndRef.current = performance.now() + 80; + setRouteTransitionMaskActive(true); + if (routeTransitionFallbackTimerRef.current !== null) { + window.clearTimeout(routeTransitionFallbackTimerRef.current); + } + routeTransitionFallbackTimerRef.current = window.setTimeout(() => { + setRouteTransitionMaskActive(false); + routeTransitionFallbackTimerRef.current = null; + }, 700); setShowImageLightbox(false); setShowRefillModal(false); setShowEditStockModal(false); @@ -508,6 +577,8 @@ function AppContent() { {scheduleLightboxImage && ( )} + +