Shared UI components#4
Conversation
📝 WalkthroughWalkthroughIntroduces a new admin dashboard system with pages for candidates management and overview, redesigns the home page with marketing layout, establishes a comprehensive typed component library for layouts and UI elements, and migrates JavaScript components to TypeScript while removing directory placeholders. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 7
🧹 Nitpick comments (5)
src/components/ui/Spinner.tsx (1)
19-45: Optional: avoid duplicate label announcement to assistive tech.The outer
<div>hasaria-label={label}and also contains a visible-to-AT<span className="sr-only">{label}</span>. Many screen readers will announce the label twice (once fromaria-label, once from the accessible name of the descendant text). Pick one source of the accessible name.♻️ Proposed fix — keep only the sr-only text
<div role="status" - aria-label={label} className={cx("inline-flex items-center justify-center", className)} {...rest} >Alternatively, drop the
<span className="sr-only">and keeparia-label.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Spinner.tsx` around lines 19 - 45, The spinner currently provides the accessible name twice (aria-label on the outer div and a visible-to-AT span), so remove the duplicate by dropping aria-label={label} from the outer element and keep the <span className="sr-only">{label}</span> as the single source of the accessible name; update the Spinner component (and its prop typing if necessary) to rely on the sr-only span for the accessible name while keeping role="status" and the span intact.src/components/states/EmptyState.tsx (1)
8-12: Optional: add an ARIA live region for screen readers.When empty states replace previously rendered content (e.g., after filtering or loading), assistive tech won't announce the change. Consider adding
role="status"(impliesaria-live="polite") so the fallback message is communicated.♿ Suggested change
- <div className="text-center p-6 text-gray-500"> + <div role="status" className="text-center p-6 text-gray-500"> {message ?? "No data available"} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/states/EmptyState.tsx` around lines 8 - 12, The EmptyState component's fallback div (in the EmptyState function/component that renders {message ?? "No data available"}) should be made an ARIA live region so screen readers announce changes; update the <div> that displays message to include role="status" (or aria-live="polite" and aria-atomic="true") to ensure the empty-state message is communicated when content changes.src/components/layout/Navbar.tsx (1)
19-40: Optional: wrap navigation actions in a<nav>for semantics/a11y.The header currently contains brand + action links but no
<nav>landmark. Wrapping the right-side action slot (or the whole row) in<nav aria-label="Primary">improves screen-reader navigation, especially once additional links are added.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/Navbar.tsx` around lines 19 - 40, Wrap the navigation actions in a semantic nav landmark: update the Navbar render so the action container (the div currently referenced as the right-side slot using the right prop and rendering <Button ... />) is enclosed in a <nav aria-label="Primary"> (or wrap the entire header row containing the Link and the action slot) to provide an accessible navigation landmark; ensure the nav contains the existing classNames/structure so no styles break and keep the right prop fallback logic intact within the nav.app/page.tsx (1)
38-105: Optional: deduplicate inline SVG icon markup.The four
FeatureCardicons share identical wrapper props (viewBox,className,fill,stroke,strokeWidth); only the inner<path>data differs. Consider extracting a smallIconhelper (the same pattern is already used inapp/admin/page.tsx) or a tiny set of dedicated icon components to reduce ~50 lines of duplicated boilerplate and keep visual consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/page.tsx` around lines 38 - 105, The four FeatureCard usages duplicate identical SVG wrapper props; extract a reusable Icon component (e.g., function Icon({children, ...props}) or specific icons like CodeIcon/TestIcon/PeopleIcon/ChatIcon) that encapsulates viewBox="0 0 24 24", className="h-5 w-5", fill="none", stroke="currentColor", strokeWidth="2" and accepts the differing <path> children, then replace each inline svg in the FeatureCard calls with <Icon>...</Icon> (or dedicated icon components) and update imports/exports accordingly to remove the duplicated markup while preserving the unique path data.app/admin/candidates/page.tsx (1)
53-117: Inconsistent layout: missingNavbar.Other admin pages (
app/admin/page.tsx) render the sharedNavbarwith the “CodeAssess Admin” brand andExit Adminaction, while this page rolls its own header row and omitsNavbarentirely. Consider hoistingNavbarinto a sharedapp/admin/layout.tsxso all admin routes get a consistent header (and you can keep the per-page sub-header below it).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/admin/candidates/page.tsx` around lines 53 - 117, This page is missing the shared Navbar and duplicates a header; move the Navbar into a shared admin layout so all admin routes render it consistently. Create or update the admin layout component to render Navbar (with the "CodeAssess Admin" brand and Exit Admin action) above the per-page content, keep Footer and the page-specific sub-header (the existing inline header/BackButton/PeopleIcon block) inside the Candidate page component, and remove the duplicated top header from the Candidate component so it only renders its sub-header beneath the shared Navbar; ensure the layout exports the layout component used by admin routes and that the Candidate page (the component containing BackButton, PeopleIcon, and Footer) becomes a child of that layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/admin/candidates/page.tsx`:
- Around line 80-89: The example URL in the admin candidates page JSX (the
"Example:" block in app/admin/candidates/page.tsx) is a hardcoded Figma export
URL and must be replaced; update the content inside the Example card (the div
with "Example:" and its URL text) to show the real production pattern or a
generic templated placeholder such as
https://<your-domain>/candidate/exam/<exam-id> (or the actual production domain
pattern used by your app) so no Figma/artifact URLs are exposed.
- Line 1: Remove the leading whitespace before the Next.js client directive so
the string "use client" is the very first token in the file; edit the top of the
file (where the directive appears) to place "use client" flush-left with no
characters or BOM before it (ensure it is the first line and token in page.tsx).
- Around line 13-18: Replace the unreliable setTimeout+window.history.length
fallback by using a deterministic referrer check: call router.back() only when
document.referrer is same-origin and non-empty, otherwise directly call
router.push("/admin"); remove the window.history.length check and the
setTimeout. Update the onClick handler that currently uses router.back(),
window.history.length, setTimeout, and router.push("/admin") to use
document.referrer (or always router.push("/admin") if you prefer a guaranteed
navigation) so we avoid the race and incorrect history assumptions.
In `@app/admin/page.tsx`:
- Around line 86-91: The SVG path used for the "Assign Candidates" Icon (the d
string in the Icon prop inside the FeatureCard) contains a degenerate sub-path
"M8.5 7a4 4 0 1 0 0.1 0Z" which does not render a proper circle; replace that
sub-path with a correct full-circle path (e.g. use two arc commands or a single
arc that closes to form a circle) or switch to a <circle> element for the head
instead of the malformed path; alternatively extend the local Icon component to
accept multiple children so you can supply separate <path> or <circle> elements
for the glyph and update the FeatureCard usage to pass the multi-element SVG.
In `@src/components/layout/Footer.tsx`:
- Around line 6-14: The Footer component currently hardcodes year = 2026 which
will become stale; change the default to compute the current year at render time
(e.g., use new Date().getFullYear()) so Footer (and FooterProps) use a dynamic
default instead of a fixed literal; update the function signature or compute a
local const inside Footer to set year = new Date().getFullYear() when no prop is
provided.
In `@src/components/ui/Button.tsx`:
- Around line 77-83: The Link branch drops onClick and other HTML attributes;
update the JSX return for the href branch (the <Link> usage) to spread the
remaining props (e.g., ...rest) and forward onClick and any aria/data/disabled
attributes so they aren’t lost — locate the Button component and the href branch
where shared and children are used and change the element to accept the same
props forwarded from ButtonProps (or split ButtonProps into a discriminated
union for anchor vs button if you want stricter typing) so attributes like
onClick, aria-*, id, target, rel, data-*, disabled, etc. are passed through to
the Link.
In `@tsconfig.json`:
- Line 32: Remove the stale .jsx entries from the tsconfig.json "include" array:
delete "src/components/cards/StatCard.jsx",
"src/components/common/SectionTitle.jsx", and
"src/components/feedback/Alert.jsx" because the project uses the .tsx files
(StatCard.tsx, SectionTitle.tsx, Alert.tsx) which are already matched by the
existing "**/*.tsx" glob; after removal, ensure the include array remains valid
JSON (commas adjusted) and that only the correct file globs remain.
---
Nitpick comments:
In `@app/admin/candidates/page.tsx`:
- Around line 53-117: This page is missing the shared Navbar and duplicates a
header; move the Navbar into a shared admin layout so all admin routes render it
consistently. Create or update the admin layout component to render Navbar (with
the "CodeAssess Admin" brand and Exit Admin action) above the per-page content,
keep Footer and the page-specific sub-header (the existing inline
header/BackButton/PeopleIcon block) inside the Candidate page component, and
remove the duplicated top header from the Candidate component so it only renders
its sub-header beneath the shared Navbar; ensure the layout exports the layout
component used by admin routes and that the Candidate page (the component
containing BackButton, PeopleIcon, and Footer) becomes a child of that layout.
In `@app/page.tsx`:
- Around line 38-105: The four FeatureCard usages duplicate identical SVG
wrapper props; extract a reusable Icon component (e.g., function Icon({children,
...props}) or specific icons like CodeIcon/TestIcon/PeopleIcon/ChatIcon) that
encapsulates viewBox="0 0 24 24", className="h-5 w-5", fill="none",
stroke="currentColor", strokeWidth="2" and accepts the differing <path>
children, then replace each inline svg in the FeatureCard calls with
<Icon>...</Icon> (or dedicated icon components) and update imports/exports
accordingly to remove the duplicated markup while preserving the unique path
data.
In `@src/components/layout/Navbar.tsx`:
- Around line 19-40: Wrap the navigation actions in a semantic nav landmark:
update the Navbar render so the action container (the div currently referenced
as the right-side slot using the right prop and rendering <Button ... />) is
enclosed in a <nav aria-label="Primary"> (or wrap the entire header row
containing the Link and the action slot) to provide an accessible navigation
landmark; ensure the nav contains the existing classNames/structure so no styles
break and keep the right prop fallback logic intact within the nav.
In `@src/components/states/EmptyState.tsx`:
- Around line 8-12: The EmptyState component's fallback div (in the EmptyState
function/component that renders {message ?? "No data available"}) should be made
an ARIA live region so screen readers announce changes; update the <div> that
displays message to include role="status" (or aria-live="polite" and
aria-atomic="true") to ensure the empty-state message is communicated when
content changes.
In `@src/components/ui/Spinner.tsx`:
- Around line 19-45: The spinner currently provides the accessible name twice
(aria-label on the outer div and a visible-to-AT span), so remove the duplicate
by dropping aria-label={label} from the outer element and keep the <span
className="sr-only">{label}</span> as the single source of the accessible name;
update the Spinner component (and its prop typing if necessary) to rely on the
sr-only span for the accessible name while keeping role="status" and the span
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3a30e3ba-bb63-43f8-8060-8ee9f311886b
📒 Files selected for processing (41)
app/admin/candidates/page.tsxapp/admin/page.tsxapp/globals.cssapp/page.tsxsrc/app/admin/candidates/.gitkeepsrc/app/admin/exams/.gitkeepsrc/app/admin/questions/.gitkeepsrc/app/admin/reviews/.gitkeepsrc/app/assessment/[examId]/instructions/.gitkeepsrc/app/assessment/[examId]/start/.gitkeepsrc/app/assessment/[examId]/submitted/.gitkeepsrc/components/cards/FeatureCard.tsxsrc/components/cards/StatCard.tsxsrc/components/common/SectionTitle.tsxsrc/components/feedback/Alert.tsxsrc/components/feedback/Notification.tsxsrc/components/layout/AdminHeader.tsxsrc/components/layout/Footer.tsxsrc/components/layout/Navbar.tsxsrc/components/layout/PageContainer.tsxsrc/components/layout/SectionHeader.jsxsrc/components/layout/SectionHeader.tsxsrc/components/states/EmptyState.jsxsrc/components/states/EmptyState.tsxsrc/components/ui/Badge.jsxsrc/components/ui/Badge.tsxsrc/components/ui/Button.jsxsrc/components/ui/Button.tsxsrc/components/ui/Card.jsxsrc/components/ui/Card.tsxsrc/components/ui/Input.jsxsrc/components/ui/Input.tsxsrc/components/ui/Loader.tsxsrc/components/ui/Modal.jsxsrc/components/ui/Modal.tsxsrc/components/ui/Select.jsxsrc/components/ui/Select.tsxsrc/components/ui/Spinner.tsxsrc/components/ui/Textarea.jsxsrc/components/ui/Textarea.tsxtsconfig.json
💤 Files with no reviewable changes (9)
- src/app/assessment/[examId]/start/.gitkeep
- src/app/admin/exams/.gitkeep
- src/app/admin/reviews/.gitkeep
- src/app/admin/questions/.gitkeep
- src/app/admin/candidates/.gitkeep
- src/components/ui/Button.jsx
- src/app/assessment/[examId]/instructions/.gitkeep
- src/app/assessment/[examId]/submitted/.gitkeep
- src/components/states/EmptyState.jsx
| @@ -0,0 +1,120 @@ | |||
| "use client"; | |||
There was a problem hiding this comment.
Remove leading whitespace before "use client".
The Next.js directive should be the very first token in the file. The leading space here is non-idiomatic and risks being broken by tooling/formatters that strip or rewrite leading whitespace. Move it flush left.
- "use client";
+"use client";📝 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.
| "use client"; | |
| "use client"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/admin/candidates/page.tsx` at line 1, Remove the leading whitespace
before the Next.js client directive so the string "use client" is the very first
token in the file; edit the top of the file (where the directive appears) to
place "use client" flush-left with no characters or BOM before it (ensure it is
the first line and token in page.tsx).
| onClick={() => { | ||
| router.back(); | ||
| setTimeout(() => { | ||
| if (window.history.length <= 1) router.push("/admin"); | ||
| }, 0); | ||
| }} |
There was a problem hiding this comment.
Unreliable back-navigation fallback (setTimeout(0) + window.history.length).
router.back() is asynchronous and window.history.length reflects total session history — including entries from before the SPA was loaded — not whether back() actually moved within this app. Two failure modes:
- User opens
/admin/candidatesdirectly (no prior in-app history).history.lengthcan still be> 1(e.g., they came from Google), so the fallback never fires and the user appears stuck. - The
setTimeout(0)race can firepush("/admin")after a successfulback(), briefly redirecting the user away from where they intended to go.
Prefer either always pushing to a known parent route, or using document.referrer/a captured history.length snapshot. Example:
🛠 Proposed fix
function BackButton () {
const router = useRouter();
return (
<button
type="button"
aria-label="Back"
onClick={() => {
- router.back();
- setTimeout(() => {
- if (window.history.length <= 1) router.push("/admin");
- }, 0);
+ if (window.history.length > 1) {
+ router.back();
+ } else {
+ router.push("/admin");
+ }
}}📝 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.
| onClick={() => { | |
| router.back(); | |
| setTimeout(() => { | |
| if (window.history.length <= 1) router.push("/admin"); | |
| }, 0); | |
| }} | |
| onClick={() => { | |
| if (window.history.length > 1) { | |
| router.back(); | |
| } else { | |
| router.push("/admin"); | |
| } | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/admin/candidates/page.tsx` around lines 13 - 18, Replace the unreliable
setTimeout+window.history.length fallback by using a deterministic referrer
check: call router.back() only when document.referrer is same-origin and
non-empty, otherwise directly call router.push("/admin"); remove the
window.history.length check and the setTimeout. Update the onClick handler that
currently uses router.back(), window.history.length, setTimeout, and
router.push("/admin") to use document.referrer (or always router.push("/admin")
if you prefer a guaranteed navigation) so we avoid the race and incorrect
history assumptions.
| <div className="mt-4 rounded-md border border-emerald-200 bg-emerald-50 p-4 text-[11px] text-emerald-900"> | ||
| <div className="font-medium">Example:</div> | ||
| <div className="mt-1"> | ||
| https://fg.story_16809212.figma.site/candidate/exam | ||
| </div> | ||
| <div className="mt-2 text-emerald-800/90"> | ||
| When candidates click this link, they’ll fill out their | ||
| information and start the assessment. | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Hardcoded Figma export URL leaked into user-facing copy.
https://fg.story_16809212.figma.site/candidate/exam is clearly a Figma site export artifact, not a real assessment link, and it is being shown to admins as the canonical example. This needs to be replaced with the real production URL pattern (or a templated placeholder like https://<your-domain>/candidate/exam/<exam-id>) before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/admin/candidates/page.tsx` around lines 80 - 89, The example URL in the
admin candidates page JSX (the "Example:" block in
app/admin/candidates/page.tsx) is a hardcoded Figma export URL and must be
replaced; update the content inside the Example card (the div with "Example:"
and its URL text) to show the real production pattern or a generic templated
placeholder such as https://<your-domain>/candidate/exam/<exam-id> (or the
actual production domain pattern used by your app) so no Figma/artifact URLs are
exposed.
| <FeatureCard | ||
| icon={<Icon d="M16 21v-2a4 4 0 0 0-4-4H5a4 4 0 0 0-4 4v2M8.5 7a4 4 0 1 0 0.1 0Z" />} | ||
| title="Assign Candidates" | ||
| description="Invite candidates and assign exams" | ||
| href="/admin/candidates" | ||
| /> |
There was a problem hiding this comment.
Malformed SVG path for the “Assign Candidates” icon.
The second sub-path M8.5 7a4 4 0 1 0 0.1 0Z is a degenerate elliptical arc with delta (0.1, 0) — it renders as a near-invisible sliver, not the head/circle the icon needs. Use a proper full-circle path (or render a <circle>/composite element). Quick fix using two arc halves:
- icon={<Icon d="M16 21v-2a4 4 0 0 0-4-4H5a4 4 0 0 0-4 4v2M8.5 7a4 4 0 1 0 0.1 0Z" />}
+ icon={<Icon d="M16 21v-2a4 4 0 0 0-4-4H5a4 4 0 0 0-4 4v2M4.5 7a4 4 0 1 0 8 0a4 4 0 1 0 -8 0" />}If you'd prefer cleaner markup, consider extending the local Icon helper to accept multiple <path>/<circle> children rather than a single d string (the candidates page already uses a multi-element SVG for the same glyph).
📝 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.
| <FeatureCard | |
| icon={<Icon d="M16 21v-2a4 4 0 0 0-4-4H5a4 4 0 0 0-4 4v2M8.5 7a4 4 0 1 0 0.1 0Z" />} | |
| title="Assign Candidates" | |
| description="Invite candidates and assign exams" | |
| href="/admin/candidates" | |
| /> | |
| <FeatureCard | |
| icon={<Icon d="M16 21v-2a4 4 0 0 0-4-4H5a4 4 0 0 0-4 4v2M4.5 7a4 4 0 1 0 8 0a4 4 0 1 0 -8 0" />} | |
| title="Assign Candidates" | |
| description="Invite candidates and assign exams" | |
| href="/admin/candidates" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/admin/page.tsx` around lines 86 - 91, The SVG path used for the "Assign
Candidates" Icon (the d string in the Icon prop inside the FeatureCard) contains
a degenerate sub-path "M8.5 7a4 4 0 1 0 0.1 0Z" which does not render a proper
circle; replace that sub-path with a correct full-circle path (e.g. use two arc
commands or a single arc that closes to form a circle) or switch to a <circle>
element for the head instead of the malformed path; alternatively extend the
local Icon component to accept multiple children so you can supply separate
<path> or <circle> elements for the glyph and update the FeatureCard usage to
pass the multi-element SVG.
| export default function Footer({ | ||
| productName = "CodeAssess", | ||
| year = 2026, | ||
| }: FooterProps) { | ||
| return ( | ||
| <footer className="w-full"> | ||
| <div className="h-px w-full bg-zinc-200/70 shadow-[0_-1px_0_rgba(0,0,0,0.04)]" /> | ||
| <div className="min-h-[15vh] flex items-center justify-center text-center text-xs text-zinc-500"> | ||
| © {year} {productName}. All rights reserved. |
There was a problem hiding this comment.
Avoid hardcoding the copyright year as a default.
year = 2026 will become stale on Jan 1 and silently misrender across every page consuming <Footer /> without a prop. Default to the current year at render time so the footer stays correct without a release.
🔧 Proposed fix
export default function Footer({
productName = "CodeAssess",
- year = 2026,
+ year = new Date().getFullYear(),
}: FooterProps) {Note: this renders on the server in App Router, so output is deterministic per request and won't cause hydration drift. If this footer is ever used client-side and needs SSR/CSR consistency, pass year explicitly from the parent.
📝 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.
| export default function Footer({ | |
| productName = "CodeAssess", | |
| year = 2026, | |
| }: FooterProps) { | |
| return ( | |
| <footer className="w-full"> | |
| <div className="h-px w-full bg-zinc-200/70 shadow-[0_-1px_0_rgba(0,0,0,0.04)]" /> | |
| <div className="min-h-[15vh] flex items-center justify-center text-center text-xs text-zinc-500"> | |
| © {year} {productName}. All rights reserved. | |
| export default function Footer({ | |
| productName = "CodeAssess", | |
| year = new Date().getFullYear(), | |
| }: FooterProps) { | |
| return ( | |
| <footer className="w-full"> | |
| <div className="h-px w-full bg-zinc-200/70 shadow-[0_-1px_0_rgba(0,0,0,0.04)]" /> | |
| <div className="min-h-[15vh] flex items-center justify-center text-center text-xs text-zinc-500"> | |
| © {year} {productName}. All rights reserved. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/layout/Footer.tsx` around lines 6 - 14, The Footer component
currently hardcodes year = 2026 which will become stale; change the default to
compute the current year at render time (e.g., use new Date().getFullYear()) so
Footer (and FooterProps) use a dynamic default instead of a fixed literal;
update the function signature or compute a local const inside Footer to set year
= new Date().getFullYear() when no prop is provided.
| if (href) { | ||
| return ( | ||
| <Link href={href} className={shared}> | ||
| {children} | ||
| </Link> | ||
| ); | ||
| } |
There was a problem hiding this comment.
<Link> branch silently drops onClick and all ...rest HTML attributes.
When href is set, only children and the merged shared className are forwarded to Link. Consumers passing onClick, aria-label, id, target, rel, data-*, disabled, etc. through ButtonProps will see those props silently dropped. This is especially confusing for accessibility attributes and analytics handlers.
♻️ Proposed fix: forward compatible attributes to the link
if (href) {
return (
- <Link href={href} className={shared}>
+ <Link href={href} className={shared} onClick={onClick as unknown as MouseEventHandler<HTMLAnchorElement> | undefined} {...(rest as Record<string, unknown>)}>
{children}
</Link>
);
}If anchor-specific typing is desired, consider splitting ButtonProps into a discriminated union of button vs anchor props so onClick and ...rest are typed correctly per branch.
📝 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.
| if (href) { | |
| return ( | |
| <Link href={href} className={shared}> | |
| {children} | |
| </Link> | |
| ); | |
| } | |
| if (href) { | |
| return ( | |
| <Link href={href} className={shared} onClick={onClick as unknown as MouseEventHandler<HTMLAnchorElement> | undefined} {...(rest as Record<string, unknown>)}> | |
| {children} | |
| </Link> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/Button.tsx` around lines 77 - 83, The Link branch drops
onClick and other HTML attributes; update the JSX return for the href branch
(the <Link> usage) to spread the remaining props (e.g., ...rest) and forward
onClick and any aria/data/disabled attributes so they aren’t lost — locate the
Button component and the href branch where shared and children are used and
change the element to accept the same props forwarded from ButtonProps (or split
ButtonProps into a discriminated union for anchor vs button if you want stricter
typing) so attributes like onClick, aria-*, id, target, rel, data-*, disabled,
etc. are passed through to the Link.
| ".next/dev/types/**/*.ts", | ||
| "**/*.mts" | ||
| ], | ||
| , "src/components/cards/StatCard.jsx", "src/components/common/SectionTitle.jsx", "src/components/feedback/Alert.jsx" ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the .jsx variants do not exist and only .tsx versions are present.
fd -t f 'StatCard\.(jsx|tsx)$'
fd -t f 'SectionTitle\.(jsx|tsx)$'
fd -t f 'Alert\.(jsx|tsx)$'Repository: SheCanCODE-Capstone-Projects/Assessment_Platfrom_fe
Length of output: 202
Remove references to non-existent .jsx files from tsconfig.json.
The entries on line 32 reference StatCard.jsx, SectionTitle.jsx, and Alert.jsx, but only their .tsx counterparts exist in the codebase. These paths are dead configuration and appear to be leftover artifacts from the JS→TS migration. The actual files (StatCard.tsx, SectionTitle.tsx, Alert.tsx) are already covered by the **/*.tsx glob pattern on line 28.
🔧 Proposed fix
"include": [
"next-env.d.ts",
"**/*.ts",
"**/*.tsx",
".next/types/**/*.ts",
".next/dev/types/**/*.ts",
"**/*.mts"
-, "src/components/cards/StatCard.jsx", "src/components/common/SectionTitle.jsx", "src/components/feedback/Alert.jsx" ],
+ ],
"exclude": ["node_modules"]📝 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.
| , "src/components/cards/StatCard.jsx", "src/components/common/SectionTitle.jsx", "src/components/feedback/Alert.jsx" ], | |
| "include": [ | |
| "next-env.d.ts", | |
| "**/*.ts", | |
| "**/*.tsx", | |
| ".next/types/**/*.ts", | |
| ".next/dev/types/**/*.ts", | |
| "**/*.mts" | |
| ], | |
| "exclude": ["node_modules"] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.json` at line 32, Remove the stale .jsx entries from the
tsconfig.json "include" array: delete "src/components/cards/StatCard.jsx",
"src/components/common/SectionTitle.jsx", and
"src/components/feedback/Alert.jsx" because the project uses the .tsx files
(StatCard.tsx, SectionTitle.tsx, Alert.tsx) which are already matched by the
existing "**/*.tsx" glob; after removal, ensure the include array remains valid
JSON (commas adjusted) and that only the correct file globs remain.
2 participants
Summary by CodeRabbit
Release Notes