Conversation
- Bumped versions for @auth0/auth0-react to 2.24.0, @testing-library/user-event to 14.6.4, eslint-plugin-jest to 29.16.1, and globals to 17.11.0. - Updated yarn.lock to reflect the changes in dependencies.
- Updated the Patrol Overview component to include a new SummaryStats section, consolidating key statistics such as active time, paused time, and distance covered. - Refactored distance calculations to utilize a new utility function for better formatting and localization support. - Improved localization files across multiple languages (English, Spanish, French, Nepali, Portuguese, Swahili) to include new labels and distance formatting. - Removed deprecated stat display elements from the Activity section, streamlining the user interface. - Introduced a new hook for managing current time updates, enhancing the responsiveness of the patrol statistics.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a patrol “Summary Stats” section in the Patrol Overview Activity area, including tracked-subject distance selection and localized distance formatting.
Changes:
- Introduced
SummaryStatsUI with duration/paused/active time, distance (selectable subject), and event count. - Added tracked-subject distance aggregation via new selectors/utils and a shared
formatDistanceInKilometershelper. - Updated i18n resources and bumped a few JS dependencies.
Reviewed changes
Copilot reviewed 28 out of 30 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates lockfile for bumped dependencies. |
| package.json | Bumps Auth0 React, Testing Library user-event, eslint-plugin-jest, and globals. |
| src/utils/patrols.js | Adds elapsed/paused time helpers and tracked-subject extraction for segments. |
| src/utils/distance.js | Adds centralized i18n-aware km formatting helper. |
| src/selectors/patrols/index.js | Adds selectors to derive tracked subjects and distance covered per segment. |
| src/i18n.js | Bumps i18n files version to invalidate cached translations. |
| src/hooks/useCurrentTime.js | Adds a hook to refresh “now” on an interval for live elapsed-time UI. |
| src/common/images/icons/star.svg | Adds star icon for “patrol leader” indicator in subject menu. |
| src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/styles.module.scss | Removes old stats-row styling (moved to SummaryStats). |
| src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/index.js | Replaces inline stats markup with SummaryStats. |
| src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/index.test.js | Extends test store state with tracks. |
| src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/SummaryStats/index.js | New component implementing the summary stats + tracked-subject distance menu. |
| src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/SummaryStats/styles.module.scss | New styles for summary stats grid and the subject selection popover. |
| src/Patrols/DistanceCovered.js | Uses shared distance formatter; simplifies distance calculation. |
| src/PatrolTrackLegend/index.js | Uses shared distance formatter; simplifies distance calculation and rendering. |
| src/PatrolListItem/index.js | Updates PatrolDistanceCovered usage (removes suffix). |
| src/PatrolDetailView/Header/index.js | Updates PatrolDistanceCovered usage (removes suffix). |
| public/locales/en-US/utils.json | Adds utils.distanceInKilometers translation key. |
| public/locales/es/utils.json | Adds utils.distanceInKilometers translation key. |
| public/locales/fr/utils.json | Adds utils.distanceInKilometers translation key. |
| public/locales/ne-NP/utils.json | Adds utils.distanceInKilometers translation key. |
| public/locales/pt/utils.json | Adds utils.distanceInKilometers translation key. |
| public/locales/sw/utils.json | Adds utils.distanceInKilometers translation key. |
| public/locales/en-US/patrols.json | Moves activity stat labels under summaryStats keyPrefix. |
| public/locales/es/patrols.json | Moves activity stat labels under summaryStats keyPrefix. |
| public/locales/fr/patrols.json | Moves activity stat labels under summaryStats keyPrefix. |
| public/locales/ne-NP/patrols.json | Moves activity stat labels under summaryStats keyPrefix. |
| public/locales/pt/patrols.json | Moves activity stat labels under summaryStats keyPrefix. |
| public/locales/sw/patrols.json | Moves activity stat labels under summaryStats keyPrefix. |
| AGENTS.md | Updates docs to mention stats heading the activity timeline. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Added a new test for displaying the distance covered by an active patrol in the Header component. - Introduced a test for showing the distance covered by patrols in the PatrolListItem component. - Implemented a new test case in the PatrolOverview to ensure correct handling of events in summary statistics. - Added a new SummaryStats component test to validate the display of patrol statistics. - Updated package.json to enable restoreMocks in Jest configuration for improved test reliability.
…istance Calculations - Updated PatrolTrackLegend and PatrolDistanceCovered components to utilize the new formatDistanceInKilometers function for improved localization support. - Enhanced SummaryStats component to incorporate effective end time calculations for patrols, ensuring accurate display of statistics. - Introduced new utility functions for handling patrol state changes and calculating distances within specified time ranges. - Improved test coverage for distance calculations and patrol state handling, ensuring robust functionality across components.
- Updated the test for image loading to use expect with rejects, ensuring proper handling of promise rejections. - Removed unnecessary try-catch block for cleaner test code.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 46 changed files in this pull request and generated no new comments.
Suppressed comments (7)
src/utils/tracks.js:1
trackLengthWithinTimeRangewill throw iftrackData/trackData.track/trackData.track.featuresis missing (e.g., caller passes null or partially-loaded track data). Add an initial guard (similar to other track utils) before destructuringtrackData.track.features, returning 0 when the track container is absent.
src/utils/tracks.js:1- This assumes
trackFeature.properties.coordinateProperties.timesalways exists whenfrom/untilis provided. If geometry is present butcoordinateProperties.timesis missing, this will throw. Consider guarding for a missing/emptytimesarray and returning 0 (or falling back to measuring all coordinates) to avoid runtime errors.
src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/SummaryStats/index.js:133 - Preventing default on
Taband forcing focus back to the trigger breaks expected keyboard navigation (Tab should typically move focus to the next focusable element). Consider only closing the menu on Tab withoutpreventDefault(or follow the ARIA menu-button pattern by using a component that manages focus/Tab semantics).
case 'Tab':
case 'Escape':
event.preventDefault();
onDistanceSubjectMenuClose();
break;
src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/SummaryStats/index.js:56
- Using a state setter as a ref callback stores a DOM node in state and can introduce avoidable re-renders (and StrictMode double-invoke ref churn). Prefer
useReffor the anchor element (e.g.,const anchorRef = useRef(null)andref={anchorRef}), and readanchorRef.currentforOverlay.targetand focus management.
const [distanceSubjectMenuAnchorEl, setDistanceSubjectMenuAnchorEl] = useState();
src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/SummaryStats/index.js:162
- Using a state setter as a ref callback stores a DOM node in state and can introduce avoidable re-renders (and StrictMode double-invoke ref churn). Prefer
useReffor the anchor element (e.g.,const anchorRef = useRef(null)andref={anchorRef}), and readanchorRef.currentforOverlay.targetand focus management.
<button
src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/SummaryStats/index.js:169
- Using a state setter as a ref callback stores a DOM node in state and can introduce avoidable re-renders (and StrictMode double-invoke ref churn). Prefer
useReffor the anchor element (e.g.,const anchorRef = useRef(null)andref={anchorRef}), and readanchorRef.currentforOverlay.targetand focus management.
ref={setDistanceSubjectMenuAnchorEl}
src/SideBar/PatrolsManager/PatrolOverview/Overview/Activity/SummaryStats/index.js:73
- Since paused-time detection is intended to be implemented later, it would be safer to clamp
activeTimeto a minimum of 0 (e.g.,Math.max(0, duration - pausedTime)) to avoid negative values if the paused-time calculation ever exceeds total duration due to data issues or edge cases.
const { activeTime, duration, pausedTime } = useMemo(() => {
const duration = getElapsedTimeForPatrol(patrol, currentTime);
const pausedTime = getPausedTimeForPatrol(patrol, currentTime);
return { activeTime: duration - pausedTime, duration, pausedTime };
}, [currentTime, patrol]);
- Updated the SummaryStats component to ensure distance is only calculated when the patrol has started and the distance data is available. - Added tests to verify behavior for cancelled patrols and subjects with unloaded tracks, ensuring accurate display of distance statistics. - Refactored selectors to return null for distances of subjects whose tracks are not loaded, improving data handling in the application. - Enhanced utility functions to correctly determine effective end times for patrols based on state changes.
What does this PR do?
Calculates and fills the patrol summary stats.
Evidence
Desktop:

Mobile:

Relevant link(s)