Expand and improve the landing page#370
Conversation
…xplaining what fdm-app can do and how it can be used. Also improve the mobile responsiveness
🦋 Changeset detectedLatest commit: 6098079 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 |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⛔ Files ignored due to path filters (4)
You can disable this status message by setting the WalkthroughRedesigns landing/sign‑in pages into a multi‑section, Card‑based marketing layout with social/email sign‑in, expanded informational and FAQ sections, improved mobile responsiveness, a scrollToMoreInfo helper, a new links export, and a changeset; also adjusts cookie banner button layout to side‑by‑side. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development #370 +/- ##
============================================
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:
|
![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
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/signin._index.tsx (1)
170-176: Critical: Undefined functiondataWithError.The
handleSignInErrorfunction callsdataWithErroron line 171, but this function is neither imported nor defined in this file. This will cause a runtime exception whenever a social sign-in error occurs.You likely need to either:
- Import a toast/notification utility (e.g.,
redirectWithErrorfrom remix-toast), or- Use React state to display the error message in the UI
Apply this diff if you want to use remix-toast for error notifications:
import { useRemixForm } from "remix-hook-form" -import { redirectWithSuccess } from "remix-toast" +import { redirectWithSuccess, redirectWithError } from "remix-toast" import { z } from "zod"Then update the error handler:
const handleSignInError = (provider: string, error: unknown) => { - dataWithError( - null, + redirectWithError( + "/signin", `Er is helaas iets misgegaan bij het aanmelden met ${provider}. Probeer het opnieuw.`, ) console.error("Social sign-in failed:", error) }Alternatively, if you want to display errors in the UI without a redirect, consider adding error state management to the component.
🧹 Nitpick comments (1)
fdm-app/app/routes/signin._index.tsx (1)
466-466: Optional: Use self-closing syntax for empty element.The empty
<span>element on line 466 (used as a decorative dot) can use self-closing syntax for consistency with React/JSX conventions.Apply this diff:
- <span className="mr-2 flex h-2 w-2 rounded-full bg-green-500"></span> + <span className="mr-2 flex h-2 w-2 rounded-full bg-green-500" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/all-olives-march.md(1 hunks)fdm-app/app/components/custom/banner.tsx(1 hunks)fdm-app/app/routes/signin._index.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 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:
.changeset/all-olives-march.md
📚 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:
.changeset/all-olives-march.md
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/routes/signin._index.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/signin._index.tsx
📚 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/signin._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/components/custom/banner.tsx
🪛 Biome (2.1.2)
fdm-app/app/routes/signin._index.tsx
[error] 17-17: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 452-452: Avoid the words "image", "picture", or "photo" in img element alt text.
Screen readers announce img elements as "images", so it is not necessary to redeclare this in alternative text.
(lint/a11y/noRedundantAlt)
[error] 466-466: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Safe fix: Use a self-closing element instead.
(lint/style/useSelfClosingElements)
🔇 Additional comments (5)
fdm-app/app/components/custom/banner.tsx (1)
186-192: LGTM! Improved button layout.The side-by-side layout for cookie consent buttons (
w-1/2) provides a cleaner, more modern appearance compared to the stacked layout. This change aligns well with the broader UI refresh described in the PR..changeset/all-olives-march.md (1)
1-5: LGTM! Changeset properly documents the release.The changeset correctly captures the scope of changes: landing page expansion with new informational sections and improved mobile responsiveness. The minor version bump is appropriate for these user-facing enhancements.
fdm-app/app/routes/signin._index.tsx (3)
151-151: LGTM! Smooth scroll implementation.The ref and scroll function provide a nice UX feature to navigate users from the sign-in form to the informational content below. The implementation is clean and uses the native
scrollIntoViewAPI appropriately.Also applies to: 186-188
204-459: LGTM! Comprehensive sign-in UI redesign.The new two-column layout with social login options, email sign-in, and clear CTAs significantly improves the user experience. The implementation properly handles:
- Async social authentication with error handling
- Form state management with React Hook Form
- Accessible links with appropriate
relattributes- Responsive design with mobile considerations
460-1324: LGTM! Comprehensive landing page content.The new informational sections successfully address the PR objective of explaining what fdm-app can do. The content is well-organized into logical sections covering:
- Innovation and partnerships
- Target steering (Doelsturing) methodology
- Fertilization advice features
- Usage space calculations
- Atlas mapping capabilities
- Collaboration features
- FAQs
The consistent use of Card components, proper semantic structure, and clear visual hierarchy create a professional, informative landing page.
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/signin._index.tsx (1)
72-79: Meta description still reads like a pure sign-in page (SEO mismatch with new “landing” content).
Consider updatingcontent:to reflect the broader product/landing messaging now shown on this route.
🧹 Nitpick comments (1)
fdm-app/app/routes/signin._index.tsx (1)
184-186: Smooth-scroll should respect “prefers-reduced-motion”.
Minor a11y improvement: usebehavior: "auto"when users prefer reduced motion.const scrollToMoreInfo = () => { - moreInfoRef.current?.scrollIntoView({ behavior: "smooth" }) + const reduce = window.matchMedia?.("(prefers-reduced-motion: reduce)")?.matches + moreInfoRef.current?.scrollIntoView({ behavior: reduce ? "auto" : "smooth" }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/routes/signin._index.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/routes/signin._index.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/signin._index.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/routes/signin._index.tsx
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: core (24)
🔇 Additional comments (1)
fdm-app/app/routes/signin._index.tsx (1)
449-456: Hero image alt text looks good now.
Concise and descriptive; also avoids embedding attribution inalt.
coderabbitai
Bot
added
branch:development
There was a problem hiding this comment.
Actionable comments posted: 3
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/signin._index.tsx (1)
184-190: Client-sidedataWithError()won't display toast notifications.
dataWithErroris designed for loader/action responses where it sets toast data via response headers. When called in a client-side click handler, the return value is ignored and users won't see any error feedback when social sign-in fails.Use React state to display the error message:
+import { useEffect, useRef, useState } from "react" -import { useEffect, useRef } from "react" ... -import { dataWithError, redirectWithSuccess } from "remix-toast" +import { redirectWithSuccess } from "remix-toast" ... export default function SignIn() { const [searchParams, setSearchParams] = useSearchParams() const moreInfoRef = useRef<HTMLDivElement>(null) + const [socialSignInError, setSocialSignInError] = useState<string | null>(null) ... const handleSignInError = (provider: string, error: unknown) => { - dataWithError( - null, + setSocialSignInError( `Er is helaas iets misgegaan bij het aanmelden met ${provider}. Probeer het opnieuw.`, ) console.error("Social sign-in failed:", error) }Then render the error near the sign-in buttons:
{socialSignInError && ( <p role="alert" className="text-sm text-destructive text-center"> {socialSignInError} </p> )}
♻️ Duplicate comments (1)
fdm-app/app/routes/signin._index.tsx (1)
1305-1350: Replace<NavLink>with<a>for external URLs, and<Link to="#">with<button>.React Router's
NavLinkandLinkcomponents are designed for internal app navigation. For external URLs, use standard<a>elements. For the cookie settings trigger, use a<button>since it's an action, not navigation.This was flagged in a previous review. Apply these changes:
<li> - <NavLink - to="https://www.nmi-agro.nl" + <a + href="https://www.nmi-agro.nl" target="_blank" rel="noopener noreferrer" className="hover:text-primary flex items-center gap-2" > Over NMI <ExternalLink className="h-3 w-3" /> - </NavLink> + </a> </li> <li> - <NavLink - to="https://github.com/SvenVw/fdm" + <a + href="https://github.com/SvenVw/fdm" target="_blank" rel="noopener noreferrer" className="hover:text-primary flex items-center gap-2" > GitHub Repository <Github className="h-3 w-3" /> - </NavLink> + </a> </li> <li> - <NavLink - to={clientConfig.privacy_url} + <a + href={clientConfig.privacy_url} target="_blank" rel="noopener noreferrer" className="hover:text-primary" > Privacybeleid - </NavLink> + </a> </li> <li> - <Link - to="#" + <button + type="button" onClick={onOpenCookieSettings} className="hover:text-primary text-left" > Cookie instellingen - </Link> + </button> </li>
🧹 Nitpick comments (2)
fdm-app/app/routes/welcome.tsx (2)
16-23: Consider reorganizing the import for consistency.The
CardDescriptionimport is placed afterCardTitlewhich breaks the alphabetical ordering of the Card component imports.import { Card, CardContent, + CardDescription, CardFooter, CardHeader, CardTitle, - CardDescription, } from "~/components/ui/card"
235-235: Consider removing empty CardFooter.The
CardFooteris empty and only provides spacing. If spacing is needed, consider using padding on the Card or CardContent instead.</CardContent> - <CardFooter className="flex justify-center" /> </Card>
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
fdm-app/app/routes/signin._index.tsx(6 hunks)fdm-app/app/routes/signin.check-your-email.tsx(1 hunks)fdm-app/app/routes/signin.invalid_token.tsx(1 hunks)fdm-app/app/routes/welcome.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (16)
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/signin.invalid_token.tsxfdm-app/app/routes/welcome.tsxfdm-app/app/routes/signin._index.tsxfdm-app/app/routes/signin.check-your-email.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/signin.invalid_token.tsxfdm-app/app/routes/welcome.tsxfdm-app/app/routes/signin._index.tsxfdm-app/app/routes/signin.check-your-email.tsx
📚 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/app/routes/welcome.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/welcome.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/routes/welcome.tsxfdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/routes/welcome.tsxfdm-app/app/routes/signin._index.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-02-24T10:49:54.523Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 84
File: fdm-app/app/root.tsx:89-145
Timestamp: 2025-02-24T10:49:54.523Z
Learning: In the ErrorBoundary component of fdm-app/app/root.tsx, all client errors (400, 401, 403, 404) are intentionally displayed with a 404 status code for security purposes.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-01-14T16:06:24.294Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 45
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:1-1
Timestamp: 2025-01-14T16:06:24.294Z
Learning: In the fdm-app codebase, the `redirect` function should be imported from `react-router`, not `react-router-dom`.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-01-14T16:06:21.832Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 45
File: fdm-app/app/routes/farm.$b_id_farm.settings._index.tsx:1-1
Timestamp: 2025-01-14T16:06:21.832Z
Learning: In the fdm project, `redirect` and other routing utilities should be imported from `react-router` instead of `react-router-dom`.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-01-09T16:07:36.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes.ts:81-81
Timestamp: 2025-01-09T16:07:36.741Z
Learning: The route pattern "api/auth/:" in React Router configuration is required by the better-auth package and should not be modified to a standard dynamic parameter format, as it would break the authentication functionality.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-01-09T16:07:36.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes.ts:81-81
Timestamp: 2025-01-09T16:07:36.741Z
Learning: The route pattern "api/auth/:" in React Router configuration is a specific requirement of the better-auth package (version ^1.1.10). It serves as a catch-all route for authentication endpoints and should not be modified to a standard dynamic parameter format, as it would break the authentication functionality.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/routes/signin._index.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/routes/signin._index.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/signin._index.tsx
🪛 Biome (2.1.2)
fdm-app/app/routes/signin.invalid_token.tsx
[error] 123-124: Avoid the words "image", "picture", or "photo" in img element alt text.
Screen readers announce img elements as "images", so it is not necessary to redeclare this in alternative text.
(lint/a11y/noRedundantAlt)
fdm-app/app/routes/signin.check-your-email.tsx
[error] 127-128: Avoid the words "image", "picture", or "photo" in img element alt text.
Screen readers announce img elements as "images", so it is not necessary to redeclare this in alternative text.
(lint/a11y/noRedundantAlt)
🔇 Additional comments (8)
fdm-app/app/routes/signin.invalid_token.tsx (1)
77-117: LGTM!The Card-based layout implementation is clean and consistent with the design system. The cookie settings handler safely checks for
window?.openCookieSettingsbefore invoking.fdm-app/app/routes/signin.check-your-email.tsx (1)
78-121: LGTM!The Card-based layout is well-structured and consistent with the design system. Good use of
CardDescriptionfor the subtitle text.fdm-app/app/routes/welcome.tsx (2)
149-236: LGTM!The form implementation is well-structured with proper validation, loading states, and accessibility attributes (
aria-required). The conditional avatar rendering and fieldset disabling during submission provide good UX.
264-294: LGTM!The action properly validates the redirect URL to prevent open redirect vulnerabilities and handles errors appropriately without leaking sensitive information.
fdm-app/app/routes/signin._index.tsx (4)
73-96: LGTM!Comprehensive SEO metadata with Open Graph and Twitter card tags. The canonical link properly references
clientConfig.url.
360-432: LGTM!The email sign-in form is well-implemented with proper validation, hidden timezone field for UX, and loading state feedback.
1363-1409: LGTM!The action properly validates the redirect URL, handles timezone validation gracefully, and follows security best practices by not logging sensitive error details.
463-473: LGTM!The alt text has been properly updated to be descriptive ("A tractor plowing a field at sunset") with photo credits moved to a code comment.
| <p> | ||
| {clientConfig.name} is een initiatief | ||
| van het Nutriënten Management Instituut | ||
| (NMI). Het platform is volop in |
There was a problem hiding this comment.
delete "het platform is volop in ontwikkeling, maar nu al inzetbaar voor uw bedrijf". This should be clear from the app itself. note that you start "is een initiatief"en dan doorgat met "het platform". I would delete the sentence.
There was a problem hiding this comment.
Dus dan {clientConfig.name} is een initiatief van het Nutriënten Management Instituut (NMI) en is volop in ontwikkeling, maar nu al inzetbaar voor uw bedrijf?
| Doelsturing: | ||
| </strong>{" "} | ||
| In samenwerking met LTO Noord, | ||
| ZLTO, LVVN, NVWA en RVO testen |
There was a problem hiding this comment.
samen met boeren, agrarische sector, kennisinstellingen, overheden en overheidsorganisaties?
There was a problem hiding this comment.
Ja kan, maar is minder specifiek
| <span> | ||
| <strong className="text-foreground"> | ||
| PPS BAAT: | ||
| </strong>{" "} |
There was a problem hiding this comment.
PPS bevat meer partijen toch?
There was a problem hiding this comment.
Ja, 19 stuks, maar in WP4 zitten alleen NMI en WUR als uitvoerders
| brengen, faciliteren we kennisdeling en | ||
| versnellen we de transitie naar een | ||
| duurzamere landbouw: | ||
| </p> |
There was a problem hiding this comment.
would it be valuable to refer already to BBWP?
There was a problem hiding this comment.
Op dit moment zit er nog geen functionaliteit van de BBWP erin. Later natuurlijk wel, maar daar heb je nog niks aan als je het nu leest
| de waterkwaliteit te verbeteren en | ||
| ammoniakemissies te verminderen, doordat u als | ||
| ondernemer zelf de maatregelen kiest. De | ||
| keerzijde is dat dit vraagt om veel en |
There was a problem hiding this comment.
Maatwerk maakt het mogelijk om landbouw en milieu te combineren op uw bedrijf.
There was a problem hiding this comment.
Hoe wil je dit toevoegen? Tussen kiest. De ?
| keerzijde is dat dit vraagt om veel en | ||
| gedetailleerde data. {clientConfig.name} streeft | ||
| ernaar om deze complexe berekening zo eenvoudig | ||
| mogelijk te maken, zonder in te leveren op |
There was a problem hiding this comment.
streeft ernaar om helder en concreet voor landbouwers in beeld te brengen wat de opgaven zijn en hoe zij kunnen sturen op een duurzame en toekomstbestendig landbouwsysteem? (als vervanging van "deze complexe berekening"?
There was a problem hiding this comment.
Ok, maar het is dan onduidelijk waarom je deze tool zou gebruiken en waarin het je helpt. Immers de website van de overheid vertelt je ook wat de opgave is, maar deze tool maakrt het ook echt concreet voor je bedrijf op een zo toeganglijke mogelijke manier
| <li>• Oogst </li> | ||
| <li>• Gewasresten </li> | ||
| <li>• Ammoniak (Vervluchtiging)</li> | ||
| </ul> |
There was a problem hiding this comment.
remove "vervluchtiging". voeg toe nitraat
| </h3> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Het resultaat van Aanvoer min Afvoer en | ||
| Ammoniak. |
There was a problem hiding this comment.
het stikstofbodemoverschot laat zien hoe efficient stikstof wordt gebruikt.
| <p className="text-lg leading-relaxed text-muted-foreground"> | ||
| Bij NMI hebben we jarenlange kennis van | ||
| bemesting en bodemvruchtbaarheid omgezet in | ||
| digitale adviezen, direct beschikbaar in{" "} |
There was a problem hiding this comment.
digitalisering is verwarrend (pdf is ook digitalisering). herschrijven als "maak concreet inzichtelijk per perceel en per gewas..."
There was a problem hiding this comment.
'Concreet inzichtelijk' kan ook een boek zijn. Kun je dat wat duidelijker maken?
| <CardContent> | ||
| <p className="text-sm text-muted-foreground leading-relaxed"> | ||
| De interactieve kaart toont | ||
| gewaspercelen tot 2020, helder |
gerardhros
left a comment
gerardhros
left a comment
There was a problem hiding this comment.
i added some tips for textual improvements. overall setup is fine.
maybe you can move up Atlas (since this might be interesting for users, more than the technical items like gebruiksruimte
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fdm-app/app/routes/signin.check-your-email.tsx (1)
69-76: Consider removing the redundant wrapper function.
onOpenCookieSettingsis a simple passthrough toopenCookieSettingswith no additional logic. You can passopenCookieSettingsdirectly to theonClickhandler.🔎 Suggested simplification
// Function to open cookie settings if available in the window object const openCookieSettings = () => { if (window?.openCookieSettings) { window.openCookieSettings() } } - const onOpenCookieSettings = () => { - openCookieSettings() - }Then update line 140:
- onClick={onOpenCookieSettings} + onClick={openCookieSettings}
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
fdm-app/app/routes/signin.check-your-email.tsx(1 hunks)fdm-app/app/routes/signin.invalid_token.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-app/app/routes/signin.invalid_token.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/signin.check-your-email.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/signin.check-your-email.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/signin.check-your-email.tsx
🔇 Additional comments (3)
fdm-app/app/routes/signin.check-your-email.tsx (3)
1-24: LGTM!Imports and meta function are well-structured. The Card component imports align with the layout refactor, and the meta function provides appropriate Dutch-language SEO metadata.
78-121: LGTM!The Card-based layout is well-structured with proper semantic hierarchy. The responsive grid, centered content, and component composition (CardHeader, CardContent, CardFooter) follow good patterns. The NavLink integration with
asChildis correct.
124-134: Alt text issue has been resolved.The previous review concern about HTML in the alt attribute has been addressed. The alt text is now a proper descriptive string: "A river running through a green landscape". The photographer attribution is preserved in an HTML comment for reference.
2 participants
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.
Closes #369