Fix LiveView input guards, rover slider sync, and beacon pending list #23
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/liveview-guards"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Audit LiveView/TS group (part 2).
Verified: ~370 targeted tests + tsc green.
- Rover forecast handlers clamp to the slider's 18 h range and push a set_slider event so keyboard steps stay in sync with the slider - phx-window-keydown on /rover replaced with the NavKeys hook (ignores arrows inside inputs/selects and the map container) - point_detail/rover_cell_detail/gps_location handlers guard is_number(lat/lon) with a fallthrough — malformed payloads no longer crash the LiveView - Beacon pending-approval section tracks a live pending_count and stream_deletes on approval instead of gating on the mount-time list - BeaconLive.Show approve no longer crashes on {:error, _} - Rover cell popup 'Open in Maps' link includes the longitude; the rescue fallback now logs - LocationResolver rejects out-of-range lat/lon - Logged previously-swallowed failures in seed_default_stations, live_table_resource export cleanup, and submit_live enqueue🤖 Skippy PR review
2 findings — 1 blocking before merge.
lib/microwaveprop_web/live/rover_live.ex:1037lib/microwaveprop_web/live/rover_live.ex:40Reviewed
697fbc32f180. Commentskippy reviewto re-run.@ -37,1 +37,4 @@@default_forecast_hour 0# Must match `max` on the forecast-hour range input below — the slider# ticks and the keyboard step clamp share this ceiling.@max_forecast_hour 18🔵 Suggestion — Slider ceiling still duplicated instead of derived
The comment says this must match
maxon the forecast-hour range input, but the input hardcodesmax="18"(~line 1506), so handler and slider can drift apart the same way they just did. Interpolate it in the template (max={@max_forecast_hour}) and the invariant is enforced instead of documented.@ -1012,2 +1035,3 @@id="rover-page"class="flex flex-col w-screen h-screen overflow-hidden bg-base-200 text-base-content"phx-window-keydown="nav_key"phx-hook="NavKeys"🟠 High — NavKeys hook does not exist on main, so /rover loses its arrow keys
NavKeysis not registered anywhere at this ref:assets/js/nav_keys_hook.tsis not inassets/js/onmainandapp.tshas noNavKeysin itshooksmap (both live only on the unmergedfix/liveview-hooks, PR #22). Merged standalone, LiveView has no hook to mount on#rover-page, and thephx-window-keydown="nav_key"binding this line replaces is gone, so left/right (forecast hour) and up/down (band) do nothing on /rover. The suite stays green becauserender_keydown/3pushesnav_keystraight into the LiveView and never touches the JS hook, and nothing else validates hook names (esbuild does not,mix xrefdoes not). Land #22 first and rebase this onto it, or keep the window binding in place until NavKeys is onmain.697fbc32f18025d33a49🤖 Skippy PR review
1 finding — 1 blocking before merge.
lib/microwaveprop_web/live/rover_live.ex:378Follow-up: this push was a rebase/force-push (
697fbc32is no longer in the branch history), so the whole PR diff was re-read via FULL-DIFF-FALLBACK, not just the new commits. 2 earlier findings are still open at this head and were not reposted: the missing NavKeys hook (High, still absent from assets/js and app.ts, PR #22 still unmerged) and the slider ceiling still hardcoded as max="18" in the template (Suggestion, partially addressed: both clamps now use @max_forecast_hour).Reviewed
8025d33a4996. Commentskippy reviewto re-run.@ -360,3 +376,3 @@enddef handle_event("rover_cell_detail", %{"lat" => lat, "lon" => lon}, socket) dodef handle_event("rover_cell_detail", %{"lat" => lat, "lon" => lon}, socket) when is_number(lat) and is_number(lon) do🟡 Warning — 🟠 Warning: The rover cell popup this PR fixes cannot be reached from the browser
Nothing in the tree pushes
rover_cell_detailand nothing listens forshow_cell_popup. A repo-wide grep for either name returns only this handler,push_cell_popup/3(line 702, whosepush_eventon line 725 has no JShandleEventfor it) and the new test;assets/js/rover_map_hook.tscontains zeropushEventcalls, so the rover map never talks to the server the waypropagation_map_hook/weather_map_hookdo (point_detail). Consequence: the "Open in Maps" longitude fix on line 761 is unreachable in a real browser, the CHANGELOG bullet describing it tells users about a link they cannot click, and the newrender_hook/3test passes only because it injects the event directly, bypassing the missing client half. Either wire the cell click inrover_map_hook.ts(this.pushEvent("rover_cell_detail", {lat, lon})plus ahandleEvent("show_cell_popup")that opens the popup), or delete the dead handler/popup and drop the changelog line: a dead code path also means theis_numberguard added here can never fire in production. Same root cause as the/roverarrow-key finding: this page's client half only exists on another branch.8025d33a49ccd264e8efAddressed in
ccd264e8:NavKeysinapp.tsand shipsnav_keys_hook.ts) merged first, and this branch is rebased on top of it.phx-hook="NavKeys"now resolves.max_forecast_hour()(a function returning@max_forecast_hour) instead of a hardcodedmax="18", so template and handlers share one source.Resolved 2 of 3 earlier findings:
/rovermounts it on#rover-page, so the arrow-key fix stands.@max_forecast_hourin both the input'smaxand every clamp.Still open (1): the rover cell popup remains unreachable from the browser -
rover_map_hook.tsnever pushesrover_cell_detailand nothing handlesshow_cell_popup, so the Maps-link longitude fix and its CHANGELOG line are still user-invisible. This is now inmain(merged as221deb1).Bookkeeping: the branch was force-pushed a second time (
8025d33ais no longer in history), so the whole diff was re-read rather than justccd264e8. No new defects in it; theis_numberguards are safe (every JS caller pushes real numbers) andset_slidercannot loop (the hook'sinputHandleronly paints the label, it does not push back).