diff --git a/src/LocationPicker/MenuPopover/index.js b/src/LocationPicker/MenuPopover/index.js index 7fc84030a..b1f728ddf 100644 --- a/src/LocationPicker/MenuPopover/index.js +++ b/src/LocationPicker/MenuPopover/index.js @@ -84,26 +84,31 @@ const MenuPopover = ({ useEffect(() => { // Select the GPS input on mount so user can type away or navigate. gpsInputRef.current.select(); + }, []); - // Create a focus trap while the component is mounted so only internal elements are focused when pressing tab. - const onKeyDown = (event) => { - if (event.key === 'Tab') { - if (event.shiftKey && document.activeElement === gpsFormatToggleRef.current) { - event.preventDefault(); + useEffect(() => { + // Create a focus trap while the component is mounted so only internal elements are focused when pressing tab only + // if the user is not picking a location from the map. + if (!isPickingLocation) { + const onKeyDown = (event) => { + if (event.key === 'Tab') { + if (event.shiftKey && document.activeElement === gpsFormatToggleRef.current) { + event.preventDefault(); - lastFocusableElementRef.current.focus(); - } else if (!event.shiftKey && document.activeElement === lastFocusableElementRef.current) { - event.preventDefault(); + lastFocusableElementRef.current.focus(); + } else if (!event.shiftKey && document.activeElement === lastFocusableElementRef.current) { + event.preventDefault(); - gpsFormatToggleRef.current.focus(); + gpsFormatToggleRef.current.focus(); + } } - } - }; + }; - document.addEventListener('keydown', onKeyDown); + document.addEventListener('keydown', onKeyDown); - return () => document.removeEventListener('keydown', onKeyDown); - }, []); + return () => document.removeEventListener('keydown', onKeyDown); + } + }, [isPickingLocation]); useEffect(() => { // Add a mouse down event to close the menu if the user clicks outside only if the user is not picking a location @@ -147,12 +152,12 @@ const MenuPopover = ({ style={{ ...style, minWidth: popoverWidth, width: popoverWidth }} {...otherProps} > -
+
diff --git a/src/LocationPicker/MenuPopover/index.test.js b/src/LocationPicker/MenuPopover/index.test.js index 68f571e7d..b10d213f7 100644 --- a/src/LocationPicker/MenuPopover/index.test.js +++ b/src/LocationPicker/MenuPopover/index.test.js @@ -112,6 +112,17 @@ describe('LocationPicker - MenuPopover', () => { expect(setLocationButtonRefFocus).toHaveBeenCalledTimes(1); }); + test('does neither close the menu nor focuse the set location button if the user presses enter while picking a location', () => { + store.view.mapLocationSelection.isPickingLocation = true; + renderMenuPopover(); + + userEvent.type(screen.getByLabelText('GPS location'), '10,10'); + userEvent.keyboard('{Enter}'); + + expect(onClose).not.toHaveBeenCalled(); + expect(setLocationButtonRefFocus).not.toHaveBeenCalled(); + }); + test('closes the menu and focuses the set location button if the user presses escape', () => { renderMenuPopover(); @@ -125,6 +136,17 @@ describe('LocationPicker - MenuPopover', () => { expect(setLocationButtonRefFocus).toHaveBeenCalledTimes(1); }); + test('does neither close the menu nor focuse the set location button if the user presses escape while picking a location', () => { + store.view.mapLocationSelection.isPickingLocation = true; + renderMenuPopover(); + + userEvent.type(screen.getByLabelText('GPS location'), '10,10'); + userEvent.keyboard('{Escape}'); + + expect(onClose).not.toHaveBeenCalled(); + expect(setLocationButtonRefFocus).not.toHaveBeenCalled(); + }); + test('changes the location when the user picks a location in the map', () => { renderMenuPopover(); diff --git a/src/SideBar/index.js b/src/SideBar/index.js index 6e21318f5..1d75621b3 100644 --- a/src/SideBar/index.js +++ b/src/SideBar/index.js @@ -65,6 +65,7 @@ const SideBar = () => { const sideBarRef = useRef(); + const isPickingLocation = useSelector((state) => state.view.mapLocationSelection.isPickingLocation); const sideBar = useSelector((state) => state.view.sideBar); const [showEventsBadge, setShowEventsBadge] = useState(false); @@ -147,12 +148,13 @@ const SideBar = () => { } }, [sidebarOpen, currentTab, socket, isReportDetailsViewActive]); + // NOTE: This is getting unmaintainable. Is it really a good practice to use escape like a navigation key? useEffect(() => { const onKeydown = (event) => { const wasEscapePressed = event.keyCode === 27; const isDetailsViewActive = isReportDetailsViewActive || isPatrolDetailsViewActive; const isSideBarFocused = sideBarRef.current.contains(document.activeElement); - if (wasEscapePressed && isDetailsViewActive && isSideBarFocused) { + if (wasEscapePressed && isDetailsViewActive && isSideBarFocused && !isPickingLocation) { navigate(`/${getCurrentTabFromURL(location.pathname)}`); } }; @@ -160,7 +162,7 @@ const SideBar = () => { document.addEventListener('keydown', onKeydown, false); return () => document.removeEventListener('keydown', onKeydown, false); - }, [isPatrolDetailsViewActive, isReportDetailsViewActive, location.pathname, navigate]); + }, [isPatrolDetailsViewActive, isPickingLocation, isReportDetailsViewActive, location.pathname, navigate]); useEffect(() => { sideBarRef.current.focus(); diff --git a/src/SideBar/index.test.js b/src/SideBar/index.test.js index 59ccd270b..1efd69224 100644 --- a/src/SideBar/index.test.js +++ b/src/SideBar/index.test.js @@ -107,6 +107,9 @@ describe('SideBar', () => { }, view: { featureFlagOverrides: {}, + mapLocationSelection: { + isPickingLocation: false, + }, userPreferences: {}, sideBar: {}, systemConfig: {