Add Atlas field details flow, route, UI cards, and soil/GW/reg data#233
Conversation
… details page when clicked
… that field, like cultivation history and soil texture are shown on fields details page
…Gebie and isFieldInNatura2000Gebied
… farm and atlas. When first logged in show 2 detailed cards with the background information about the farm and atlas tool
🦋 Changeset detectedLatest commit: 5019e38 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 4 minutes and 9 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 centroid-click navigation from Atlas fields to a new field details route, implements loader aggregating NMI, catalogue and calculator data, renders cultivation/soil/groundwater/field-detail cards with skeletons, threads new boolean cultivation flag b_lu_rest_oravib through core/data/tests, and re-exports several calculator helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as Atlas Map
participant R as Router
participant L as FieldDetails Loader
participant Q as getFieldByCentroid
participant N as NMI API
participant C as Calculator
participant D as Data Services
U->>M: Click field feature
M->>R: Navigate to /farm/:id/:calendar/atlas/fields/:centroid
R->>L: loader(params)
L->>Q: getFieldByCentroid(lon,lat,calendar)
alt field found
L->>N: fetch soil parameter estimates
L->>D: fetch cultivation catalogue
L->>C: compute region / NV / GWBG / Natura2000
L-->>R: return aggregated data (cultivationHistory, fieldDetails, soil, groundwater, calendar)
else none
L-->>R: return null / error
end
R-->>U: render Field Details page (cards & skeletons)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #233 +/- ##
===============================================
- Coverage 92.88% 92.86% -0.02%
===============================================
Files 85 85
Lines 12885 12889 +4
Branches 1294 1294
===============================================
+ Hits 11968 11970 +2
- Misses 915 917 +2
Partials 2 2
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:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
coderabbitai
Bot
added
branch:main
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/email/welcome.tsx (1)
66-69: Intro copy update: clear and on-point. Minor deliverability note.Copy looks good. Consider ensuring the logo URL is absolute for email clients; many won’t load relative paths. You can derive it from appBaseUrl or fall back to the main link URL.
Example (outside this range) to make
logoPathrobust:const base = appBaseUrl || url const logoPath = logoFileName.startsWith("http") ? logoFileName : new URL(logoFileName, base).toString()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
fdm-app/app/lib/email.server.tsis excluded by!fdm-app/app/lib/**
📒 Files selected for processing (1)
fdm-app/app/components/blocks/email/welcome.tsx(1 hunks)
🔇 Additional comments (1)
fdm-app/app/components/blocks/email/welcome.tsx (1)
124-125: Closing copy: LGTM.Minor wording tweak is fine and reads naturally.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
fdm-app/app/lib/email.server.tsis excluded by!fdm-app/app/lib/**
📒 Files selected for processing (1)
fdm-app/app/components/blocks/email/welcome.tsx(1 hunks)
🔇 Additional comments (5)
fdm-app/app/components/blocks/email/welcome.tsx (5)
66-69: Intro copy LGTMConcise and clear; good setup for the guided steps.
71-71: Structural divider improves scannabilityGood addition for separating intro from the action steps.
118-118: CTA label LGTMClear and action-oriented.
124-124: Contact phrasing LGTMFriendly and straightforward.
32-32: Ensure absolute logo URL for email clientsWe searched the repo for
<WelcomeEmail(...)>and didn’t find any direct usages, so it’s unclear whether callers always supply an absoluteurlor a non-emptyappBaseUrl. To guarantee the logo is rendered reliably across all email clients, update yourlogoPathlogic as follows:- const logoPath = `${appBaseUrl}${logoFileName}` + // Ensure absolute URL for email clients + const origin = (() => { + try { + // Prefer explicit base URL; fall back to the URL prop’s origin + return appBaseUrl || new URL(url).origin + } catch { + // If neither is available, origin will be an empty string + return appBaseUrl || "" + } + })() + const logoPath = logoFileName.startsWith("http") + ? logoFileName + : `${origin}${logoFileName.startsWith("/") ? "" : "/"}${logoFileName}`Next steps:
- Confirm that every caller of
WelcomeEmailpasses an absoluteurl(e.g.https://…) or provides a non-emptyappBaseUrl.- If you need stricter guarantees, throw an error when both
appBaseUrland the fallback origin are empty.
| className="bg-primary rounded text-primary-foreground text-[14px] font-semibold no-underline px-5 py-3" | ||
| href={url} | ||
| > | ||
| Start met {appName} | ||
| Naar de website |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Email-client safe CTA styling: avoid custom Tailwind tokens
Using bg-primary and text-primary-foreground relies on Tailwind config/CSS variables that most email clients won’t resolve. Prefer default Tailwind colors (react-email will inline) or explicit inline styles.
Apply this diff:
- <Button
- // Use Tailwind padding classes instead of pX/pY props
- className="bg-primary rounded text-primary-foreground text-[14px] font-semibold no-underline px-5 py-3"
- href={url}
- >
+ <Button
+ // Use safe, email-friendly classes (react-email will inline these to CSS)
+ className="bg-blue-600 rounded text-white text-[14px] font-semibold no-underline px-5 py-3"
+ href={url}
+ >Alternatively, set explicit inline styles (strongest compatibility):
- Keep className for spacing/typography.
- Add style={{ backgroundColor: "#2563eb", color: "#ffffff" }}.
Also ensure href is an absolute URL (relative links won’t work in emails).
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' fdm-app/app/components/blocks/email/welcome.tsxLength of output: 6323
Update email CTA styling to use default Tailwind colors
The current bg-primary and text-primary-foreground tokens rely on CSS variables that many email clients won’t resolve. Please replace them with default Tailwind classes (which React Email inlines) and add an explicit inline‐style fallback. Also ensure the href is an absolute URL.
• fdm-app/app/components/blocks/email/welcome.tsx (lines 115–118):
- Replace the custom color tokens in
className - Add
styleprop for strongest compatibility - Confirm
urlis a fully qualified URL before rendering
- <Button
- // Use Tailwind padding classes instead of pX/pY props
- className="bg-primary rounded text-primary-foreground text-[14px] font-semibold no-underline px-5 py-3"
- href={url}
- >
+ <Button
+ // Use safe, email-friendly classes and inline colors
+ className="bg-blue-600 rounded text-white text-[14px] font-semibold no-underline px-5 py-3"
+ style={{ backgroundColor: "#2563eb", color: "#ffffff" }}
+ href={url /* ensure this is an absolute URL */}
+ >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className="bg-primary rounded text-primary-foreground text-[14px] font-semibold no-underline px-5 py-3" | |
| href={url} | |
| > | |
| Start met {appName} | |
| Naar de website | |
| <Button | |
| // Use safe, email-friendly classes and inline colors | |
| className="bg-blue-600 rounded text-white text-[14px] font-semibold no-underline px-5 py-3" | |
| style={{ backgroundColor: "#2563eb", color: "#ffffff" }} | |
| href={url /* ensure this is an absolute URL */} | |
| > | |
| Naar de website | |
| </Button> |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
fdm-app/app/components/blocks/email/welcome.tsx (3)
73-95: Fix typo and use semantic lists instead of manual bullets for accessibility
- Typo: “Stikstofbaland” → “Stikstofbalans”.
- Replace manual bullets with
/
- for better semantics, screen readers, and more reliable spacing across clients.
Apply this diff for the bullet block (fixes typo and switches to a list):
- <Text className="text-black text-[14px] leading-[24px] ml-4"> - • Stikstofbaland, bemestingsadvies en - gebruiksruimte apps. <br />• Beheer van meerdere - jaren voor een compleet overzicht. <br />• Een - uitgebreide lijst van meststoffen die je zelf - kunt aanpassen. <br />• De mogelijkheid om je - adviseur toegang te geven. - </Text> + <ul className="text-black text-[14px] leading-[24px] ml-4 pl-4"> + <li>Apps voor stikstofbalans, bemestingsadvies en gebruiksruimte.</li> + <li>Beheer van meerdere jaren voor een compleet overzicht.</li> + <li>Een uitgebreide lijst van meststoffen die je zelf kunt aanpassen.</li> + <li>De mogelijkheid om je adviseur toegang te geven.</li> + </ul>Optional: if there’s a direct “bedrijf aanmaken” route, consider adding a secondary CTA/link here to increase activation.
96-110: Improve copy (“tot 2009” → “sinds 2009”) and use semantic list markup
- “De volledige gewashistorie van percelen tot 2009.” likely intends “sinds 2009” (history available starting 2009).
- “valt” → “ligt” reads more natural in NL here.
- Switch to
/
- for email/readability.
Apply this diff for the list:
- <Text className="text-black text-[14px] leading-[24px] ml-4"> - • De volledige gewashistorie van percelen tot - 2009. <br />• Of een perceel in een gebied met - beperkingen voor de gebruiksruimte valt. <br />• - Een inschatting van de bodemtextuur en het - grondwaterpeil. - </Text> + <ul className="text-black text-[14px] leading-[24px] ml-4 pl-4"> + <li>De volledige gewashistorie van percelen sinds 2009.</li> + <li>Of een perceel in een gebied met beperkingen voor de gebruiksruimte ligt.</li> + <li>Een inschatting van de bodemtextuur en het grondwaterpeil.</li> + </ul>
118-118: Ensure email-safe button colors and absolute URLThe CTA label change is fine. Two checks:
- Verify that Tailwind tokens (bg-primary, text-primary-foreground) inline to concrete hex colors (not CSS variables). Many email clients don’t support CSS variables.
- Ensure href is an absolute URL.
For maximum compatibility, prefer hex colors or inline style. Example:
<Button className="rounded text-[14px] font-semibold no-underline px-5 py-3" style={{ backgroundColor: "#0ea5e9", color: "#ffffff" }} href={url} // absolute URL like https://app.example.com/... > Naar de website </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
fdm-app/app/lib/email.server.tsis excluded by!fdm-app/app/lib/**
📒 Files selected for processing (1)
fdm-app/app/components/blocks/email/welcome.tsx(1 hunks)
🔇 Additional comments (3)
fdm-app/app/components/blocks/email/welcome.tsx (3)
66-70: Copy update looks goodClear, concise intro to the feature overview. No issues.
71-71: Good use of a separator between intro and actionsThe
improves scannability in email clients. LGTM.
124-125: Nice touch inviting repliesEncouraging “reply-to” drives engagement and support reachability. LGTM.
BoraIneviNMI
left a comment
BoraIneviNMI
left a comment
There was a problem hiding this comment.
Everything looks good. I tested out the map position storage, field details screen, and ensured map creation screens works.
About the grid layout used in the field details screen, you could also consider using two different grid templates (the method where you give names to different regions of the grid) if the responsiveness is still problematic somehow. I don't think it is problematic but just as a suggestion, maybe for next time.
Thanks for suggestion, but is has been resolved with the current code, so I think it is fine to keep it this way |
3 participants
Summary by CodeRabbit
New Features
UI/UX
Data
Chores
Closes #220