Skip to content

ERA-10349: Support EFB schemas in ER > Location Field [3] #1268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 31, 2025
Merged

ERA-10349: Support EFB schemas in ER > Location Field [3] #1268

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00b4a9f
ERA-10349: Fix bug when pressing escape while choosing location
luixlive Mar 31, 2025
e1e8a4c
ERA-10349: Add and fix unit tests
luixlive Mar 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 21 additions & 16 deletions src/LocationPicker/MenuPopover/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -147,12 +152,12 @@ const MenuPopover = ({
style={{ ...style, minWidth: popoverWidth, width: popoverWidth }}
{...otherProps}
>
<div className={styles.wrapper} onKeyDown={onWrapperKeyDown} ref={wrapperRef}>
<div className={styles.wrapper} onKeyDown={isPickingLocation ? undefined : onWrapperKeyDown} ref={wrapperRef}>
<GpsInput
gpsFormatToggleRef={gpsFormatToggleRef}
id={`${id}-menuPopover-gpsInput`}
inputRef={gpsInputRef}
onKeyDown={onGpsInputKeyDown}
onKeyDown={isPickingLocation ? undefined : onGpsInputKeyDown}
onChange={onChange}
value={value}
/>
Expand Down
6 changes: 4 additions & 2 deletions src/SideBar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -147,20 +148,21 @@ const SideBar = () => {
}
}, [sidebarOpen, currentTab, socket, isReportDetailsViewActive]);

// NOTE: This is getting unmaintainable. Is it really a good practice to use escape like a navigation key?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature has been causing many issues since we added it. This useEffect adds a key down listener in the SideBar to navigate back if Escape is pressed. I wonder if it's worth it to keep adding patches everytime we find a new unwanted side effect this is adding. Pressing Escape to navigate is not common. I know that key is used to close views, and given our navigation is related to a SideBar it kind of makes sense to expect the side bar to close when pressing it. But our SideBar is not just a presentational element to close, its tabs and its items are connected to the router, causing the mentioned side effects like the one I'm fixing it when pressing escape while picking a location in the map.

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)}`);
}
};

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();
Expand Down