fix(ChapterMap): enforce locked map state with explicit unlock

kris70lesgo · kris70lesgo · commit 8ad904c8b602 · 2026-01-01T20:20:17.000+05:30
diff --git a/frontend/__tests__/unit/components/ChapterMap.test.tsx b/frontend/__tests__/unit/components/ChapterMap.test.tsx
@@ -11,10 +11,30 @@ const mockMap = {
   getCenter: jest.fn(() => ({ lat: 20, lng: 0 })),
   getZoom: jest.fn(() => 2),
   getContainer: jest.fn(() => document.getElementById('chapter-map')),
+  dragging: {
+    enable: jest.fn(),
+    disable: jest.fn(),
+  },
+  touchZoom: {
+    enable: jest.fn(),
+    disable: jest.fn(),
+  },
+  doubleClickZoom: {
+    enable: jest.fn(),
+    disable: jest.fn(),
+  },
   scrollWheelZoom: {
     enable: jest.fn(),
     disable: jest.fn(),
   },
+  boxZoom: {
+    enable: jest.fn(),
+    disable: jest.fn(),
+  },
+  keyboard: {
+    enable: jest.fn(),
+    disable: jest.fn(),
+  },
 }
 
 const mockMarker = {
@@ -160,16 +180,20 @@ describe('ChapterMap', () => {
       expect(mockTileLayer.addTo).toHaveBeenCalledWith(mockMap)
     })
 
-    it('creates and adds marker cluster group', () => {
+    it('disables all interaction handlers on initialization', () => {
       render(<ChapterMap {...defaultProps} />)
-      expect(L.markerClusterGroup).toHaveBeenCalled()
-      expect(mockMap.addLayer).toHaveBeenCalledWith(mockMarkerClusterGroup)
+      expect(mockMap.dragging.disable).toHaveBeenCalled()
+      expect(mockMap.touchZoom.disable).toHaveBeenCalled()
+      expect(mockMap.doubleClickZoom.disable).toHaveBeenCalled()
+      expect(mockMap.scrollWheelZoom.disable).toHaveBeenCalled()
+      expect(mockMap.boxZoom.disable).toHaveBeenCalled()
+      expect(mockMap.keyboard.disable).toHaveBeenCalled()
     })
 
-    it('sets up event listeners for map interaction', () => {
+    it('creates and adds marker cluster group', () => {
       render(<ChapterMap {...defaultProps} />)
-      expect(mockMap.on).toHaveBeenCalledWith('click', expect.any(Function))
-      expect(mockMap.on).toHaveBeenCalledWith('mouseout', expect.any(Function))
+      expect(L.markerClusterGroup).toHaveBeenCalled()
+      expect(mockMap.addLayer).toHaveBeenCalledWith(mockMarkerClusterGroup)
     })
   })
 
@@ -283,48 +307,35 @@ describe('ChapterMap', () => {
       expect(getByText('Unlock map')).toBeInTheDocument()
     })
 
-    it('removes overlay when clicked', () => {
+    it('removes overlay when unlock button is clicked', () => {
       const { getByText, queryByText } = render(<ChapterMap {...defaultProps} />)
 
-      const overlay = getByText('Unlock map').closest('button')
-      fireEvent.click(overlay!)
+      const button = getByText('Unlock map').closest('button')
+      fireEvent.click(button!)
 
       expect(queryByText('Unlock map')).not.toBeInTheDocument()
     })
 
-    it('enables scroll wheel zoom when overlay is clicked', () => {
-      const { getByText } = render(<ChapterMap {...defaultProps} />)
-
-      const overlay = getByText('Unlock map').closest('button')
-      fireEvent.click(overlay!)
-
-      expect(mockMap.scrollWheelZoom.enable).toHaveBeenCalled()
-    })
-
-    it('handles keyboard interaction with Enter key', () => {
+    it('enables all interaction handlers when unlock button is clicked', () => {
       const { getByText } = render(<ChapterMap {...defaultProps} />)
 
-      const overlay = getByText('Unlock map').closest('button')
-      fireEvent.keyDown(overlay!, { key: 'Enter' })
-
-      expect(mockMap.scrollWheelZoom.enable).toHaveBeenCalled()
-    })
-
-    it('handles keyboard interaction with Space key', () => {
-      const { getByText } = render(<ChapterMap {...defaultProps} />)
-
-      const overlay = getByText('Unlock map').closest('button')
-      fireEvent.keyDown(overlay!, { key: ' ' })
+      const button = getByText('Unlock map').closest('button')
+      fireEvent.click(button!)
 
+      expect(mockMap.dragging.enable).toHaveBeenCalled()
+      expect(mockMap.touchZoom.enable).toHaveBeenCalled()
+      expect(mockMap.doubleClickZoom.enable).toHaveBeenCalled()
       expect(mockMap.scrollWheelZoom.enable).toHaveBeenCalled()
+      expect(mockMap.boxZoom.enable).toHaveBeenCalled()
+      expect(mockMap.keyboard.enable).toHaveBeenCalled()
     })
 
     it('has proper accessibility attributes', () => {
       const { getByText } = render(<ChapterMap {...defaultProps} />)
 
-      const overlay = getByText('Unlock map').closest('button')
-      expect(overlay).toHaveAttribute('tabIndex', '0')
-      expect(overlay).toHaveAttribute('aria-label', 'Unlock map')
+      const button = getByText('Unlock map').closest('button')
+      expect(button).toHaveAttribute('type', 'button')
+      expect(button).toHaveAttribute('aria-label', 'Unlock map')
     })
   })
 
@@ -427,8 +438,8 @@ describe('ChapterMap', () => {
     it('shows zoom control when unlock button is clicked', () => {
       const { getByText } = render(<ChapterMap {...defaultProps} />)
 
-      const overlay = getByText('Unlock map').closest('button')
-      fireEvent.click(overlay)
+      const button = getByText('Unlock map').closest('button')
+      fireEvent.click(button!)
 
       expect(L.control.zoom).toHaveBeenCalledWith({ position: 'topleft' })
       expect(mockZoomControl.addTo).toHaveBeenCalledWith(mockMap)
@@ -468,4 +479,75 @@ describe('ChapterMap', () => {
       expect(queryByLabelText(/share location/i)).not.toBeInTheDocument()
     })
   })
+
+  describe('Escape Key Re-lock', () => {
+    it('re-locks the map when Escape key is pressed', () => {
+      const { getByText, queryByText } = render(<ChapterMap {...defaultProps} />)
+
+      // First unlock the map
+      const unlockButton = getByText('Unlock map')
+      fireEvent.click(unlockButton)
+
+      // Verify map is unlocked
+      expect(queryByText('Unlock map')).not.toBeInTheDocument()
+      expect(mockMap.dragging.enable).toHaveBeenCalled()
+
+      // Press Escape to re-lock
+      fireEvent.keyDown(window, { key: 'Escape' })
+
+      // Verify map is locked again
+      expect(getByText('Unlock map')).toBeInTheDocument()
+      expect(mockMap.dragging.disable).toHaveBeenCalled()
+      expect(mockMap.scrollWheelZoom.disable).toHaveBeenCalled()
+    })
+
+    it('does nothing when Escape is pressed and map is already locked', () => {
+      const { getByText } = render(<ChapterMap {...defaultProps} />)
+
+      const disableCallsBefore = mockMap.dragging.disable.mock.calls.length
+
+      // Press Escape while map is locked
+      fireEvent.keyDown(window, { key: 'Escape' })
+
+      // Should still show unlock button
+      expect(getByText('Unlock map')).toBeInTheDocument()
+
+      // Disable should not be called again
+      expect(mockMap.dragging.disable.mock.calls.length).toBe(disableCallsBefore)
+    })
+
+    it('removes zoom control when Escape re-locks the map', () => {
+      const { getByText } = render(<ChapterMap {...defaultProps} />)
+
+      // Unlock the map
+      const unlockButton = getByText('Unlock map')
+      fireEvent.click(unlockButton)
+
+      // Zoom control should be added
+      expect(mockZoomControl.addTo).toHaveBeenCalled()
+
+      // Press Escape to re-lock
+      fireEvent.keyDown(window, { key: 'Escape' })
+
+      // Zoom control should be removed
+      expect(mockZoomControl.remove).toHaveBeenCalled()
+    })
+  })
+
+  describe('Pointer Events Structure', () => {
+    it('overlay wrapper has pointer-events-none class', () => {
+      const { container } = render(<ChapterMap {...defaultProps} />)
+
+      const overlay = container.querySelector('.pointer-events-none')
+      expect(overlay).toBeInTheDocument()
+      expect(overlay).toHaveClass('absolute', 'inset-0', 'z-[500]')
+    })
+
+    it('unlock button has pointer-events-auto class', () => {
+      const { getByText } = render(<ChapterMap {...defaultProps} />)
+
+      const button = getByText('Unlock map')
+      expect(button).toHaveClass('pointer-events-auto')
+    })
+  })
 })
diff --git a/frontend/src/components/ChapterMap.tsx b/frontend/src/components/ChapterMap.tsx
@@ -33,6 +33,22 @@ const ChapterMap = ({
   const initialViewRef = useRef<{ center: L.LatLngExpression; zoom: number } | null>(null)
   const [isMapActive, setIsMapActive] = useState(false)
 
+  const setMapInteractivity = (map: L.Map, enabled: boolean) => {
+    const action = enabled ? 'enable' : 'disable'
+    map.dragging[action]()
+    map.touchZoom[action]()
+    map.doubleClickZoom[action]()
+    map.scrollWheelZoom[action]()
+    map.boxZoom[action]()
+    map.keyboard[action]()
+  }
+
+  const unlockMap = () => {
+    if (!mapRef.current) return
+    setMapInteractivity(mapRef.current, true)
+    setIsMapActive(true)
+  }
+
   useEffect(() => {
     if (!mapRef.current) {
       mapRef.current = L.map('chapter-map', {
@@ -56,25 +72,7 @@ const ChapterMap = ({
         className: 'map-tiles',
       }).addTo(mapRef.current)
 
-      mapRef.current.on('click', () => {
-        mapRef.current?.scrollWheelZoom.enable()
-        setIsMapActive(true)
-      })
-
-      mapRef.current.on('mouseout', (e: L.LeafletMouseEvent) => {
-        const originalEvent = e.originalEvent as MouseEvent
-        const relatedTarget = originalEvent.relatedTarget as Node | null
-        const container = mapRef.current?.getContainer()
-        const mapParent = container?.parentElement
-        if (
-          relatedTarget &&
-          (container?.contains(relatedTarget) || mapParent?.contains(relatedTarget))
-        )
-          return
-
-        mapRef.current?.scrollWheelZoom.disable()
-        setIsMapActive(false)
-      })
+      setMapInteractivity(mapRef.current, false)
     }
 
     const map = mapRef.current
@@ -194,6 +192,17 @@ const ChapterMap = ({
     }
   }, [geoLocData, showLocal, userLocation])
 
+  useEffect(() => {
+    const handleEscape = (e: KeyboardEvent) => {
+      if (e.key === 'Escape' && isMapActive && mapRef.current) {
+        setMapInteractivity(mapRef.current, false)
+        setIsMapActive(false)
+      }
+    }
+    window.addEventListener('keydown', handleEscape)
+    return () => window.removeEventListener('keydown', handleEscape)
+  }, [isMapActive])
+
   useEffect(() => {
     const map = mapRef.current
     if (!map) return
@@ -220,28 +229,17 @@ const ChapterMap = ({
     <div className="relative" style={style}>
       <div id="chapter-map" className="h-full w-full" />
       {!isMapActive && (
-        <button
-          type="button"
-          tabIndex={0}
-          className="pointer-events-none absolute inset-0 z-[500] flex cursor-pointer items-center justify-center rounded-[inherit] bg-black/10"
-          onClick={() => {
-            mapRef.current?.scrollWheelZoom.enable()
-            setIsMapActive(true)
-          }}
-          onKeyDown={(e) => {
-            if (e.key === 'Enter' || e.key === ' ') {
-              e.preventDefault()
-              mapRef.current?.scrollWheelZoom.enable()
-              setIsMapActive(true)
-            }
-          }}
-          aria-label="Unlock map"
-        >
-          <p className="pointer-events-auto flex items-center gap-2 rounded-md bg-white/90 px-5 py-3 text-sm font-medium text-gray-700 shadow-lg transition-colors hover:bg-gray-200 hover:text-gray-900 dark:bg-gray-700 dark:text-white dark:hover:bg-gray-600 dark:hover:text-white">
+        <div className="pointer-events-none absolute inset-0 z-[500] flex items-center justify-center bg-black/10">
+          <button
+            type="button"
+            className="pointer-events-auto flex items-center gap-2 rounded-md bg-white/90 px-5 py-3 text-sm font-medium text-gray-700 shadow-lg transition-colors hover:bg-gray-200 hover:text-gray-900 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 dark:bg-gray-700 dark:text-white dark:hover:bg-gray-600 dark:hover:text-white"
+            onClick={unlockMap}
+            aria-label="Unlock map"
+          >
             <FaUnlock aria-hidden="true" />
             Unlock map
-          </p>
-        </button>
+          </button>
+        </div>
       )}
       {isMapActive && (
         <div className="absolute top-20 left-3 z-[999] w-fit">
