components/navbar.tsx (1)

123-123: Remove trailing spaces before closing tags.

Lines 123 and 194 have trailing spaces before the > character (</div > and </nav >), which is inconsistent with the rest of the codebase formatting.

🔎 Proposed fix

-      </div >
+      </div>

-    </nav >
+    </nav>

Also applies to: 194-194

Configuration used: defaults

Review profile: CHILL

Plan: Pro

actions/create-issue.ts (2)

39-59: Consider adding rate limiting to prevent abuse.

The server action has no rate limiting or abuse prevention mechanisms. A malicious user could spam the endpoint and create numerous GitHub issues, potentially exhausting API rate limits or cluttering the repository.

Consider implementing:

• Rate limiting per IP address or user session
• Captcha or similar verification for anonymous requests
• Logging and monitoring of request patterns

---

48-52: Enhance success response with issue URL for better UX.

The success response could include the issue number and URL so users can track their request directly.

🔎 Proposed enhancement

         return {
             success: true,
             data: response.data,
-            message: "Issue created successfully!",
+            message: `Issue #${response.data.number} created successfully!`,
+            issueUrl: response.data.html_url,
         };

components/request-icon-modal.tsx (1)

47-76: Consider centralizing the categories list.

The hardcoded categories array might be reused elsewhere (e.g., for filtering icons, display logic, or validation). Centralizing it as a constant would improve maintainability.

🔎 Proposed refactor: Extract to shared constant

Create a new file src/constants/icon-categories.ts:

export const ICON_CATEGORIES = [
  "Arrow",
  "Brand",
  "Chart",
  "Communication",
  "Database",
  "Design",
  "Development",
  "Device",
  "Ecommerce",
  "Editor",
  "Finance",
  "Game",
  "Health",
  "Map",
  "Math",
  "Media",
  "Nature",
  "Office",
  "Security",
  "Shape",
  "Social",
  "System",
  "Text",
  "Time",
  "Travel",
  "User",
  "Weather",
  "Other",
] as const;

export type IconCategory = typeof ICON_CATEGORIES[number];

Then import and use:

+import { ICON_CATEGORIES } from "@/constants/icon-categories";

-const categories = [ ... ];
-
 ...
 <SelectContent>
-  {categories.map((cat) => (
+  {ICON_CATEGORIES.map((cat) => (

components/ui/dialog.tsx (1)

56-56: Remove redundant data-slot prop on DialogPortal.

Line 56 passes data-slot="dialog-portal" to the <DialogPortal> component, but DialogPortal (lines 19-23) already sets this attribute internally. This is redundant and the prop will be ignored since DialogPortal spreads {...props} after setting its own data-slot.

🔎 Proposed fix

-<DialogPortal data-slot="dialog-portal">
+<DialogPortal>
   <DialogOverlay />

components/icons-list.tsx (1)

132-142: Add type="button" to prevent unintended form submission.

Although the button isn't currently inside a form, explicitly setting type="button" is a best practice that prevents unexpected behavior if the component is later placed within a form context.

🔎 Proposed fix

 <button
+  type="button"
   className="text-primary hover:underline hover:cursor-pointer"
 >
   Request this icon
 </button>

Configuration used: defaults

Review profile: CHILL

Plan: Pro

actions/create-issue.ts (2)

4-6: Add input validation and sanitization to prevent abuse.

The form inputs are extracted but not validated for length or content. This could allow:

• Extremely long submissions that create oversized GitHub issues
• Markdown injection in category, links, or handle fields
• Empty or whitespace-only categories (line 8-10 only checks falsy, not trimmed content)

🔎 Recommended validation logic

 export async function createIssue(formData: FormData) {
-    const category = formData.get("category") as string;
-    const links = formData.get("links") as string;
-    const handle = formData.get("handle") as string;
+    const category = (formData.get("category") as string)?.trim();
+    const links = (formData.get("links") as string)?.trim();
+    const handle = (formData.get("handle") as string)?.trim();
 
     if (!category) {
         return { error: "Category is required" };
     }
+
+    // Enforce length limits
+    if (category.length > 50) {
+        return { error: "Category must be 50 characters or less" };
+    }
+    
+    if (links && links.length > 1000) {
+        return { error: "Reference links must be 1000 characters or less" };
+    }
+    
+    if (handle && handle.length > 39) {
+        return { error: "GitHub handle must be 39 characters or less" };
+    }

---

14-14: Avoid logging sensitive details in production.

The console.error statements may expose sensitive information (tokens, internal error details) in production logs or monitoring systems.

🔎 Proposed fix

     const token = process.env.GITHUB_ACCESS_TOKEN;
     if (!token) {
-        console.error("Missing GITHUB_ACCESS_TOKEN");
+        // Log without exposing config details in production
+        if (process.env.NODE_ENV !== 'production') {
+            console.error("Missing GITHUB_ACCESS_TOKEN");
+        }
         return { error: "Server configuration error" };
     }

         if (!response.ok) {
             const errorData = await response.json();
-            console.error("GitHub API Error:", errorData);
+            console.error("GitHub API Error:", response.status);
             return { error: "Failed to create issue on GitHub" };
         }

     } catch (err) {
-        console.error("Server Action Error:", err);
+        console.error("Server Action Error:", err instanceof Error ? err.message : "Unknown error");
         return { error: "Internal server error" };
     }

Also applies to: 40-40, 47-47

components/request-icon-modal.tsx (1)

21-27: Form fields are not reset when modal closes.

The effect resets success/error/loading state when the modal opens, but the form fields themselves (category, links, handle) retain their values when the modal is closed and reopened. This may confuse users who partially fill the form, close it, then reopen it to see their previous input still there.

🔎 Add form reset on close

     // Reset state when modal opens
     useEffect(() => {
         if (isOpen) {
             setSuccess(null);
             setError(null);
             setLoading(false);
+        } else {
+            // Reset form fields when modal closes
+            const form = document.querySelector('form');
+            form?.reset();
         }
     }, [isOpen]);

Alternatively, use a ref to the form and reset it in the effect:

+    const formRef = useRef<HTMLFormElement>(null);
+
     // Reset state when modal opens
     useEffect(() => {
         if (isOpen) {
             setSuccess(null);
             setError(null);
             setLoading(false);
+        } else {
+            formRef.current?.reset();
         }
     }, [isOpen]);

Then add the ref to the form:

-    <form onSubmit={handleSubmit} className="space-y-4">
+    <form ref={formRef} onSubmit={handleSubmit} className="space-y-4">

actions/create-issue.ts (1)

18-25: Consider escaping user input to prevent markdown formatting issues.

While GitHub's issue body is markdown-safe in most contexts, user-provided content like handle could contain markdown syntax that breaks formatting or creates misleading issues (e.g., **Bold** or [Link](url)).

Consider sanitizing or escaping markdown special characters in user inputs, or wrapping user content in code blocks:

🔎 Alternative approach using code blocks

     const title = `Request: [${category}] Icon`;
     const body = `
 **Category:** ${category}
 **Reference Links:**
-${links || "None provided"}
+\`\`\`
+${links || "None provided"}
+\`\`\`
 
-**Requested by:** ${handle || "Anonymous"}
+**Requested by:** \`${handle || "Anonymous"}\`
   `.trim();

components/request-icon-modal.tsx (2)

148-162: Add validation hint for GitHub handle format.

The GitHub handle input is optional but could benefit from a validation hint or pattern to guide users. GitHub usernames have specific rules (alphanumeric, hyphens, max 39 chars, cannot start/end with hyphen).

🔎 Add client-side pattern validation

                                                <input
                                                    id="handle"
                                                    name="handle"
                                                    type="text"
                                                    placeholder="username"
+                                                    pattern="[a-zA-Z0-9]([a-zA-Z0-9-]{0,37}[a-zA-Z0-9])?"
+                                                    title="GitHub username (alphanumeric and hyphens, 1-39 characters)"
+                                                    maxLength={39}
                                                    className="flex h-10 w-full rounded-xl border bg-background pl-8 pr-3 py-2 text-sm ring-offset-background file:border-0 file:bg-transparent file:text-sm file:font-medium placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50"
                                                />

---

56-62: Consider preventing backdrop click during form submission.

The backdrop's onClick={onClose} (line 60) allows users to close the modal while a submission is in progress (when loading is true). This could lead to confusion if the request completes after the modal closes.

🔎 Prevent close during submission

                    <motion.div
                        initial={{ opacity: 0 }}
                        animate={{ opacity: 1 }}
                        exit={{ opacity: 0 }}
-                        onClick={onClose}
+                        onClick={() => !loading && onClose()}
                        className="fixed inset-0 z-50 bg-black/60 backdrop-blur-sm"
                    />

Also disable the close button during submission:

                                    <button
                                        onClick={onClose}
+                                        disabled={loading}
-                                        className="text-muted-foreground hover:text-foreground transition-colors p-1"
+                                        className="text-muted-foreground hover:text-foreground transition-colors p-1 disabled:opacity-50 disabled:cursor-not-allowed"
                                    >

Configuration used: defaults

Review profile: CHILL

Plan: Pro

components/request-icon-modal.tsx (1)

20-26: Form fields still persist when modal is reopened.

The useEffect resets loading, success, and error state, but does not reset the actual form fields (category, links, handle). Because these are uncontrolled inputs, their DOM values persist when the modal is closed and reopened, showing stale data to users.

This issue was previously flagged but remains unresolved.

🔎 Proposed fix: Add form ref and reset on open

-import { useState, useRef, useEffect } from "react";
+import { useState, useEffect, useRef } from "react";

 export default function RequestIconModal({ isOpen, onClose }: RequestIconModalProps) {
+    const formRef = useRef<HTMLFormElement>(null);
     const [loading, setLoading] = useState(false);
     const [success, setSuccess] = useState<string | null>(null);
     const [error, setError] = useState<string | null>(null);

     useEffect(() => {
         if (isOpen) {
             setSuccess(null);
             setError(null);
             setLoading(false);
+            formRef.current?.reset();
         }
     }, [isOpen]);

Then add the ref to the form element:

-                                    <form onSubmit={handleSubmit} className="space-y-4">
+                                    <form ref={formRef} onSubmit={handleSubmit} className="space-y-4">

components/request-icon-modal.tsx (2)

3-3: Remove unused import.

useRef is imported but never used in this component.

🔎 Proposed fix

-import { useState, useRef, useEffect } from "react";
+import { useState, useEffect } from "react";

---

28-48: Consider manual category validation before submission.

While the native required attribute on the <select> should enforce category selection, adding an explicit check in handleSubmit provides a clearer user experience and guards against edge cases where browser validation might not trigger.

🔎 Proposed enhancement

 const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
     e.preventDefault();
+    
+    const formData = new FormData(e.currentTarget);
+    if (!formData.get("category")) {
+        setError("Please select a category.");
+        return;
+    }
+    
     setLoading(true);
     setError(null);

-    const formData = new FormData(e.currentTarget);
-
     try {

Configuration used: defaults

Review profile: CHILL

Plan: Pro

actions/create-issue.ts (1)

3-10: Add comprehensive input validation to prevent malicious input.

The current validation only checks if category is truthy, which allows whitespace-only values (e.g., " ") and does not enforce any length limits. This was flagged in previous reviews but remains unaddressed. Malicious users could submit extremely long strings or craft markdown injection attacks.

Implement validation to:

• Trim and reject empty/whitespace-only category values
• Enforce maximum lengths (e.g., category ≤ 50 chars, links ≤ 1000 chars, handle ≤ 50 chars)
• Optionally sanitize markdown-sensitive characters to prevent formatting manipulation

🔎 Proposed validation

 export async function createIssue(formData: FormData) {
     const category = formData.get("category") as string;
     const links = formData.get("links") as string;
     const handle = formData.get("handle") as string;
 
-    if (!category) {
+    const trimmedCategory = category?.trim();
+    if (!trimmedCategory || trimmedCategory.length === 0) {
         return { error: "Category is required" };
     }
+
+    if (trimmedCategory.length > 50) {
+        return { error: "Category must be 50 characters or less" };
+    }
+
+    if (links && links.length > 1000) {
+        return { error: "Reference links must be 1000 characters or less" };
+    }
+
+    if (handle && handle.length > 50) {
+        return { error: "Handle must be 50 characters or less" };
+    }

Then use trimmedCategory and trimmed versions of other fields in the issue body.

actions/create-issue.ts (1)

37-42: Validate the GitHub API response structure.

The code assumes issue.html_url exists without validation. If the GitHub API response format changes or is unexpected, this could result in returning undefined in the success response.

Add a check to ensure the expected field is present:

🔎 Proposed validation

     const issue = await response.json();
+    
+    if (!issue?.html_url) {
+        return { error: "Invalid response from GitHub API" };
+    }
+    
     return { success: true, url: issue.html_url };

Configuration used: defaults

Review profile: CHILL

Plan: Pro

components/request-icon-modal.tsx (3)

50-199: Add Escape key handler for keyboard accessibility.

Users expect pressing Escape to close modals, but this behavior is not implemented. This affects keyboard navigation and is a common accessibility requirement.

Based on learnings, this issue was previously flagged and remains unresolved.

🔎 Recommended fix

     useEffect(() => {
         if (isOpen) {
             setSuccess(null);
             setError(null);
             setLoading(false);
         }
     }, [isOpen]);

+    useEffect(() => {
+        const handleEscape = (e: KeyboardEvent) => {
+            if (e.key === "Escape" && isOpen && !loading) {
+                onClose();
+            }
+        };
+        
+        if (isOpen) {
+            window.addEventListener("keydown", handleEscape);
+        }
+        
+        return () => window.removeEventListener("keydown", handleEscape);
+    }, [isOpen, loading, onClose]);
+
     const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {

---

66-70: Add ARIA attributes for screen reader accessibility.

The modal container is missing essential ARIA attributes that help assistive technologies understand the dialog structure.

🔎 Proposed fix

                         <motion.div
                             initial={{ scale: 0.95, opacity: 0, y: 20 }}
                             animate={{ scale: 1, opacity: 1, y: 0 }}
                             exit={{ scale: 0.95, opacity: 0, y: 20 }}
+                            role="dialog"
+                            aria-modal="true"
+                            aria-labelledby="modal-title"
                             className="bg-background w-full max-w-md rounded-2xl border shadow-2xl pointer-events-auto overflow-hidden"
                         >

And update the heading to reference the ID:

-                                    <h2 className="text-xl font-semibold">Request an Icon</h2>
+                                    <h2 id="modal-title" className="text-xl font-semibold">Request an Icon</h2>

---

20-26: Form inputs persist across modal open/close cycles.

The useEffect resets state variables (success, error, loading) when the modal opens, but the actual form inputs remain uncontrolled and retain their values. If a user partially fills the form, closes the modal, and reopens it, the old input values will still be present.

Based on learnings, this issue was previously flagged and remains unresolved.

🔎 Recommended fix: Add form ref and reset

+import { useState, useEffect, useRef } from "react";
+
 export default function RequestIconModal({ isOpen, onClose }: RequestIconModalProps) {
     const [loading, setLoading] = useState(false);
     const [success, setSuccess] = useState<string | null>(null);
     const [error, setError] = useState<string | null>(null);
+    const formRef = useRef<HTMLFormElement>(null);

     useEffect(() => {
         if (isOpen) {
             setSuccess(null);
             setError(null);
             setLoading(false);
+            formRef.current?.reset();
         }
     }, [isOpen]);

Then add the ref to the form element:

-                                    <form onSubmit={handleSubmit} className="space-y-4">
+                                    <form ref={formRef} onSubmit={handleSubmit} className="space-y-4">

components/request-icon-modal.tsx (1)

3-3: Remove unused import.

useRef is imported but never used in this component.

🔎 Proposed fix

-import { useState, useRef, useEffect } from "react";
+import { useState, useEffect } from "react";

Configuration used: defaults

Review profile: CHILL

Plan: Pro

components/request-icon-modal.tsx (1)

72-89: Add accessibility attributes to the modal and close button.

The modal is missing key accessibility features flagged in previous reviews:

1. The close button (line 82-88) needs aria-label="Close modal" for screen reader users
2. The modal container should have role="dialog", aria-modal="true", and aria-labelledby referencing the heading
3. Consider adding an Escape key handler to dismiss the modal

🔎 Proposed accessibility improvements

             <motion.div
               initial={{ scale: 0.95, opacity: 0, y: 20 }}
               animate={{ scale: 1, opacity: 1, y: 0 }}
               exit={{ scale: 0.95, opacity: 0, y: 20 }}
+              role="dialog"
+              aria-modal="true"
+              aria-labelledby="request-modal-title"
               className="bg-background pointer-events-auto max-h-[85vh] w-full max-w-md overflow-y-auto rounded-2xl border shadow-2xl"
             >
               <div className="p-5">
                 <div className="mb-4 flex items-center justify-between">
-                  <h2 className="text-xl font-semibold">Request an Icon</h2>
+                  <h2 id="request-modal-title" className="text-xl font-semibold">Request an Icon</h2>
                   <button
+                    aria-label="Close modal"
                     onClick={() => !loading && onClose()}
                     disabled={loading}

components/request-icon-modal.tsx (1)

192-211: Consider extracting the loading spinner to an in-house icon.

Per the project convention discussed in previous comments, inline SVGs should be replaced with in-house icons where possible. If a spinner icon isn't available yet, consider adding a // TODO: comment.

🔎 Suggested change

                         {loading ? (
                           <div className="flex items-center gap-2">
+                            {/* TODO: Replace with in-house spinner icon when available */}
                             <svg
                               className="mr-3 -ml-1 h-4 w-4 animate-spin text-white"

Configuration used: defaults

Review profile: CHILL

Plan: Pro

actions/create-issue.ts (1)

8-10: Add whitespace trimming to prevent empty category submission.

The validation only checks if category is truthy, but doesn't trim whitespace. A user could submit a category consisting entirely of spaces (e.g., " ") which would pass validation and create a malformed issue title.

🔎 Proposed fix

 export async function createIssue(formData: FormData) {
   const category = formData.get("category") as string;
   const links = formData.get("links") as string;
   const handle = formData.get("handle") as string;
 
-  if (!category) {
+  if (!category || !category.trim()) {
     return { error: "Category is required" };
   }

actions/create-issue.ts (2)

3-10: Consider adding length validation for better UX.

While GitHub likely enforces its own limits, explicit length validation would provide better user feedback. Past reviews suggested limits such as category ≤ 50 characters and links/references ≤ 1000 characters.

🔎 Proposed validation

 export async function createIssue(formData: FormData) {
   const category = formData.get("category") as string;
   const links = formData.get("links") as string;
   const handle = formData.get("handle") as string;
 
   if (!category || !category.trim()) {
     return { error: "Category is required" };
   }
+
+  if (category.length > 50) {
+    return { error: "Category must be 50 characters or less" };
+  }
+
+  if (links && links.length > 1000) {
+    return { error: "Reference links must be 1000 characters or less" };
+  }
+
+  if (handle && handle.length > 50) {
+    return { error: "GitHub handle must be 50 characters or less" };
+  }

---

3-3: Consider adding an explicit return type.

Adding an explicit return type would improve type safety and make the function's contract clearer to consumers.

🔎 Proposed type annotation

+type CreateIssueResult =
+  | { success: true; url: string }
+  | { error: string };
+
-export async function createIssue(formData: FormData) {
+export async function createIssue(formData: FormData): Promise<CreateIssueResult> {

Configuration used: defaults

Review profile: CHILL

Plan: Pro