Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added src/assets/icons/avatarIcon.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
181 changes: 181 additions & 0 deletions src/components/CamperPopupModal.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
"use client";

import React, { useState, useEffect } from "react";
import Image from "next/image";
import avatarIcon from "@/assets/icons/avatarIcon.png";
import crossIcon from "@/assets/icons/crossIcon.svg";

type CamperDetails = {
name: string;
bunk: string;
navStatus: "NAV" | "OCP" | string;
swimLevel: string;
emergencyMedication?: string;
conflicts: string[];
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Display the navStatus field as required by PR objectives.

The navStatus field is defined in the type but never displayed in the component. According to the PR objectives, the modal must include "Bunk and (NAV/OCP)" information. Add the navStatus display alongside the bunk information.

Additionally, the type "NAV" | "OCP" | string is redundant since string already covers all possible values. Simplify to just string or remove the trailing | string to enforce only "NAV" or "OCP":

-    navStatus: "NAV" | "OCP" | string;
+    navStatus: "NAV" | "OCP";
📝 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.

Suggested change
type CamperDetails = {
name: string;
bunk: string;
navStatus: "NAV" | "OCP" | string;
swimLevel: string;
emergencyMedication?: string;
conflicts: string[];
}
type CamperDetails = {
name: string;
bunk: string;
navStatus: "NAV" | "OCP";
swimLevel: string;
emergencyMedication?: string;
conflicts: string[];
}
🤖 Prompt for AI Agents
In src/components/CamperPopupModal.tsx around lines 8 to 15, the CamperDetails
type defines navStatus but the component doesn't render it and the type
currently uses the redundant `"NAV" | "OCP" | string`; update the type to either
just string or to `"NAV" | "OCP"` (choose enforcing union if you want to
restrict values), and modify the modal JSX to display bunk and navStatus
together (e.g., "Bunk: {bunk} ({navStatus})") so the UI shows the required "Bunk
and (NAV/OCP)" information.


type CamperModalProps = {
camper: CamperDetails;
isOpen: boolean;
onClose: () => void;
onEdit?: () => void;
};

export default function CamperModal({
camper,
isOpen,
onClose,
onEdit,
}: CamperModalProps) {
const [isEditMode, setIsEditMode] = useState(false);
const [editedCamper, setEditedCamper] = useState(camper);

// Update edited camper when camper prop changes
useEffect(() => {
setEditedCamper(camper);
}, [camper]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Prevent data loss by checking edit mode before syncing state.

The useEffect will overwrite editedCamper even when the user is actively editing, causing data loss if the parent component re-renders with a new camper reference.

Apply this diff to only sync when not in edit mode:

     // Update edited camper when camper prop changes
     useEffect(() => {
-        setEditedCamper(camper);
-    }, [camper]);
+        if (!isEditMode) {
+            setEditedCamper(camper);
+        }
+    }, [camper, isEditMode]);
📝 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.

Suggested change
// Update edited camper when camper prop changes
useEffect(() => {
setEditedCamper(camper);
}, [camper]);
// Update edited camper when camper prop changes
useEffect(() => {
if (!isEditMode) {
setEditedCamper(camper);
}
}, [camper, isEditMode]);
🤖 Prompt for AI Agents
In src/components/CamperPopupModal.tsx around lines 33-36, the useEffect
currently always syncs setEditedCamper(camper) on camper changes which
overwrites in-progress edits; change the effect to only setEditedCamper when the
component is NOT in edit mode by checking an edit flag (e.g., isEditing or
isEditMode) before calling setEditedCamper, add that flag to the effect
dependency array, and if there is no existing flag add a local state to track
whether the user has started editing (set it in your input change handlers) so
parent re-renders won’t overwrite in-progress edits.


if (!isOpen) return null;

const handleEditClick = () => {
setIsEditMode(true);
if (onEdit) {
onEdit();
}
};

const handleSave = () => {
// TODO: Add save logic here (e.g., API call)
setIsEditMode(false);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Implement save logic to persist changes.

The handleSave function exits edit mode without persisting the changes. All edits are lost since editedCamper is never communicated back to the parent component. You need a callback prop (e.g., onSave) to pass the updated camper data to the parent.

Add an onSave prop to CamperModalProps and call it with the edited data:

 type CamperModalProps = {
     camper: CamperDetails;
     isOpen: boolean;
     onClose: () => void;
     onEdit?: () => void;
+    onSave?: (updatedCamper: CamperDetails) => void;
 };

Then update handleSave:

 const handleSave = () => {
-    // TODO: Add save logic here (e.g., API call)
+    if (onSave) {
+        onSave(editedCamper);
+    }
     setIsEditMode(false);
 };
📝 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.

Suggested change
const handleSave = () => {
// TODO: Add save logic here (e.g., API call)
setIsEditMode(false);
};
const handleSave = () => {
if (onSave) {
onSave(editedCamper);
}
setIsEditMode(false);
};
🤖 Prompt for AI Agents
In src/components/CamperPopupModal.tsx around lines 47-50, handleSave currently
only toggles edit mode and loses edits because editedCamper is never persisted;
add an onSave callback to CamperModalProps (with appropriate typing, e.g.,
(camper: Camper) => void), call props.onSave(editedCamper) inside handleSave
before setIsEditMode(false), and update any parent usage to pass a handler that
persists the camper (API call/state update) so edits are communicated back to
the parent.


const handleCancel = () => {
setEditedCamper(camper); // Reset to original values
setIsEditMode(false);
};

const handleFieldChange = (field: keyof CamperDetails, value: string | string[]) => {
setEditedCamper(prev => ({
...prev,
[field]: value
}));
};

const {
name,
bunk,
swimLevel,
emergencyMedication,
conflicts,
} = isEditMode ? editedCamper : camper;

return (
<div className="fixed inset-0 z-50 flex items-center justify-center bg-black/60 px-4">
<div className="relative w-full max-w-md bg-white p-6 text-camp-text-headingBody">
<button
onClick={onClose}
className="absolute left-4 top-4"
>
<Image src={crossIcon} alt="Close" width={20} height={20} />
</button>

<div className="absolute left-1/2 top-4 flex -translate-x-1/2 items-center gap-2">
<div className="relative h-8 w-8">
<Image src={avatarIcon} alt="Camper avatar"/>
</div>
<h2 className="text-xl font-semibold">{name}</h2>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add required width and height props to the Image component.

The Next.js Image component requires explicit dimensions via width and height props (or the fill prop with a positioned parent). Without these, the image may not render correctly.

Apply this diff:

             <div className="absolute left-1/2 top-4 flex -translate-x-1/2 items-center gap-2">
               <div className="relative h-8 w-8">
-                <Image src={avatarIcon} alt="Camper avatar"/>
+                <Image src={avatarIcon} alt="Camper avatar" width={32} height={32} />
               </div>
               <h2 className="text-xl font-semibold">{name}</h2>
             </div>
📝 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.

Suggested change
<div className="absolute left-1/2 top-4 flex -translate-x-1/2 items-center gap-2">
<div className="relative h-8 w-8">
<Image src={avatarIcon} alt="Camper avatar"/>
</div>
<h2 className="text-xl font-semibold">{name}</h2>
<div className="absolute left-1/2 top-4 flex -translate-x-1/2 items-center gap-2">
<div className="relative h-8 w-8">
<Image src={avatarIcon} alt="Camper avatar" width={32} height={32} />
</div>
<h2 className="text-xl font-semibold">{name}</h2>
🤖 Prompt for AI Agents
In src/components/CamperPopupModal.tsx around lines 82 to 86, the Next.js Image
is missing required size props; add explicit width and height (or use fill with
a positioned parent). Fix by giving the Image component width={32} and
height={32} to match the surrounding h-8 w-8 container (or alternatively replace
with fill and ensure the parent has position:relative and explicit size).

</div>

{!isEditMode ? (
<button
onClick={handleEditClick}
className="absolute right-4 top-3 px-3 text-camp-primary underline"
>
EDIT
</button>
) : (
<div className="absolute right-4 top-3 flex">
<button
onClick={handleCancel}
className="px-3 text-camp-text-headingBody underline"
>
X
</button>
<span className="text-camp-text-headingBody">|</span>
<button
onClick={handleSave}
className="px-3 text-camp-primary underline font-semibold"
>
SAVE
</button>
</div>
)}


<section className="mt-10 text-sm font-semibold">
<div>
<span className="font-semibold text-camp-text-headingBody italic">BUNK:</span>{" "}
{isEditMode ? (
<input
type="text"
value={editedCamper.bunk}
onChange={(e) => handleFieldChange("bunk", e.target.value)}
className="border border-gray-300 px-2 py-1 rounded"
/>
) : (
<span>{bunk}</span>
)}
</div>
<div>
<span className="font-semibold text-camp-text-headingBody italic">Swim Level:</span>{" "}
{isEditMode ? (
<input
type="text"
value={editedCamper.swimLevel}
onChange={(e) => handleFieldChange("swimLevel", e.target.value)}
className="border border-gray-300 px-2 py-1 rounded"
/>
) : (
<span>{swimLevel}</span>
)}
</div>

<div>
<span className="font-semibold text-camp-text-headingBody">
Emergency Medication:
</span>{" "}
{isEditMode ? (
<input
type="text"
value={editedCamper.emergencyMedication || ""}
onChange={(e) => handleFieldChange("emergencyMedication", e.target.value)}
className="border border-gray-300 px-2 py-1 rounded"
/>
) : (
<span>{emergencyMedication?.trim() || "None reported"}</span>
)}
</div>

<div className="flex items-start gap-3">
<span className="font-semibold text-camp-text-headingBody mt-1">
Conflicts:
</span>

{isEditMode ? (
<textarea
value={editedCamper.conflicts.join(", ")}
onChange={(e) => handleFieldChange("conflicts", e.target.value.split(", ").filter(c => c.trim()))}
className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1 rounded"
rows={3}
/>
) : (
<div className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1">
{conflicts.length > 0 ? conflicts.join(", ") : "None"}
</div>
)}
</div>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Improve conflicts parsing to handle inconsistent spacing.

The current logic splits on ", " (comma with space), which breaks if users type commas without spaces or with inconsistent spacing. Additionally, individual conflict names should be trimmed.

Apply this diff to handle various spacing patterns:

                 {isEditMode ? (
                   <textarea
                     value={editedCamper.conflicts.join(", ")}
-                    onChange={(e) => handleFieldChange("conflicts", e.target.value.split(", ").filter(c => c.trim()))}
+                    onChange={(e) => handleFieldChange("conflicts", e.target.value.split(",").map(c => c.trim()).filter(c => c))}
                     className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1 rounded"
                     rows={3}
                   />

This splits on any comma, trims each item, and filters out empty strings.

📝 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.

Suggested change
{isEditMode ? (
<textarea
value={editedCamper.conflicts.join(", ")}
onChange={(e) => handleFieldChange("conflicts", e.target.value.split(", ").filter(c => c.trim()))}
className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1 rounded"
rows={3}
/>
) : (
<div className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1">
{conflicts.length > 0 ? conflicts.join(", ") : "None"}
</div>
)}
</div>
{isEditMode ? (
<textarea
value={editedCamper.conflicts.join(", ")}
onChange={(e) => handleFieldChange("conflicts", e.target.value.split(",").map(c => c.trim()).filter(c => c))}
className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1 rounded"
rows={3}
/>
) : (
<div className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1">
{conflicts.length > 0 ? conflicts.join(", ") : "None"}
</div>
)}
</div>
🤖 Prompt for AI Agents
In src/components/CamperPopupModal.tsx around lines 164 to 176, the textarea
splitting currently uses ", " which fails on inconsistent spacing; change the
onChange handler to split on any comma, trim each entry, and filter out empty
strings (e.g. split by "," then map trim then filter(Boolean)), and ensure the
textarea value still displays conflicts joined with ", " for consistent
presentation.

</section>
</div>
</div>
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Enhance modal accessibility with keyboard support and ARIA attributes.

The modal lacks several accessibility features that are standard for modal dialogs:

  1. No ESC key handler to close the modal
  2. No focus trap (keyboard users can tab to background elements)
  3. Missing ARIA attributes (role="dialog", aria-modal="true", aria-labelledby)

Add keyboard support:

+    useEffect(() => {
+        const handleEsc = (e: KeyboardEvent) => {
+            if (e.key === 'Escape' && isOpen) {
+                if (isEditMode) {
+                    handleCancel();
+                } else {
+                    onClose();
+                }
+            }
+        };
+        document.addEventListener('keydown', handleEsc);
+        return () => document.removeEventListener('keydown', handleEsc);
+    }, [isOpen, isEditMode]);

Add ARIA attributes to the modal container:

-        <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/60 px-4">
-          <div className="relative w-full max-w-md bg-white p-6 text-camp-text-headingBody">
+        <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/60 px-4" role="dialog" aria-modal="true">
+          <div className="relative w-full max-w-md bg-white p-6 text-camp-text-headingBody" role="document">

For a complete solution, consider using a library like react-focus-lock for proper focus trapping.

📝 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.

Suggested change
return (
<div className="fixed inset-0 z-50 flex items-center justify-center bg-black/60 px-4">
<div className="relative w-full max-w-md bg-white p-6 text-camp-text-headingBody">
<button
onClick={onClose}
className="absolute left-4 top-4"
>
<Image src={crossIcon} alt="Close" width={20} height={20} />
</button>
<div className="absolute left-1/2 top-4 flex -translate-x-1/2 items-center gap-2">
<div className="relative h-8 w-8">
<Image src={avatarIcon} alt="Camper avatar"/>
</div>
<h2 className="text-xl font-semibold">{name}</h2>
</div>
{!isEditMode ? (
<button
onClick={handleEditClick}
className="absolute right-4 top-3 px-3 text-camp-primary underline"
>
EDIT
</button>
) : (
<div className="absolute right-4 top-3 flex">
<button
onClick={handleCancel}
className="px-3 text-camp-text-headingBody underline"
>
X
</button>
<span className="text-camp-text-headingBody">|</span>
<button
onClick={handleSave}
className="px-3 text-camp-primary underline font-semibold"
>
SAVE
</button>
</div>
)}
<section className="mt-10 text-sm font-semibold">
<div>
<span className="font-semibold text-camp-text-headingBody italic">BUNK:</span>{" "}
{isEditMode ? (
<input
type="text"
value={editedCamper.bunk}
onChange={(e) => handleFieldChange("bunk", e.target.value)}
className="border border-gray-300 px-2 py-1 rounded"
/>
) : (
<span>{bunk}</span>
)}
</div>
<div>
<span className="font-semibold text-camp-text-headingBody italic">Swim Level:</span>{" "}
{isEditMode ? (
<input
type="text"
value={editedCamper.swimLevel}
onChange={(e) => handleFieldChange("swimLevel", e.target.value)}
className="border border-gray-300 px-2 py-1 rounded"
/>
) : (
<span>{swimLevel}</span>
)}
</div>
<div>
<span className="font-semibold text-camp-text-headingBody">
Emergency Medication:
</span>{" "}
{isEditMode ? (
<input
type="text"
value={editedCamper.emergencyMedication || ""}
onChange={(e) => handleFieldChange("emergencyMedication", e.target.value)}
className="border border-gray-300 px-2 py-1 rounded"
/>
) : (
<span>{emergencyMedication?.trim() || "None reported"}</span>
)}
</div>
<div className="flex items-start gap-3">
<span className="font-semibold text-camp-text-headingBody mt-1">
Conflicts:
</span>
{isEditMode ? (
<textarea
value={editedCamper.conflicts.join(", ")}
onChange={(e) => handleFieldChange("conflicts", e.target.value.split(", ").filter(c => c.trim()))}
className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1 rounded"
rows={3}
/>
) : (
<div className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1">
{conflicts.length > 0 ? conflicts.join(", ") : "None"}
</div>
)}
</div>
</section>
</div>
</div>
);
return (
<div className="fixed inset-0 z-50 flex items-center justify-center bg-black/60 px-4" role="dialog" aria-modal="true">
<div className="relative w-full max-w-md bg-white p-6 text-camp-text-headingBody" role="document">
<button
onClick={onClose}
className="absolute left-4 top-4"
>
<Image src={crossIcon} alt="Close" width={20} height={20} />
</button>
<div className="absolute left-1/2 top-4 flex -translate-x-1/2 items-center gap-2">
<div className="relative h-8 w-8">
<Image src={avatarIcon} alt="Camper avatar"/>
</div>
<h2 className="text-xl font-semibold">{name}</h2>
</div>
{!isEditMode ? (
<button
onClick={handleEditClick}
className="absolute right-4 top-3 px-3 text-camp-primary underline"
>
EDIT
</button>
) : (
<div className="absolute right-4 top-3 flex">
<button
onClick={handleCancel}
className="px-3 text-camp-text-headingBody underline"
>
X
</button>
<span className="text-camp-text-headingBody">|</span>
<button
onClick={handleSave}
className="px-3 text-camp-primary underline font-semibold"
>
SAVE
</button>
</div>
)}
<section className="mt-10 text-sm font-semibold">
<div>
<span className="font-semibold text-camp-text-headingBody italic">BUNK:</span>{" "}
{isEditMode ? (
<input
type="text"
value={editedCamper.bunk}
onChange={(e) => handleFieldChange("bunk", e.target.value)}
className="border border-gray-300 px-2 py-1 rounded"
/>
) : (
<span>{bunk}</span>
)}
</div>
<div>
<span className="font-semibold text-camp-text-headingBody italic">Swim Level:</span>{" "}
{isEditMode ? (
<input
type="text"
value={editedCamper.swimLevel}
onChange={(e) => handleFieldChange("swimLevel", e.target.value)}
className="border border-gray-300 px-2 py-1 rounded"
/>
) : (
<span>{swimLevel}</span>
)}
</div>
<div>
<span className="font-semibold text-camp-text-headingBody">
Emergency Medication:
</span>{" "}
{isEditMode ? (
<input
type="text"
value={editedCamper.emergencyMedication || ""}
onChange={(e) => handleFieldChange("emergencyMedication", e.target.value)}
className="border border-gray-300 px-2 py-1 rounded"
/>
) : (
<span>{emergencyMedication?.trim() || "None reported"}</span>
)}
</div>
<div className="flex items-start gap-3">
<span className="font-semibold text-camp-text-headingBody mt-1">
Conflicts:
</span>
{isEditMode ? (
<textarea
value={editedCamper.conflicts.join(", ")}
onChange={(e) => handleFieldChange("conflicts", e.target.value.split(", ").filter(c => c.trim()))}
className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1 rounded"
rows={3}
/>
) : (
<div className="border border-black bg-gray-100 p-3 text-sm text-camp-text-headingBody italic flex-1">
{conflicts.length > 0 ? conflicts.join(", ") : "None"}
</div>
)}
</div>
</section>
</div>
</div>
);
🤖 Prompt for AI Agents
In src/components/CamperPopupModal.tsx around lines 72-180, the modal lacks
keyboard accessibility and ARIA attributes; add role="dialog", aria-modal="true"
and aria-labelledby to the modal container and give the title h2 an id (e.g.
camper-modal-title). Implement an ESC key handler inside a useEffect that
listens for keydown and calls onClose, cleaning up the listener on unmount. Add
focus management: save document.activeElement before open, set focus to the
first focusable element (or the close button) when the modal mounts, and restore
focus on unmount; for robust tab trapping, wrap the modal content with a
focus-trap (e.g. install/use react-focus-lock or react-focus-trap) to prevent
tabbing to background elements. Ensure all event listeners and focus restoration
are properly cleaned up in the effect cleanup.

}