SANC-31: Implement trip survey frontend modal #55
Conversation
JustinTan-1
requested review from
Lujarios,
Yemyam,
burtonjong,
jason-duong4509,
promatty,
tanvimahal,
themaxboucher and
wesleylui
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
JustinTan-1
changed the title
JustinTan-1
changed the title
📝 WalkthroughWalkthroughAdds a new TripSurveyModal component with styles and five rating SVG icons, integrates it into the survey notification flow, and makes the common modal responsive by enabling fullscreen on viewports <=50em via a media query. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
src/app/_components/common/modal/modal.tsx (1)
1-61:⚠️ Potential issue | 🟠 MajorAdd
"use client"directive for hook usage.The component uses
useMediaQueryfrom@mantine/hooksat line 43. In Next.js app directory, all components using React hooks must be marked as client components. All comparable components in this directory (Button, calendar-view, DatePicker) include the"use client"directive—this file needs it as well.Suggested fix
+"use client"; import { Group, Modal as MantineModal, type ModalBaseOverlayProps } from "@mantine/core"; import { useMediaQuery } from "@mantine/hooks";
🤖 Fix all issues with AI agents
In `@src/app/_components/drivercomponents/TripSurveyModal.tsx`:
- Around line 83-88: The handleConfirm function currently logs PII via
console.log(form.getValues()) after validation; remove that production logging
and either delete the console.log call or wrap it behind a development-only
guard (e.g., check process.env.NODE_ENV === 'development') so only
non-production builds emit the form values. Update the handleConfirm
implementation where form.validate(), form.getValues(), and console.log are
referenced to avoid exposing addresses/notes in production.
- Around line 104-143: In TripSurveyModal, two Mantine <Text> components use an
invalid size token `size="l"` (look for the headings "Drive Details" and "Fit or
Not fit"); update both to a valid token `size="lg"` so the component uses
supported Mantine sizes and avoids runtime/visual issues.
- Around line 32-38: The endReading validator in TripSurveyModal.tsx must be
converted to Mantine's cross-field signature to compare against
values.startReading: update the endReading function to accept (value, values),
keep the existing empty/number checks, and add a check that Number(value) >=
Number(values.startReading) returning an appropriate error message (e.g., "End
reading must be greater than or equal to start reading") when it fails;
reference the existing startReading and endReading validator functions to
implement this comparison and preserve the integer validation logic.
🧹 Nitpick comments (2)
src/assets/icons/rating4.tsx (1)
15-28: Remove the duplicate circle path to avoid redundant rendering.♻️ Proposed cleanup
<path stroke="#424242" strokeLinecap="round" strokeLinejoin="round" strokeWidth={3} d="M16 29.333c7.364 0 13.333-5.97 13.333-13.333 0-7.364-5.97-13.333-13.333-13.333C8.636 2.667 2.667 8.637 2.667 16c0 7.364 5.97 13.333 13.333 13.333Z" /> - <path - stroke="#424242" - strokeLinecap="round" - strokeLinejoin="round" - strokeWidth={3} - d="M16 29.333c7.364 0 13.333-5.97 13.333-13.333 0-7.364-5.97-13.333-13.333-13.333C8.636 2.667 2.667 8.637 2.667 16c0 7.364 5.97 13.333 13.333 13.333Z" - />src/app/_components/drivercomponents/TripSurveyModal.tsx (1)
117-139: Add native input type affordances for improved validation and mobile UX.Mantine's
TextInputforwards all native input element props, enablingtype="time",inputMode, andpatternattributes. Using these improves mobile keyboard selection and leverages browser-native validation.💡 Input type improvements
<TextInput label="Time of Departure" placeholder="Enter departure time" + type="time" {...form.getInputProps("timeOfDeparture")} required /> <TextInput label="Time of Arrival" placeholder="Enter arrival time" + type="time" {...form.getInputProps("timeOfArrival")} required /> <TextInput label="Odometer start" placeholder="Enter odometer starting value" + inputMode="numeric" + pattern="[0-9]*" {...form.getInputProps("startReading")} required /> <TextInput label="Odometer end" placeholder="Enter odometer ending value" + inputMode="numeric" + pattern="[0-9]*" {...form.getInputProps("endReading")} required />
| startReading: (value) => | ||
| value.length < 1 || !Number.isInteger(Number(value)) | ||
| ? "Please enter a valid numerical reading" | ||
| : null, | ||
| endReading: (value) => | ||
| value.length < 1 || !Number.isInteger(Number(value)) | ||
| ? "Please enter a valid numerical reading" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "TripSurveyModal.tsx"Repository: Code-the-Change-YYC/salvationarmy
Length of output: 133
🏁 Script executed:
cat -n src/app/_components/drivercomponents/TripSurveyModal.tsx | head -50Repository: Code-the-Change-YYC/salvationarmy
Length of output: 2259
🌐 Web query:
Mantine useForm validate cross-field validation (value, values) signature
💡 Result:
In Mantine useForm (rules-object validation), each field validator is called with:
(value, values, path) => ReactNode | nullvalue: current field valuevalues: all form values (use this for cross-field validation)path: field path (e.g."user.email","cart.0.price") [1]
Example (cross-field):
validate: {
confirmPassword: (value, values) =>
value !== values.password ? 'Passwords did not match' : null,
}
``` [1]
Sources: Mantine “Form validation” docs (current) [1] and same section in v5 docs [2].
---
</details>
**Add cross-field validation to ensure end reading ≥ start reading.**
Users can currently submit an end reading lower than the start reading. The `endReading` validator needs to compare against `values.startReading` using Mantine's cross-field validation signature.
<details>
<summary>Suggested validation update</summary>
```diff
endReading: (value) =>
value.length < 1 || !Number.isInteger(Number(value))
? "Please enter a valid numerical reading"
- : null,
+ : null,
+ endReading: (value, values) => {
+ if (value.length < 1 || !Number.isInteger(Number(value))) {
+ return "Please enter a valid numerical reading";
+ }
+ if (Number(value) < Number(values.startReading)) {
+ return "End reading must be greater than or equal to start reading";
+ }
+ return null;
+ },🤖 Prompt for AI Agents
In `@src/app/_components/drivercomponents/TripSurveyModal.tsx` around lines 32 -
38, The endReading validator in TripSurveyModal.tsx must be converted to
Mantine's cross-field signature to compare against values.startReading: update
the endReading function to accept (value, values), keep the existing
empty/number checks, and add a check that Number(value) >=
Number(values.startReading) returning an appropriate error message (e.g., "End
reading must be greater than or equal to start reading") when it fails;
reference the existing startReading and endReading validator functions to
implement this comparison and preserve the integer validation logic.
…Trip-Survey-Modal-Component-Frontend-Only
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/app/_components/drivercomponents/TripSurveyModal.module.scss`:
- Around line 10-41: .formRow currently uses grid-specific properties (gap,
grid-column on :global(.mantine-InputWrapper-*)) but never sets display: grid
and is wrapped around a Radio.Group in TripSurveyModal.tsx which doesn't render
.mantine-InputWrapper-* children; either remove the .formRow wrapper from the
Radio.Group in TripSurveyModal.tsx, or make .formRow a real grid by adding
display: grid and grid-template-columns: auto 1fr and only apply it to
components that render Mantine InputWrapper children (e.g., TextInput, Textarea)
so the :global(.mantine-InputWrapper-*) selectors take effect.
🧹 Nitpick comments (1)
src/app/_components/drivercomponents/TripSurveyModal.module.scss (1)
6-8: Negative margin is fragile — consider a comment explaining why.
margin-top: -15pxis a magic number that could break if the rating component's spacing changes. A brief inline comment noting what it compensates for would help future maintainers.
| .formRow { | ||
| gap: 0 1rem; | ||
| align-items: start; | ||
|
|
||
| // Override Mantine's default input styles | ||
| :global(.mantine-InputWrapper-root) { | ||
| display: contents; | ||
| } | ||
|
|
||
| :global(.mantine-InputWrapper-label) { | ||
| grid-column: 1; | ||
| margin-bottom: 0 !important; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: flex-start; | ||
| } | ||
|
|
||
| :global(.mantine-InputWrapper-input) { | ||
| grid-column: 2; | ||
| } | ||
|
|
||
| :global(.mantine-InputWrapper-error) { | ||
| grid-column: 2; | ||
| margin-top: 0.25rem; | ||
| margin-bottom: 0; | ||
| } | ||
|
|
||
| // Increase textarea height | ||
| :global(.mantine-Textarea-input) { | ||
| min-height: 100px; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the TripSurveyModal component to see how .formRow is used
fd "TripSurveyModal" --type fRepository: Code-the-Change-YYC/salvationarmy
Length of output: 198
🏁 Script executed:
# Search for .formRow usage in TSX/JSX files
rg "formRow" --type-add 'tsx:*.tsx' --type tsx -B 5 -A 5Repository: Code-the-Change-YYC/salvationarmy
Length of output: 22179
🏁 Script executed:
# Check if Mantine is installed and locate its InputWrapper classes
fd "node_modules/mantine" --type d | head -1Repository: Code-the-Change-YYC/salvationarmy
Length of output: 59
🏁 Script executed:
cat src/app/_components/drivercomponents/TripSurveyModal.tsxRepository: Code-the-Change-YYC/salvationarmy
Length of output: 7588
🏁 Script executed:
# Check if node_modules exists and search for Mantine InputWrapper
find . -path "*/node_modules/@mantine/*" -name "*.css" -o -name "*.module.css" 2>/dev/null | head -20Repository: Code-the-Change-YYC/salvationarmy
Length of output: 103
🏁 Script executed:
# Search for `@mantine` packages to understand structure
ls -la node_modules/@mantine 2>/dev/null | head -20Repository: Code-the-Change-YYC/salvationarmy
Length of output: 59
🏁 Script executed:
cat src/app/_components/drivercomponents/TripSurveyModal.module.scssRepository: Code-the-Change-YYC/salvationarmy
Length of output: 934
🏁 Script executed:
# Search for references to mantine InputWrapper or Input classes in the codebase
rg "mantine-InputWrapper" --type-add 'scss:*.scss' --type scssRepository: Code-the-Change-YYC/salvationarmy
Length of output: 2187
🏁 Script executed:
# Check the actual TripSurveyModal scss module to see all defined classes
cat -n src/app/_components/drivercomponents/TripSurveyModal.module.scss | head -50Repository: Code-the-Change-YYC/salvationarmy
Length of output: 1277
.formRow lacks display: grid and is used incorrectly in TripSurveyModal.
The .formRow class applies grid gap and uses :global(.mantine-InputWrapper-*) selectors with grid-column properties, but never declares display: grid. Without this, the grid-column rules have no effect.
Additionally, in TripSurveyModal.tsx, .formRow only wraps a Radio.Group component, which doesn't produce .mantine-InputWrapper-* child elements. This means all the InputWrapper-targeting selectors (lines 19-35) are dead code here. Either:
- Remove
.formRowwrapper from the Radio.Group, or - If
.formRowis meant for input fields, adddisplay: grid; grid-template-columns: auto 1fr;to the class and apply it only to actual form inputs like TextInput and Textarea.
🤖 Prompt for AI Agents
In `@src/app/_components/drivercomponents/TripSurveyModal.module.scss` around
lines 10 - 41, .formRow currently uses grid-specific properties (gap,
grid-column on :global(.mantine-InputWrapper-*)) but never sets display: grid
and is wrapped around a Radio.Group in TripSurveyModal.tsx which doesn't render
.mantine-InputWrapper-* children; either remove the .formRow wrapper from the
Radio.Group in TripSurveyModal.tsx, or make .formRow a real grid by adding
display: grid and grid-template-columns: auto 1fr and only apply it to
components that render Mantine InputWrapper children (e.g., TextInput, Textarea)
so the :global(.mantine-InputWrapper-*) selectors take effect.
burtonjong
deleted the
SANC-31-Build-Driver-Trip-Survey-Modal-Component-Frontend-Only
branch
2 participants
Created frontend form for trip surveys.
Summary by CodeRabbit
New Features
Style