Add button in atlases to be able to focus on the farm fields#506
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors atlas controls into a composable AtlasControls system and adds a "fly to fields" capability: Controls now accept Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant Controls as "AtlasControls\n(Controls)"
participant Map as "MapGL / Map Component"
participant MapRef as "MapRef (maplibre)\nfitBounds"
User->>Controls: Click "Fly to fields"
Controls->>Map: reset viewState / compute combined bounds
Controls->>MapRef: call fitBounds(bounds, options)
MapRef-->>Map: update camera / viewport
Map-->>Controls: new viewState applied
Controls-->>User: map centered on fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx (1)
263-291:⚠️ Potential issue | 🔴 CriticalMapGL component is missing the
refprop.The
mapRefis created but never passed to theMapGLcomponent. Withoutref={mapRef}, themapRef.current?.fitBounds()call at line 327 will always be a no-op sincemapRef.currentwill remainnull.🐛 Proposed fix
<MapGL {...viewState} + ref={mapRef} style={{ height: "calc(100vh - 64px - 147px)", width: "100%",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/farm.create`.$b_id_farm.$calendar.atlas.tsx around lines 263 - 291, The MapGL instance never receives the mapRef, so mapRef.current stays null and calls like mapRef.current?.fitBounds() do nothing; update the MapGL JSX to include ref={mapRef} (the ref created earlier) so the MapGL component is wired to the existing mapRef used elsewhere (e.g., the fitBounds call at line 327); ensure the ref type matches the MapGL/MapRef expected by the component and leave other handlers (onMove/onClick/handleClickSavedField) unchanged.
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/atlas/atlas-controls.tsx (1)
132-136: Minor typo in documentation.Line 135 has a typo: "tall" should be "tell".
📝 Proposed fix
/** * React root that can be added to a react-map-gl Map to include buttons etc. on it * - * - position will tall MapGL where to put the controls + * - position will tell MapGL where to put the controls */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/components/blocks/atlas/atlas-controls.tsx` around lines 132 - 136, Update the JSDoc comment in atlas-controls.tsx: change the typo "tall" to "tell" in the block describing the React root/MapGL controls so the sentence reads "position will tell MapGL where to put the controls"; locate the comment near the React root description in the atlas controls component and correct the single word.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-app/app/routes/farm.create`.$b_id_farm.$calendar.atlas.tsx:
- Around line 229-230: The file is missing imports for useRef and MapRef causing
build failure; add useRef to the React import (e.g., import React, { useRef }
from 'react') and import MapRef from 'react-map-gl' (e.g., import type { MapRef
} from 'react-map-gl') so the mapRef declaration const mapRef =
useRef<MapRef>(null) resolves correctly; update the existing React and
react-map-gl import blocks to include these symbols.
- Around line 318-332: The onFlyToFields handler uses featureCollection but the
symbol is not imported; update the `@turf/turf` import to include
featureCollection (the code using it is inside onFlyToFields which builds a
combined featureCollection from fieldsSaved.features and
selectedFieldsData.features, computes overallViewState via getViewState, calls
setViewState and mapRef.current?.fitBounds). Add featureCollection to the
existing import alongside simplify so the identifier resolves and the
onFlyToFields logic works.
---
Outside diff comments:
In `@fdm-app/app/routes/farm.create`.$b_id_farm.$calendar.atlas.tsx:
- Around line 263-291: The MapGL instance never receives the mapRef, so
mapRef.current stays null and calls like mapRef.current?.fitBounds() do nothing;
update the MapGL JSX to include ref={mapRef} (the ref created earlier) so the
MapGL component is wired to the existing mapRef used elsewhere (e.g., the
fitBounds call at line 327); ensure the ref type matches the MapGL/MapRef
expected by the component and leave other handlers
(onMove/onClick/handleClickSavedField) unchanged.
---
Nitpick comments:
In `@fdm-app/app/components/blocks/atlas/atlas-controls.tsx`:
- Around line 132-136: Update the JSDoc comment in atlas-controls.tsx: change
the typo "tall" to "tell" in the block describing the React root/MapGL controls
so the sentence reads "position will tell MapGL where to put the controls";
locate the comment near the React root description in the atlas controls
component and correct the single word.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fbec3b85-e30b-4ce5-8b10-81cfb8ad0a59
📒 Files selected for processing (7)
.changeset/brave-kiwis-wash.mdfdm-app/app/components/blocks/atlas/atlas-controls.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.soil.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new._index.tsxfdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/atlas/atlas-controls.tsx (1)
170-187: Consider addingaria-labelfor improved accessibility.The button uses
titlefor the tooltip, which provides hover text but limited screen reader support. Addingaria-label={props.label}would improve accessibility for assistive technologies.♿ Proposed accessibility improvement
<button className="maplibregl-ctrl-icon flex items-center justify-center p-0!" type="button" title={props.label} + aria-label={props.label} onClick={(e) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/components/blocks/atlas/atlas-controls.tsx` around lines 170 - 187, The AtlasButton component currently only sets title for the button which is not reliably read by screen readers; update the JSX for the button in AtlasButton to add aria-label={props.label} (or a fallback string when props.label is undefined) alongside the existing title, ensuring the accessible name comes from props.label and remains consistent with props.Icon usage and the onClick behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@fdm-app/app/components/blocks/atlas/atlas-controls.tsx`:
- Around line 170-187: The AtlasButton component currently only sets title for
the button which is not reliably read by screen readers; update the JSX for the
button in AtlasButton to add aria-label={props.label} (or a fallback string when
props.label is undefined) alongside the existing title, ensuring the accessible
name comes from props.label and remains consistent with props.Icon usage and the
onClick behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4749ed9b-bc35-4bce-be68-52317fd5e431
📒 Files selected for processing (2)
fdm-app/app/components/blocks/atlas/atlas-controls.tsxfdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx
Enhancements
Summary by CodeRabbit