Add AHN4 elevation map feature with COG integration#376
Conversation
🦋 Changeset detectedLatest commit: d0f243c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded@SvenVw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds an AHN4 elevation feature to Atlas: new elevation route and map UI using a COG protocol, elevation controls and legend, breadcrumb/sidebar navigation updates, projection and spatial helpers, COG index caching, and two new dependencies for COG protocol and spatial intersection. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as UI/Controls
participant Map as MapLibre GL
participant Loader as Route/Loader
participant Index as Kaartbladindex (cached)
participant PDOK as PDOK COG Service
participant Legend as ElevationLegend
User->>UI: Open Atlas → select Hoogtekaart
UI->>Loader: Initialize elevation route
Loader->>Index: Fetch kaartbladindex.json (localStorage cache)
Index-->>Loader: Return tile index
rect rgb(220,240,255)
Note over Loader,Map: Initial viewport setup
Loader->>Map: Set view + register cog protocol / RD projection
Map->>Index: Request visible tiles (bounds → RD)
Index-->>Map: Return intersecting tiles
Map->>PDOK: Request COG tiles & hillshade (via cog://)
PDOK-->>Map: Return raster tiles
end
rect rgb(240,220,255)
Note over Loader,Legend: Dynamic scaling
Loader->>PDOK: Sample overviews for visible tiles (min/max)
PDOK-->>Loader: Return sampled elevation stats
Loader->>Legend: Update min/max and color scale
Legend-->>UI: Render gradient + labels
end
rect rgb(220,255,230)
Note over User,Map: Interaction (hover/pan)
User->>Map: Hover / Pan / Zoom
Map->>PDOK: Query elevation at point or request tile samples
PDOK-->>Map: Return elevation value(s)
Map->>Legend: Update hoverValue / Loader: throttle & recalc stats
Legend-->>User: Display "Hoogte: X m NAP"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
coderabbitai
Bot
changed the title
coderabbitai
Bot
added
branch:development
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
fdm-app/package.json (1)
17-47: Consider pinning COG protocol dependency more strictly and reviewing Turf duplicationTwo small dependency hygiene points:
@geomatico/maplibre-cog-protocolis pulled from a branch (#add-prepare). For reproducibility, consider pinning to a specific commit SHA or a released version once available.- You now depend on both
@turf/turfand@turf/boolean-intersects. If you don’t need the standalone package for bundle-size reasons, you could drop it and import from@turf/turfinstead to reduce duplication.fdm-app/app/components/blocks/header/atlas.tsx (1)
2-51: Header dropdown implementation looks solid; consider tightening the route checkThe dropdown wiring and use of
NavLinkfor “Gewaspercelen” / “Hoogtekaart” look good and align with the new atlas routes.You currently derive
isElevationvialocation.pathname.includes("/elevation"). If this header is ever reused on other pages that also contain/elevationin the path, this could mislabel the current view. A more specific check (e.g. matching the atlas elevation route prefix or equality) would make this a bit more robust, but it’s not blocking.fdm-app/app/components/blocks/atlas/atlas-controls.tsx (1)
1-195: Nice abstraction of map controls; small DRY opportunityThe refactor to a generic
ControlButton+CustomControlworks well, and the newElevationControlis wired cleanly viashowElevation/onToggleElevation.Both
FieldsControlandElevationControlrepeat the sameCustomControlconstruction andupdatePropsshape (only labels and icon differ). You could factor a small helper likecreateToggleControlProps({ active, onToggle, labels, Icon })to avoid duplicating these object literals in two places, but the current version is perfectly readable.fdm-app/app/components/blocks/sidebar/apps.tsx (1)
43-58: Atlas sidebar collapsible and sub-links look consistent with existing patternsThe introduction of
atlasFieldsLink/atlasElevationLinkand the collapsible “Atlas” section fits nicely with the existing sidebar structure (e.g. Balans). Conditional enabling in the create-farm wizard and the active-state wiring viaisActiveall look correct.If you ever add routes whose paths merely contain these URLs as substrings, the
location.pathname.includes(atlasFieldsLink/atlasElevationLink)checks could become slightly over-broad. In that case, switching to stricter matching (e.g. equality or prefix checks) would avoid accidental active states, but for the current route set this is fine.Also applies to: 100-158
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx (2)
1-90: Projection and COG protocol setup are appropriate; consider using Turf for intersectionsRegistering EPSG:28992 via
proj4.defsand the globalmaplibregl.addProtocol("cog", cogProtocol)at module scope is a reasonable one-time setup for this route.For tile visibility, you’ve implemented
isPointInPolygon+polygonIntersectsPolygonwith a simplified “any vertex inside the other” heuristic. Given thatkaartbladindexpolygons are regular tiles, this is likely sufficient in practice, but you’ve also introduced@turf/boolean-intersectsas a dependency inpackage.json. If you want more robust intersection handling with less custom geometry code to maintain, you could delegate this check to Turf instead of keeping a bespoke implementation.
266-436: Viewport sampling and COG tile updates are thoughtful; watch network load per updateThe
updateVisibleTilesflow is well-designed overall: zoom gating (<13 uses WMS, >=13 uses COG), cancellation viaupdateId, slow-network detection, legend range padding, and limiting to 24 tiles all help keep things responsive.One thing to keep an eye on is the number of remote reads per update: for a 4×4 grid you can end up calling
locationValuesonce per sample point, even when several samples land on the same tile. In practice that means up to 16 COG queries per throttled update. If you see this becoming a bottleneck, a small optimization would be to:
- Group
samplePointsby the tile feature they fall into, and- Call
locationValuesonce per (tile, samplePoints[]) pair, or cache per-tile results across nearby updates.Not urgent, but it could materially reduce network and COG decoding load on very active panning/zooming sessions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
fdm-app/app/lib/cache.server.tsis excluded by!fdm-app/app/lib/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/jolly-ravens-shop.md(1 hunks)fdm-app/app/components/blocks/atlas/atlas-controls.tsx(8 hunks)fdm-app/app/components/blocks/atlas/atlas-legend.tsx(1 hunks)fdm-app/app/components/blocks/header/atlas.tsx(2 hunks)fdm-app/app/components/blocks/sidebar/apps.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx(2 hunks)fdm-app/package.json(2 hunks)fdm-app/vite.config.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (17)
📚 Learning: 2024-11-25T12:42:32.783Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
Applied to files:
fdm-app/vite.config.ts
📚 Learning: 2024-12-11T12:09:35.540Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 20
File: fdm-app/tsconfig.json:8-9
Timestamp: 2024-12-11T12:09:35.540Z
Learning: In the `fdm-app/tsconfig.json` file, the include path `.react-router/types/**/*` refers to a build-time generated directory which is intentionally not included in the repository.
Applied to files:
fdm-app/vite.config.ts
📚 Learning: 2025-01-24T11:46:49.990Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-data/rollup.config.js:7-17
Timestamp: 2025-01-24T11:46:49.990Z
Learning: When suggesting external dependencies in Rollup configuration, only include packages that are actually listed in the package's dependencies or peerDependencies.
Applied to files:
fdm-app/vite.config.ts
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/sidebar/apps.tsxfdm-app/app/components/blocks/header/atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
Applied to files:
fdm-app/app/components/blocks/sidebar/apps.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/components/blocks/sidebar/apps.tsx
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-app/package.json
📚 Learning: 2025-08-13T10:33:05.313Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-08-13T10:33:05.313Z
Learning: In the fdm project, fdm-calculator integration for new features like b_lu_variety is handled in separate updates from the core data model changes. When fdm-core functions are updated to support new fields, fdm-calculator can consume these enhanced APIs without requiring changes in the same PR that introduces the core functionality.
Applied to files:
fdm-app/package.json
📚 Learning: 2025-06-10T13:10:03.154Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 161
File: fdm-app/app/components/blocks/field-map.tsx:0-0
Timestamp: 2025-06-10T13:10:03.154Z
Learning: When facing prop name inconsistencies with react-map-gl (like mapboxAccessToken vs mapboxApiAccessToken), using different import statements can resolve the issue more elegantly than changing prop names across multiple files.
Applied to files:
fdm-app/app/components/blocks/atlas/atlas-controls.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2024-11-25T14:42:26.660Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/app/components/blocks/field-map.tsx:0-0
Timestamp: 2024-11-25T14:42:26.660Z
Learning: In `fdm-app/app/components/blocks/field-map.tsx`, explicit cleanup of Mapbox GL resources is not necessary, as `react-map-gl` handles it automatically upon component unmount, and `MapRef` does not have a `remove` method.
Applied to files:
fdm-app/app/components/blocks/atlas/atlas-controls.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T15:41:43.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/components/custom/atlas/atlas-panels.tsx:28-28
Timestamp: 2025-01-31T15:41:43.741Z
Learning: When handling different map event types in react-map-gl v7.1.8, use MapLayerMouseEvent for mouse events (which have the point property) and ViewStateChangeEvent for view state changes. Use a type guard like 'point' in evt to safely access event-specific properties.
Applied to files:
fdm-app/app/components/blocks/atlas/atlas-controls.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T15:06:35.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/components/custom/atlas/atlas-sources.tsx:21-66
Timestamp: 2025-01-31T15:06:35.764Z
Learning: In react-map-gl components, when querying rendered features via map.queryRenderedFeatures(), the effect's dependency array must include any props that affect the map's rendered state (like source data) to ensure features are queried against the current map state.
Applied to files:
fdm-app/app/components/blocks/atlas/atlas-controls.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T16:06:33.810Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:164-212
Timestamp: 2025-01-31T16:06:33.810Z
Learning: Map configuration in the application should be modularized using the `useMapConfig` hook and `MapControls` component to maintain consistency across all MapGL instances.
Applied to files:
fdm-app/app/components/blocks/atlas/atlas-controls.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T16:06:33.810Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:164-212
Timestamp: 2025-01-31T16:06:33.810Z
Learning: MapGL implementations should use the shared `useMapConfig` hook for configuration and `getLayerStyle` utility for consistent styling. The hook supports both interactive and non-interactive maps, handling bounds calculation and view state management.
Applied to files:
fdm-app/app/components/blocks/atlas/atlas-controls.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/components/blocks/header/atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2024-12-16T10:56:07.561Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 16
File: fdm-app/app/routes/app.addfarm.$b_id_farm.cultivations.$b_lu_catalogue.fertilizers.tsx:1-1
Timestamp: 2024-12-16T10:56:07.561Z
Learning: The project uses `react-router` v7, and the `data` function is exported and used for error handling in loaders and actions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T14:29:37.599Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/components/custom/atlas/atlas.d.tsx:8-8
Timestamp: 2025-01-31T14:29:37.599Z
Learning: In the Atlas component's MapFieldsProps interface, mapStyle is intentionally restricted to "mapbox://styles/mapbox/satellite-streets-v12" as it's currently the only supported style option.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
🪛 GitHub Check: CodeQL
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
[failure] 212-212: Clear text storage of sensitive information
This stores sensitive data returned by an access to longitude as clear text.
This stores sensitive data returned by an access to latitude as clear text.
This stores sensitive data returned by an access to longitude as clear text.
This stores sensitive data returned by an access to latitude as clear text.
This stores sensitive data returned by an access to longitude as clear text.
This stores sensitive data returned by an access to latitude as clear text.
This stores sensitive data returned by an access to longitude as clear text.
This stores sensitive data returned by an access to latitude as clear text.
🔇 Additional comments (6)
fdm-app/vite.config.ts (1)
34-35: Add COG protocol package tonoExternallooks correctIncluding
@geomatico/maplibre-cog-protocolalongside the other SSR externals is consistent with the existing pattern and should help avoid SSR bundling issues. No further changes needed here..changeset/jolly-ravens-shop.md (1)
1-5: Changeset text and bump level match the feature scopeDescription clearly captures the AHN4 elevation work, and using a minor bump for @svenvw/fdm-app is appropriate.
fdm-app/app/components/blocks/atlas/atlas-legend.tsx (1)
1-63: ElevationLegend component is well-scoped and readableProps, conditional rendering (loading/slow/error), and the gradient scale are all handled cleanly. The min/max/hover formatting and fallbacks (“Laag”/“Hoog”) make sense for the AHN4 use case. No changes needed from my side.
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx (3)
110-150: Loader logic for fields and map style looks correctThe loader’s guard on
b_id_farm && b_id_farm !== "undefined"and conversion ofgetFieldsresults into a GeoJSONFeatureCollection(with the extrab_lu_name/b_id_sourceproperties) are all consistent and type-safe. ReturningmapStyle = getMapStyle("satellite")alongsidefields/calendarprovides everything the client component needs without over-fetching. No changes needed here.
468-521: Hover elevation handler and throttling look goodThe throttled
handleMouseMovethat:
- Short-circuits below zoom 13 or when no active tiles,
- Transforms to RD and reuses the same point-in-polygon logic as the tile sampler, and
- Calls
locationValuesfor the hovered location and updateshoverElevation,is a sensible approach and ties in nicely with
ElevationLegend. Using a sharedstateRefto avoid stale closures in the throttled function is also a nice touch. I don’t see any correctness issues here.
524-662: MapGL composition and layering are coherent and match the intended UXThe main
MapGLinstance is wired cleanly:
- View state is controlled via
onMove/onMoveEndwith throttled tile updates.showElevationconsistently gatesonMouseMove, WMS overview, COG raster tiles, hillshade, and the legend’sshowScale.- Fields are overlaid via a single GeoJSON source with outline and (invisible but interactive) fill layers, with
beforeIdcorrectly positioning elevation layers under the outlines when fields are present.- The floating
ElevationLegend+FieldsPanelHoverpanel in the top-left corner matches the Atlas UX.Overall this composition looks robust and matches the AHN4 elevation feature description.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development #376 +/- ##
============================================
Coverage 87.41% 87.41%
============================================
Files 91 91
Lines 4497 4497
Branches 1345 1345
============================================
Hits 3931 3931
Misses 566 566
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
BoraIneviNMI
left a comment
BoraIneviNMI
left a comment
There was a problem hiding this comment.
It looks and works great.
I was only annoyed as an user by all tiles disappearing while the map switches from WMS to COG or the other way, and when my connection is slow. Seconds would pass before new elevation detail appeared.
I tried to find a fix myself but couldn't. I thought of using the network state to clear the previous tiles at the right time, but network state wasn't very useful for this.
Maybe it is just my connection and this is not an important concern.
Thanks for the feedback! I have improved the transition between WMS and COG in ba8ef5f |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx (4)
267-436: Consider extracting tile update logic into smaller functions.The
updateVisibleTilesfunction handles multiple responsibilities in 170 lines: viewport conversion, tile filtering, elevation sampling, and min/max calculation. This makes the code harder to test and maintain.Consider extracting helper functions:
// Separate concerns into focused functions function convertViewportToRD(bounds: LngLatBounds): [number, number][] { // Lines 292-305 } function findIntersectingTiles( viewportRD: [number, number][], index: FeatureCollection, maxTiles: number = 24 ): Feature[] { // Lines 309-316 } async function sampleElevationRange( bounds: LngLatBounds, visibleFeatures: Feature[], gridSize: number = 3 ): Promise<{ min: number; max: number }> { // Lines 319-396 } function createActiveTiles( features: Feature[], min: number, max: number ): ActiveTile[] { // Lines 404-421 }This would make the main function more readable and each piece independently testable.
316-316: Document the 24-tile limit rationale.The hard-coded limit
.slice(0, 24)constrains visible tiles but lacks explanation for why 24 specifically. Consider adding a comment explaining the performance/memory trade-off, or extracting it as a named constant.+// Limit active tiles to prevent memory/performance issues +const MAX_ACTIVE_TILES = 24 + const visibleFeatures = indexData.features .filter((f) => { if (!f.geometry || f.geometry.type !== "Polygon") return false const ring = (f.geometry as any).coordinates[0] return polygonIntersectsPolygon(rdCoords, ring) }) - .slice(0, 24) + .slice(0, MAX_ACTIVE_TILES)
313-313: Improve type safety for GeoJSON geometry access.The code uses
as anyto access polygon coordinates in multiple places (lines 313, 344, 489). This bypasses TypeScript's type checking and could lead to runtime errors if the geometry type assumption is incorrect.Create a type guard:
import type { Polygon } from "geojson" function isPolygonFeature(feature: Feature): feature is Feature<Polygon> { return feature.geometry?.type === "Polygon" }Then use it consistently:
const visibleFeatures = indexData.features .filter((f) => { - if (!f.geometry || f.geometry.type !== "Polygon") - return false - const ring = (f.geometry as any).coordinates[0] - return polygonIntersectsPolygon(rdCoords, ring) + if (!isPolygonFeature(f)) return false + const ring = f.geometry.coordinates[0] + return polygonIntersectsPolygon(rdCoords, ring) })Apply the same pattern at lines 344 and 489.
581-581: Consider extracting Netherlands bounds as a named constant.The bounds
[3.3, 50.7, 7.2, 53.7]appear multiple times (lines 581, 602) and represent the Netherlands bounding box. Extracting this as a named constant would improve maintainability.const NETHERLANDS_BOUNDS: [number, number, number, number] = [3.3, 50.7, 7.2, 53.7] // Then use in Source components: <Source // ... bounds={NETHERLANDS_BOUNDS} />
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
fdm-app/app/components/blocks/atlas/atlas-legend.tsx(1 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields._index.tsx(1 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-app/app/components/blocks/atlas/atlas-legend.tsx
🧰 Additional context used
🧠 Learnings (15)
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields._index.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields._index.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields._index.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks, as evidenced by the changelog store using createJSONStorage(() => localStorage) directly.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2024-11-25T14:42:26.660Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/app/components/blocks/field-map.tsx:0-0
Timestamp: 2024-11-25T14:42:26.660Z
Learning: In `fdm-app/app/components/blocks/field-map.tsx`, explicit cleanup of Mapbox GL resources is not necessary, as `react-map-gl` handles it automatically upon component unmount, and `MapRef` does not have a `remove` method.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T16:06:33.810Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:164-212
Timestamp: 2025-01-31T16:06:33.810Z
Learning: Map configuration in the application should be modularized using the `useMapConfig` hook and `MapControls` component to maintain consistency across all MapGL instances.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T16:06:33.810Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:164-212
Timestamp: 2025-01-31T16:06:33.810Z
Learning: MapGL implementations should use the shared `useMapConfig` hook for configuration and `getLayerStyle` utility for consistent styling. The hook supports both interactive and non-interactive maps, handling bounds calculation and view state management.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T15:06:35.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/components/custom/atlas/atlas-sources.tsx:21-66
Timestamp: 2025-01-31T15:06:35.764Z
Learning: In react-map-gl components, when querying rendered features via map.queryRenderedFeatures(), the effect's dependency array must include any props that affect the map's rendered state (like source data) to ensure features are queried against the current map state.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T15:41:43.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/components/custom/atlas/atlas-panels.tsx:28-28
Timestamp: 2025-01-31T15:41:43.741Z
Learning: When handling different map event types in react-map-gl v7.1.8, use MapLayerMouseEvent for mouse events (which have the point property) and ViewStateChangeEvent for view state changes. Use a type guard like 'point' in evt to safely access event-specific properties.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-31T14:29:37.599Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/components/custom/atlas/atlas.d.tsx:8-8
Timestamp: 2025-01-31T14:29:37.599Z
Learning: In the Atlas component's MapFieldsProps interface, mapStyle is intentionally restricted to "mapbox://styles/mapbox/satellite-streets-v12" as it's currently the only supported style option.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-06-10T13:10:03.154Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 161
File: fdm-app/app/components/blocks/field-map.tsx:0-0
Timestamp: 2025-06-10T13:10:03.154Z
Learning: When facing prop name inconsistencies with react-map-gl (like mapboxAccessToken vs mapboxApiAccessToken), using different import statements can resolve the issue more elegantly than changing prop names across multiple files.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
🧬 Code graph analysis (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx (5)
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.tsx (1)
loader(42-77)fdm-app/app/components/blocks/atlas/atlas-styles.tsx (1)
getFieldsStyle(8-12)fdm-app/app/components/blocks/atlas/atlas-viewstate.tsx (1)
getViewState(19-42)fdm-app/app/components/blocks/atlas/atlas-controls.tsx (1)
Controls(26-54)fdm-app/app/components/blocks/atlas/atlas.tsx (1)
ZOOM_LEVEL_FIELDS(1-1)
🔇 Additional comments (7)
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.tsx (1)
24-24: Meta title update aligns with Atlas rebranding.The title change from "Kaarten" to "Atlas" is consistent with the PR's objective to rename the map section.
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields._index.tsx (1)
31-31: Consistent Atlas naming convention.The meta title update maintains consistency with the broader Atlas rebranding across all map-related routes.
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx (5)
54-83: Spatial intersection helpers are appropriate for RD coordinates.The custom
isPointInPolygonandpolygonIntersectsPolygonimplementations work directly with RD New (EPSG:28992) projected coordinates. While@turf/boolean-intersectsis available as a dependency, it operates on WGS84 geographic coordinates, making these custom helpers necessary for the projected coordinate system.The simplified intersection check is appropriate for tile filtering and should perform well.
106-154: Loader implementation follows established patterns.The loader correctly fetches farm fields and prepares them as a GeoJSON FeatureCollection, consistent with the fields index route. Error handling and data transformation are appropriate.
161-183: State management is well-structured.The component maintains appropriate state for elevation rendering, including tile tracking, loading states, legend ranges, and UI toggles. The separation of concerns between network status, update status, and data state is clear.
468-521: Hover elevation query is appropriately throttled.The mouse move handler queries elevation values from COG tiles with 200ms throttling and proper guards for zoom level. The COG protocol should cache tile data, making repeated queries efficient. Error handling is acceptable for hover interactions.
552-623: Rendering strategy appropriately switches between WMS and COG based on zoom.The implementation correctly uses WMS tiles for overview (zoom < 13) and switches to detailed COG tiles with hillshade at higher zoom levels. Layer ordering with
beforeIdensures fields render above elevation data. The use ofFragmentwithtile.idas key is appropriate for dynamic tile lists.
@SvenVw yes, seems to appear much more performant now. Great! |
3 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Closes #361