Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideAdds controlled form state and client-side validation to the record entry modal, wiring inputs into a form submission flow that disables save until all fields are complete and resets state when the modal closes. Sequence diagram for record entry modal validation and submissionsequenceDiagram
actor User
participant RecordEntryCard
participant ReactState
participant ModalUI
User->>RecordEntryCard: Click open record entry modal
RecordEntryCard->>ReactState: setOpen(true)
ReactState-->>RecordEntryCard: open=true
RecordEntryCard->>ModalUI: Render modal with controlled inputs
User->>ModalUI: Type into input field
ModalUI->>RecordEntryCard: onChange(field, value)
RecordEntryCard->>ReactState: setFormValues(prev, field, value)
ReactState-->>RecordEntryCard: formValues updated
RecordEntryCard->>ModalUI: Re-render with new input value
User->>ModalUI: Blur input field
ModalUI->>RecordEntryCard: onBlur(field)
RecordEntryCard->>ReactState: setTouched(prev, field, true)
ReactState-->>RecordEntryCard: touched updated
RecordEntryCard->>RecordEntryCard: Compute isComplete from formValues
RecordEntryCard->>ModalUI: Update aria-invalid and error hint
RecordEntryCard->>ModalUI: Enable or disable Save button based on isComplete
User->>ModalUI: Click Save entry
ModalUI->>RecordEntryCard: handleSubmit(event)
RecordEntryCard->>RecordEntryCard: Prevent default form submission
RecordEntryCard->>ReactState: setTouched(all fields true)
RecordEntryCard->>RecordEntryCard: Check isComplete
alt Form incomplete
RecordEntryCard->>ModalUI: Show required field messages
else Form complete
RecordEntryCard->>ReactState: setOpen(false)
end
ReactState-->>RecordEntryCard: open=false
RecordEntryCard->>RecordEntryCard: useEffect(open) resets formValues and touched
RecordEntryCard->>ModalUI: Modal unmounts with cleared form state
Class diagram for RecordEntryCard component state and handlersclassDiagram
class RecordEntryCard {
+boolean open
+FormValues formValues
+Touched touched
+boolean isComplete
+handleChange(field, value)
+handleBlur(field)
+handleSubmit(event)
}
class FormValues {
+string entryName
+string value
+string notes
}
class Touched {
+boolean entryName
+boolean value
+boolean notes
}
RecordEntryCard --> FormValues : uses
RecordEntryCard --> Touched : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- You repeat the initial state objects for
formValuesandtouchedin bothuseStateand theuseEffectreset; consider extracting these into shared constants or a small initializer function to avoid duplication and keep them in sync. - Right now
aria-invalidis always set to a boolean which becomes the string "false" when valid; it would be more accessible to only setaria-invalidwhen a field is actually invalid (e.g.,aria-invalid={isInvalid || undefined}) and optionally wire the inline error text viaaria-describedby.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You repeat the initial state objects for `formValues` and `touched` in both `useState` and the `useEffect` reset; consider extracting these into shared constants or a small initializer function to avoid duplication and keep them in sync.
- Right now `aria-invalid` is always set to a boolean which becomes the string "false" when valid; it would be more accessible to only set `aria-invalid` when a field is actually invalid (e.g., `aria-invalid={isInvalid || undefined}`) and optionally wire the inline error text via `aria-describedby`.
## Individual Comments
### Comment 1
<location> `components/record-entry-card.tsx:23` </location>
<code_context>
children
}: RecordEntryCardProps) {
const [open, setOpen] = React.useState(false);
+ const [formValues, setFormValues] = React.useState({
+ entryName: "",
+ value: "",
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared form state, validation, and field definitions into reusable helpers and a config map to avoid duplicating similar logic for each field.
You can keep all the current behavior but significantly reduce repetition by:
1. Centralizing initial state and reset logic.
2. Centralizing validation.
3. Driving fields from a config map instead of inlined per-field wiring.
### 1. Centralize initial state and reset
Instead of repeating shapes in `useState` and `useEffect`:
```ts
const [formValues, setFormValues] = React.useState({
entryName: "",
value: "",
notes: ""
});
const [touched, setTouched] = React.useState({
entryName: false,
value: false,
notes: false
});
React.useEffect(() => {
if (!open) {
setFormValues({ entryName: "", value: "", notes: "" });
setTouched({ entryName: false, value: false, notes: false });
}
}, [open]);
```
extract the initial objects and reuse them:
```ts
const initialFormValues = { entryName: "", value: "", notes: "" } as const;
const initialTouched = { entryName: false, value: false, notes: false } as const;
const [formValues, setFormValues] = React.useState(initialFormValues);
const [touched, setTouched] = React.useState(initialTouched);
React.useEffect(() => {
if (!open) {
setFormValues(initialFormValues);
setTouched(initialTouched);
}
}, [open]);
```
This also makes it trivial to extend with more fields later.
### 2. Centralize validation
The repeated `touched.<field> && !formValues.<field>.trim()` can be wrapped in a helper:
```ts
type FieldName = keyof typeof formValues;
const isFieldInvalid = (field: FieldName) =>
touched[field] && !formValues[field].trim();
```
Then your inputs become:
```tsx
<input
// ...
aria-invalid={isFieldInvalid("entryName")}
/>
{isFieldInvalid("entryName") && (
<p className="text-xs text-amber-600">Entry name is required.</p>
)}
```
Same for `value` and `notes`. This pulls the validation rule into a single place.
### 3. Drive fields from a small config
You can avoid repeating the `value/onChange/onBlur/aria-invalid` wiring by using a field config and `.map`:
```ts
const fields = [
{
name: "entryName" as const,
label: "Entry name",
id: "entry-name",
placeholder: `Add ${entryTitle} reference`,
as: "input" as const
},
{
name: "value" as const,
label: "Value",
id: "entry-value",
placeholder: "Enter a value",
as: "input" as const
},
{
name: "notes" as const,
label: "Notes",
id: "entry-notes",
placeholder: "Add context for the record",
as: "textarea" as const
}
];
```
Render them uniformly:
```tsx
<div className="space-y-3 text-sm">
{fields.map(field => (
<div key={field.name} className="space-y-1">
<label
htmlFor={field.id}
className="text-xs font-medium text-slate-500"
>
{field.label}
</label>
{field.as === "input" ? (
<input
id={field.id}
type="text"
placeholder={field.placeholder}
value={formValues[field.name]}
onChange={e => handleChange(field.name, e.target.value)}
onBlur={() => handleBlur(field.name)}
aria-invalid={isFieldInvalid(field.name)}
className="w-full rounded-md border border-slate-200 px-3 py-2 text-sm text-slate-900 focus:border-slate-300 focus:outline-none"
/>
) : (
<textarea
id={field.id}
rows={3}
placeholder={field.placeholder}
value={formValues[field.name]}
onChange={e => handleChange(field.name, e.target.value)}
onBlur={() => handleBlur(field.name)}
aria-invalid={isFieldInvalid(field.name)}
className="w-full rounded-md border border-slate-200 px-3 py-2 text-sm text-slate-900 focus:border-slate-300 focus:outline-none"
/>
)}
{isFieldInvalid(field.name) && (
<p className="text-xs text-amber-600">
{field.label} {field.name === "notes" ? "are" : "is"} required.
</p>
)}
</div>
))}
</div>
```
This keeps all your current behavior (per-field touch, per-field errors, disabled submit, reset on close) but:
- Removes repeated handler/validation wiring.
- Centralizes field metadata.
- Makes adding new fields a one-line config change instead of duplicating JSX + logic.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| children | ||
| }: RecordEntryCardProps) { | ||
| const [open, setOpen] = React.useState(false); | ||
| const [formValues, setFormValues] = React.useState({ |
There was a problem hiding this comment.
issue (complexity): Consider extracting shared form state, validation, and field definitions into reusable helpers and a config map to avoid duplicating similar logic for each field.
You can keep all the current behavior but significantly reduce repetition by:
- Centralizing initial state and reset logic.
- Centralizing validation.
- Driving fields from a config map instead of inlined per-field wiring.
1. Centralize initial state and reset
Instead of repeating shapes in useState and useEffect:
const [formValues, setFormValues] = React.useState({
entryName: "",
value: "",
notes: ""
});
const [touched, setTouched] = React.useState({
entryName: false,
value: false,
notes: false
});
React.useEffect(() => {
if (!open) {
setFormValues({ entryName: "", value: "", notes: "" });
setTouched({ entryName: false, value: false, notes: false });
}
}, [open]);extract the initial objects and reuse them:
const initialFormValues = { entryName: "", value: "", notes: "" } as const;
const initialTouched = { entryName: false, value: false, notes: false } as const;
const [formValues, setFormValues] = React.useState(initialFormValues);
const [touched, setTouched] = React.useState(initialTouched);
React.useEffect(() => {
if (!open) {
setFormValues(initialFormValues);
setTouched(initialTouched);
}
}, [open]);This also makes it trivial to extend with more fields later.
2. Centralize validation
The repeated touched.<field> && !formValues.<field>.trim() can be wrapped in a helper:
type FieldName = keyof typeof formValues;
const isFieldInvalid = (field: FieldName) =>
touched[field] && !formValues[field].trim();Then your inputs become:
<input
// ...
aria-invalid={isFieldInvalid("entryName")}
/>
{isFieldInvalid("entryName") && (
<p className="text-xs text-amber-600">Entry name is required.</p>
)}Same for value and notes. This pulls the validation rule into a single place.
3. Drive fields from a small config
You can avoid repeating the value/onChange/onBlur/aria-invalid wiring by using a field config and .map:
const fields = [
{
name: "entryName" as const,
label: "Entry name",
id: "entry-name",
placeholder: `Add ${entryTitle} reference`,
as: "input" as const
},
{
name: "value" as const,
label: "Value",
id: "entry-value",
placeholder: "Enter a value",
as: "input" as const
},
{
name: "notes" as const,
label: "Notes",
id: "entry-notes",
placeholder: "Add context for the record",
as: "textarea" as const
}
];Render them uniformly:
<div className="space-y-3 text-sm">
{fields.map(field => (
<div key={field.name} className="space-y-1">
<label
htmlFor={field.id}
className="text-xs font-medium text-slate-500"
>
{field.label}
</label>
{field.as === "input" ? (
<input
id={field.id}
type="text"
placeholder={field.placeholder}
value={formValues[field.name]}
onChange={e => handleChange(field.name, e.target.value)}
onBlur={() => handleBlur(field.name)}
aria-invalid={isFieldInvalid(field.name)}
className="w-full rounded-md border border-slate-200 px-3 py-2 text-sm text-slate-900 focus:border-slate-300 focus:outline-none"
/>
) : (
<textarea
id={field.id}
rows={3}
placeholder={field.placeholder}
value={formValues[field.name]}
onChange={e => handleChange(field.name, e.target.value)}
onBlur={() => handleBlur(field.name)}
aria-invalid={isFieldInvalid(field.name)}
className="w-full rounded-md border border-slate-200 px-3 py-2 text-sm text-slate-900 focus:border-slate-300 focus:outline-none"
/>
)}
{isFieldInvalid(field.name) && (
<p className="text-xs text-amber-600">
{field.label} {field.name === "notes" ? "are" : "is"} required.
</p>
)}
</div>
))}
</div>This keeps all your current behavior (per-field touch, per-field errors, disabled submit, reset on close) but:
- Removes repeated handler/validation wiring.
- Centralizes field metadata.
- Makes adding new fields a one-line config change instead of duplicating JSX + logic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
formValuesandtouchedalong withisCompleteto determine whether the form can be saved.React.useEffectand addhandleChange,handleBlur, andhandleSubmithandlers to manage interactions.<form onSubmit={handleSubmit}>, wire inputs to state, addidattributes andaria-invalid, and render inline required hints when fields are touched or on submit.isCompleteis true and close the modal on successful submit.Testing
/manufacturingpage successfully (Next fell back to a local font after Google Fonts fetch warnings).http://127.0.0.1:3000/manufacturing, opened the record modal, and captured a screenshot; the script completed and produced the artifact successfully.Codex Task
Summary by Sourcery
Add client-side validation and controlled form handling to the record entry modal to prevent saving incomplete entries and reset state on close.
New Features:
Enhancements: