RF-1224: Use deployment dates instead of update dates in gear popup#1550
RF-1224: Use deployment dates instead of update dates in gear popup#1550jeslefcourt wants to merge 9 commits into
Conversation
Gear popup header and device list now show last_deployed timestamps. The header falls back to gear.last_updated when no device has a deployment date. Devices are sorted ascending by last_deployed, with undated devices last. Popup width increased 25% and name is now clickable (jump to location). Adds test coverage for device sort order and popup rendering branches. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the Gear popup and sidebar interactions to use deployment dates (instead of update dates) and improve navigation to gear locations.
Changes:
- Gear popup header now uses the most recent
device.last_deployed(fallback togear.last_updated) and device rows showlast_deployed. - Devices in the popup are sorted ascending by
last_deployed(undated devices last; ties preserve order). - Gear popup styling updated to widen the Mapbox popup content, and gear names in the sidebar are now clickable to jump to location.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/SideBar/GearTab/styles.module.scss | Adds styling for a “clickable” gear display id in the sidebar list. |
| src/SideBar/GearTab/index.js | Makes the gear name clickable/keyboard-focusable to jump to the gear’s location. |
| src/PopupLayer/index.js | Applies a GearPopup-specific popup class for width styling via popup template attrs. |
| src/GearPopup/styles.module.scss | Adds a .popup wrapper to increase Mapbox popup content width for gear popups. |
| src/GearPopup/index.js | Switches popup header/device dates to last_deployed logic and sorts devices by deployment date. |
| src/GearPopup/index.test.js | Adds unit tests for GearPopup rendering and device sort order. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…oyed across devices Agent-Logs-Url: https://github.com/PADAS/das-web-react/sessions/5f2a8b61-505a-4b8d-bebe-cedff1eba9cd Co-authored-by: jeslefcourt <32883811+jeslefcourt@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — small frontend change with good test coverage.
Extended reasoning...
Overview
This PR updates the gear popup to surface the most recent device last_deployed date (with fallback to gear.last_updated), sorts devices ascending by deployment date, widens the popup, and makes the gear name in the sidebar list clickable to jump to location. Touches 7 files, all frontend (React + SCSS), with a new 141-line test file covering the popup behavior and sort ordering edge cases.
Security risks
None. No auth, crypto, network calls, user input parsing, or permission logic. Date strings are compared lexicographically (safe for ISO 8601), and the new offset option to useJumpToLocation is conditionally spread only when truthy.
Level of scrutiny
Low. This is presentation-layer logic with clear intent, deterministic behavior, and direct test coverage. The most-recent-date reduce, the sort comparator, and the conditional offset spread are all small and easy to verify by inspection. The test file specifically covers the most-recent-date selection (added in 00bfe94 in response to Copilot feedback), the fallback path, undated-devices-last ordering, and stable sort for ties.
Other factors
- Prior Copilot review surfaced 4 items; 3 were addressed (focus-visible styling now present in SCSS, missing test added, PR description corrected).
- The remaining nit (Space key on the role=button span doesn't
preventDefault, so Space both activates and scrolls the page) is flagged as an inline comment by the bug-hunting system. It's a real a11y nit but minor and isolated — does not block approval. - No CODEOWNER-sensitive areas touched. The
useJumpToLocationhook change is backward-compatible (new optional second-arg property).
luixlive
left a comment
luixlive
left a comment
There was a problem hiding this comment.
Almost done, some small suggestions on improvements for code readability and HTML semantics 👍
- Memoize most-recent-deployment date in GearPopup with useMemo - Extract device sort comparator to top-level sortByLastDeployed helper - useJumpToLocation: accept generic options param spread into fit/easeTo - GearTab: render semantic <button> instead of span[role=button], drop <p> - Reset button defaults in styles so it matches the prior span appearance Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… options - GearPopup: compare last_deployed via Date() in both the sort comparator and the most-recent-date reducer, removing the lexicographic timezone assumption - useJumpToLocation: JSDoc the new options param and its override behavior - GearTab: document the approximate 175px popup-center offset Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
last_deployedacross all devices, falling back togear.last_updatedlast_deployed(undated devices last, stable sort for ties)16.25rem)Fixes: RF-1224
Test plan
last_deployedappear after dated devices in the popup listnpx jest src/GearPopup/index.test.js— all 11 tests pass🤖 Generated with Claude Code