implement job detail page with integrated ad slots and responsive la…#4
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces a database-driven ad system with three position slots (header, inline, job-detail), rewrites the post-job form as a multi-step modal with payment and screenshot upload, converts mock-tests analytics from API-driven to client-side computation, and applies styling updates across multiple UI components. ChangesAd System & Integration
Post Job Popup Complete Rewrite
Mock Tests Analytics & Recommendations
UI Styling & Icon Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.4.16)src/app/globals.cssFile contains syntax errors that prevent linting: Line 3: Tailwind-specific syntax is disabled.; Line 3: Expected a qualified rule, or an at rule but instead found '('.; Line 3: expected 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the job browsing and job detail experience to support new responsive layouts and integrate database-driven ad placements, while also modernizing the “Post a Private Job” flow into an in-page modal.
Changes:
- Add DB-backed ad selection for the home header banner, inline job-list slot, and job-detail top slot.
- Redesign the home/job list UI (sticky header row + grid rows) and inject an inline ad after the 3rd job.
- Replace the
/post-jobnavigation with an in-page multi-step Post Job modal (UPI flow + screenshot upload).
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/adConstants.ts | Centralizes ad-position label/icon metadata for admin UI usage. |
| src/components/PostJobPopup.tsx | Replaces old post-job popup with a multi-step modal including payment + upload. |
| src/components/MockTestLayoutClient.tsx | Tweaks mock-test layout sizing/labels for better responsiveness. |
| src/components/JobList.tsx | Adds a sticky header row layout and supports an inline ad after the 3rd job. |
| src/components/JobDetailPage.tsx | Adds a top-of-detail ad slot when an ad is available. |
| src/components/JobCard.tsx | Redesigns job rows into a two-column grid with compact action buttons. |
| src/components/HomeClient.tsx | Makes the job list area independently scrollable and opens the PostJobPopup via FAB; wires header/inline ads. |
| src/components/Header.tsx | Updates header layout and buttons (Mock Test + Job Alert). |
| src/components/Footer.tsx | Updates footer styling to match the new dark header aesthetic. |
| src/components/AdSlot.tsx | Changes AdSlot to render DB-provided ads (custom image + optional link) with a fallback. |
| src/app/page.tsx | Fetches header/inline ads from Prisma and passes them to HomeClient. |
| src/app/mock-tests/page.tsx | Updates book links and computes “weakest subject” recommendation locally from analytics. |
| src/app/job/[slug]/page.tsx | Fetches job-detail ad from Prisma and passes it to JobDetailPage. |
| src/app/globals.css | Updates theme tokens (including --font-mono). |
| src/app/auth/login/page.tsx | Replaces inline SVG with lucide icon for the login header. |
| src/app/admin/upgrade-requests/page.tsx | Switches screenshot rendering to <img> with URL sanitization + safer download naming. |
| src/app/admin/ads/page.tsx | Refactors position labels to use shared constants and updates UI/preview rendering. |
| src/app/admin/ads/create/page.tsx | Uses shared position details for the “create ad” position dropdown. |
| src/app/admin/ads/[id]/page.tsx | Uses shared position details for the “edit ad” position dropdown and supports legacy positions. |
| package.json | Bumps package version. |
| package-lock.json | Bumps lockfile package version. |
| next.config.ts | Extends allowedDevOrigins. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { useSyncExternalStore } from 'react'; | ||
|
|
||
| const emptySubscribe = () => () => {}; | ||
| import { Ad } from '@prisma/client'; |
| export function AdSlot({ id, ad, className = '' }: AdSlotProps) { | ||
| if (!ad) return null; | ||
|
|
||
| if (ad.type === 'CUSTOM' && ad.imageUrl) { |
Comment on lines
8
to
10
| import { JobList } from "@/components/JobList"; | ||
| import { AdSlot } from "@/components/AdSlot"; | ||
| import { Toast } from "@/components/Toast"; |
| import { buildShareText } from "@/lib/utils"; | ||
| import { Job } from "@/types/job"; | ||
| import { Category } from "@prisma/client"; | ||
| import { Category, Ad } from "@prisma/client"; |
| import { Job } from '@/types/job'; | ||
| import { JobCard } from './JobCard'; | ||
| import { AdSlot } from '@/components/AdSlot'; | ||
| import { Ad } from '@prisma/client'; |
| style={{ gridTemplateColumns: '1fr 96px' }} | ||
| > | ||
| {/* Left — job info */} | ||
| <div className="py-3.25 pl-4 pr-2.5 min-w-0"> |
Comment on lines
+9
to
+25
| export const POSITION_DETAILS: Partial<Record<AdPosition, PositionDetail>> = { | ||
| HEADER_TOP: { | ||
| label: 'Header Top (Leaderboard)', | ||
| shortLabel: 'Header Top', | ||
| icon: 'vertical_align_top', | ||
| }, | ||
| INLINE_AFTER_3RD: { | ||
| label: 'Inline After 3rd Job', | ||
| shortLabel: 'Inline (After 3rd Job)', | ||
| icon: 'view_day', | ||
| }, | ||
| JOB_DETAIL_TOP: { | ||
| label: 'Job Detail Top', | ||
| shortLabel: 'Job Detail Top', | ||
| icon: 'article', | ||
| }, | ||
| }; |
Comment on lines
+13
to
+21
| const headerAd = await prisma.ad.findFirst({ | ||
| where: { isActive: true, position: AdPosition.HEADER_TOP }, | ||
| orderBy: { createdAt: 'desc' } | ||
| }); | ||
|
|
||
| const inlineAd = await prisma.ad.findFirst({ | ||
| where: { isActive: true, position: AdPosition.INLINE_AFTER_3RD }, | ||
| orderBy: { createdAt: 'desc' } | ||
| }); |
Comment on lines
3
to
7
| const nextConfig: NextConfig = { | ||
| /* config options here */ | ||
| // Forced restart to load new Prisma Client | ||
| allowedDevOrigins: ['10.52.111.223'], | ||
| allowedDevOrigins: ['10.52.111.223','uploaded-mime-confidence-honors.trycloudflare.com'], | ||
| }; |
|
|
||
| <div className="overflow-x-auto"> | ||
| <table className="w-full text-left border-collapse min-w-[700px]"> | ||
| <table className="w-full text-left border-collapse min-w-175"> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@next.config.ts`:
- Line 6: The allowedDevOrigins array in next.config.ts currently hard-codes a
private IP and a Cloudflare tunnel domain; change it to read from an environment
variable (e.g., process.env.ALLOWED_DEV_ORIGINS) and parse it into an array
(split on commas, trim entries) so dev tunnel domains are configured per
environment, and provide a safe default or empty array when the env var is
unset; update references to allowedDevOrigins accordingly (look for the
allowedDevOrigins symbol) and ensure .env.local is used to set
ALLOWED_DEV_ORIGINS (no hard-coded domains remain in the file).
In `@src/app/admin/upgrade-requests/page.tsx`:
- Around line 22-33: The isSafeImageUrl function currently treats
url.startsWith("/") as safe but that also matches protocol-relative URLs like
"//evil.com/..." — update isSafeImageUrl to explicitly reject strings that start
with "//" (and optionally "./" or "../" still allowed) before treating
single-leading-slash paths as safe; ensure the check occurs early in
isSafeImageUrl so any url starting with "//" returns false and only true
local-relative paths ("/", "./", "../") and http/https origins parsed by new
URL(url) are allowed.
In `@src/app/globals.css`:
- Line 71: The CSS variable --font-mono currently points to var(--font-lato) (a
sans-serif) which is incorrect for monospaced contexts; update the mapping for
--font-mono in globals.css so it uses a proper monospace stack (remove
var(--font-lato) from the value) and include common monospace fallbacks such as
ui-monospace/SFMono-Regular/Menlo/Monaco/Roboto Mono and final monospace
generic; ensure code/pre elements that rely on --font-mono will then render with
fixed-width glyphs.
In `@src/app/job/`[slug]/page.tsx:
- Around line 76-79: The Prisma query for jobDetailAd using prisma.ad.findFirst
(with AdPosition.JOB_DETAIL_TOP) should be wrapped in a try-catch so a DB
failure doesn't crash the page; update the code in page.tsx where jobDetailAd is
fetched to use try { const jobDetailAd = await prisma.ad.findFirst(...) } catch
(err) { log the error (e.g., console.error or existing logger) and set
jobDetailAd = null } and ensure subsequent rendering code tolerates a
null/undefined jobDetailAd to gracefully degrade the UI.
In `@src/app/page.tsx`:
- Around line 9-21: The four independent fetches (getJobs, getCategories,
getSiteSettings, and the two prisma.ad.findFirst calls using
AdPosition.HEADER_TOP and AdPosition.INLINE_AFTER_3RD) are currently run
serially; change them to run in parallel with Promise.all so latency doesn't
stack. Collect promises for getJobs(), getCategories(), getSiteSettings(), plus
the two prisma.ad.findFirst(...) calls, await Promise.all on that array, then
assign results back to jobs, categories, settings, headerAd and inlineAd to
preserve behavior and ordering. Ensure you keep the same query options
(where/orderBy) for the prisma calls and handle any errors the same way as
before.
In `@src/components/AdSlot.tsx`:
- Around line 27-50: Validate ad.imageUrl with the same isSafeUrl function
before rendering the <img>: inside the AdSlot component when building AdContent
(where ad.imageUrl and AdContent are used), only render the img element if
ad.imageUrl exists AND isSafeUrl(ad.imageUrl) returns true; otherwise omit the
img (or render a safe fallback). Keep the existing onError behavior and preserve
the existing targetUrl check for linking.
In `@src/components/JobCard.tsx`:
- Line 26: The JobCard component mixes inline style props (notably the style={{
gridTemplateColumns: '1fr 96px' }} on the root grid and other inline styles for
fontSize, letterSpacing, padding, height, width across its JSX) which should be
replaced with Tailwind utilities or scoped CSS variables; update the root grid
to use Tailwind arbitrary column syntax (e.g., className="grid
grid-cols-[1fr_96px] ...") and replace each inline fontSize, letterSpacing,
padding, height and width usage with equivalent Tailwind arbitrary classes
(e.g., text-[9.5px], tracking-[0.02em], p-[6px], h-[22px], w-[22px]) or extract
them to CSS custom properties in globals.css and reference via className,
ensuring you replace the style={{ ... }} occurrences inside the JobCard render
(the JSX nodes where inline styles are set) with those className changes for
consistency and maintainability.
- Around line 52-66: In JobCard replace the inline calendar SVG with the
Calendar component from lucide-react: import { Calendar } from 'lucide-react'
(alongside the existing Eye/Share2 imports), remove the entire <svg>...</svg>
block and render <Calendar /> where the SVG was, passing the same visual props
(className="shrink-0", size={9} or equivalent, strokeWidth={2} and
aria-hidden="true") so the icon matches the previous appearance and
accessibility.
In `@src/components/JobList.tsx`:
- Around line 67-96: The skeleton rows in JobList (inside the loading branch of
the JobList component) use the Tailwind class gap-1.25 which Tailwind v4 doesn't
generate; replace that class on the inner div (the one with className="flex
gap-1.25") with an arbitrary-value class such as gap-[5px] or gap-[1.25rem] (or
add a theme token if you prefer) so the spacing renders correctly.
In `@src/components/PostJobPopup.tsx`:
- Around line 259-274: The modal in PostJobPopup (the returned JSX containing
the div with role="dialog" and aria-modal="true") lacks a focus trap, so
keyboard users can tab out of the dialog; wrap the dialog contents in a
focus-trap (e.g., use FocusTrap from focus-trap-react) or implement manual focus
management: on open save activeElement, move focus to the first focusable
element in the dialog, intercept Tab/Shift+Tab to cycle within the dialog,
handle Escape to call onClose, and restore focus to the previously active
element on close; ensure the focus-trap surrounds the same element currently
rendered as the dialog (the component function PostJobPopup and its onClose
handler are the key symbols to update).
- Around line 69-101: In the useEffect inside the PostJobPopup component that
currently creates a timer with setTimeout(..., 0), remove the setTimeout
wrappers and the timer variable and perform the state resets (when open is
false) and the from/until date calculations (when open is true) directly;
replace the return cleanup that clears the timer with no-op (or remove it) since
no timeout is created. Update references to setStep, setTitle, setDescription,
setContactEmail, setContactPhone, setTotalVacancies, setEmailError,
setPhoneError, setIsSubmitting, setSubmitError, setCopied, setShowPayment,
setScreenshotName, setScreenshotBase64, setQrError, setFromDate, setUntilDate,
and getFormattedDate accordingly.
- Around line 175-196: The date-range validation is missing: ensure the app
detects when untilDate < fromDate (not just relying on the date input's min).
Update getDurationDays and the form validation by adding an explicit check
(e.g., compute start = new Date(fromDate), end = new Date(untilDate) and set a
dateRangeError flag when end < start), make getDurationDays return the actual
calculated days only when the range is valid, and include this new
dateRangeError (or the boolean check end >= start) in isStep1Valid so the step
is invalid when the range is backward; also use the valid durationDays to
compute totalCost only when the date range is valid. Ensure you reference
getDurationDays, durationDays, totalCost, isStep1Valid, fromDate and untilDate
when making changes.
- Around line 155-173: The FileReader in handleFileChange can call
setScreenshotBase64 or setSubmitError after the component unmounts; to fix,
store the FileReader in a ref (e.g., readerRef) or a local variable accessible
to cleanup, set a mounted flag (e.g., isMountedRef) in a useEffect cleanup, and
in reader.onloadend and reader.onerror check isMountedRef.current before calling
setScreenshotBase64, setSubmitError, or setScreenshotName; additionally, on
popup close/unmount abort any in-flight read by calling
readerRef.current?.abort() and clear readerRef to avoid dangling callbacks.
- Line 268: The popup container in PostJobPopup uses a nonstandard Tailwind
class `z-60` which may not be generated; change it to the arbitrary value syntax
`z-[60]` (or update theme.zIndex) on the div with className containing `z-60` to
guarantee it renders above the `z-50` backdrop, and add a keyboard focus trap
for the modal (e.g., integrate a FocusTrap/TabTrap around the modal content in
the PostJobPopup component and ensure initial focus is set to the dialog
container and focus is returned on close) to satisfy accessibility requirements
while keeping the existing ARIA attributes.
- Line 585: The Tailwind class min-h-27.5 in the div inside PostJobPopup (the
element with className starting "border-2 border-dashed ... min-h-27.5") is
invalid; replace it with a valid Tailwind utility such as min-h-28 or an
arbitrary value like min-h-[110px] to achieve the intended height and ensure the
class is generated.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 335f5e5c-c343-4729-b063-56e612ab303e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
next.config.tspackage.jsonsrc/app/admin/ads/[id]/page.tsxsrc/app/admin/ads/create/page.tsxsrc/app/admin/ads/page.tsxsrc/app/admin/upgrade-requests/page.tsxsrc/app/auth/login/page.tsxsrc/app/globals.csssrc/app/job/[slug]/page.tsxsrc/app/mock-tests/page.tsxsrc/app/page.tsxsrc/components/AdSlot.tsxsrc/components/Footer.tsxsrc/components/Header.tsxsrc/components/HomeClient.tsxsrc/components/JobCard.tsxsrc/components/JobDetailPage.tsxsrc/components/JobList.tsxsrc/components/MockTestLayoutClient.tsxsrc/components/PostJobPopup.tsxsrc/lib/adConstants.ts
📜 Review details
🔇 Additional comments (30)
src/app/admin/upgrade-requests/page.tsx (3)
174-182: LGTM!
194-203: LGTM!
255-261: LGTM!src/components/Header.tsx (1)
3-73: LGTM!src/components/Footer.tsx (1)
11-20: LGTM!src/app/auth/login/page.tsx (1)
7-7: LGTM!Also applies to: 102-102
src/components/MockTestLayoutClient.tsx (1)
135-318: LGTM!package.json (1)
3-3: LGTM!src/components/PostJobPopup.tsx (1)
1-8: LGTM!Also applies to: 10-50, 103-153, 197-255, 308-457, 460-650, 652-697
src/app/mock-tests/page.tsx (3)
22-28: LGTM!
172-199: LGTM!
455-487: LGTM!src/lib/adConstants.ts (1)
1-25: LGTM!src/app/admin/ads/page.tsx (1)
29-34: LGTM!Also applies to: 48-50, 59-67, 73-82, 89-101, 111-113, 119-124, 130-130, 159-162, 179-182, 184-187, 193-205, 218-220, 224-232, 235-240, 261-266, 274-276, 281-289, 293-297, 308-333, 346-351, 355-369, 386-390
src/app/admin/ads/[id]/page.tsx (1)
4-4: LGTM!Also applies to: 6-6, 73-79
src/app/admin/ads/create/page.tsx (1)
5-5: LGTM!Also applies to: 72-74
src/components/AdSlot.tsx (2)
9-22: LGTM!
53-58: LGTM!src/components/HomeClient.tsx (4)
11-12: LGTM!Also applies to: 15-15, 25-26
34-36: LGTM!Also applies to: 40-40
79-81: LGTM!Also applies to: 96-96
105-131: LGTM!src/app/job/[slug]/page.tsx (2)
7-8: LGTM!
84-84: LGTM!src/components/JobDetailPage.tsx (2)
11-12: LGTM!Also applies to: 17-17, 23-23
94-100: LGTM!src/components/JobList.tsx (4)
5-6: LGTM!Also applies to: 13-13
16-65: LGTM!
98-113: LGTM!
115-137: LGTM!
| /* config options here */ | ||
| // Forced restart to load new Prisma Client | ||
| allowedDevOrigins: ['10.52.111.223'], | ||
| allowedDevOrigins: ['10.52.111.223','uploaded-mime-confidence-honors.trycloudflare.com'], |
There was a problem hiding this comment.
Move development tunnel configuration to environment variables.
Hard-coding the Cloudflare tunnel domain uploaded-mime-confidence-honors.trycloudflare.com in version control creates security and maintainability risks:
- Cloudflare tunnel domains with random-word patterns are typically ephemeral and regenerate when the tunnel restarts
- Hard-coded tunnel URLs become stale and require code changes
allowedDevOriginscontrols WebSocket access to the development server; exposing this to external domains should be explicitly controlled per environment- Mixing a private IP (
10.52.111.223) with a public tunnel suggests inconsistent access patterns
Move to environment variables:
- allowedDevOrigins: ['10.52.111.223','uploaded-mime-confidence-honors.trycloudflare.com'],
+ allowedDevOrigins: process.env.ALLOWED_DEV_ORIGINS?.split(',') || [],Then set ALLOWED_DEV_ORIGINS in .env.local (git-ignored) per developer or CI environment.
📝 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
| allowedDevOrigins: ['10.52.111.223','uploaded-mime-confidence-honors.trycloudflare.com'], | |
| allowedDevOrigins: process.env.ALLOWED_DEV_ORIGINS?.split(',') || [], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@next.config.ts` at line 6, The allowedDevOrigins array in next.config.ts
currently hard-codes a private IP and a Cloudflare tunnel domain; change it to
read from an environment variable (e.g., process.env.ALLOWED_DEV_ORIGINS) and
parse it into an array (split on commas, trim entries) so dev tunnel domains are
configured per environment, and provide a safe default or empty array when the
env var is unset; update references to allowedDevOrigins accordingly (look for
the allowedDevOrigins symbol) and ensure .env.local is used to set
ALLOWED_DEV_ORIGINS (no hard-coded domains remain in the file).
Comment on lines
+22
to
+33
| function isSafeImageUrl(url: string | null): boolean { | ||
| if (!url) return false; | ||
| try { | ||
| if (url.startsWith("/") || url.startsWith("./") || url.startsWith("../")) { | ||
| return true; | ||
| } | ||
| const parsed = new URL(url); | ||
| return parsed.protocol === "http:" || parsed.protocol === "https:"; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Protocol-relative URLs bypass the safety check.
url.startsWith("/") matches protocol-relative URLs like //evil.com/malicious.jpg, which browsers resolve to the current page's protocol and load from an external domain. An attacker-controlled screenshotUrl value could exploit this to serve arbitrary content.
Add an explicit check to reject double-slash prefixes before treating as relative:
🛡️ Proposed fix
function isSafeImageUrl(url: string | null): boolean {
if (!url) return false;
try {
- if (url.startsWith("/") || url.startsWith("./") || url.startsWith("../")) {
+ // Reject protocol-relative URLs (//example.com)
+ if (url.startsWith("//")) {
+ return false;
+ }
+ if (url.startsWith("/") || url.startsWith("./") || url.startsWith("../")) {
return true;
}
const parsed = new URL(url);
return parsed.protocol === "http:" || parsed.protocol === "https:";
} catch {
return 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
| function isSafeImageUrl(url: string | null): boolean { | |
| if (!url) return false; | |
| try { | |
| if (url.startsWith("/") || url.startsWith("./") || url.startsWith("../")) { | |
| return true; | |
| } | |
| const parsed = new URL(url); | |
| return parsed.protocol === "http:" || parsed.protocol === "https:"; | |
| } catch { | |
| return false; | |
| } | |
| } | |
| function isSafeImageUrl(url: string | null): boolean { | |
| if (!url) return false; | |
| try { | |
| // Reject protocol-relative URLs (//example.com) | |
| if (url.startsWith("//")) { | |
| return false; | |
| } | |
| if (url.startsWith("/") || url.startsWith("./") || url.startsWith("../")) { | |
| return true; | |
| } | |
| const parsed = new URL(url); | |
| return parsed.protocol === "http:" || parsed.protocol === "https:"; | |
| } catch { | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/upgrade-requests/page.tsx` around lines 22 - 33, The
isSafeImageUrl function currently treats url.startsWith("/") as safe but that
also matches protocol-relative URLs like "//evil.com/..." — update
isSafeImageUrl to explicitly reject strings that start with "//" (and optionally
"./" or "../" still allowed) before treating single-leading-slash paths as safe;
ensure the check occurs early in isSafeImageUrl so any url starting with "//"
returns false and only true local-relative paths ("/", "./", "../") and
http/https origins parsed by new URL(url) are allowed.
| --color-background: var(--background); | ||
| --color-foreground: var(--foreground); | ||
| --font-sans: var(--font-lato), sans-serif; | ||
| --font-mono: var(--font-lato), monospace; |
There was a problem hiding this comment.
Semantic mismatch: --font-mono references a sans-serif font.
The --font-mono token is mapped to var(--font-lato), monospace, but --font-lato (line 70) is defined as a sans-serif font family. Monospace contexts (code blocks, preformatted text) should use monospace fonts exclusively. If --font-lato resolves successfully, monospace text will be rendered in a proportional sans-serif font instead of a fixed-width font.
🔧 Proposed fix
--font-sans: var(--font-lato), sans-serif;
- --font-mono: var(--font-lato), monospace;
+ --font-mono: ui-monospace, 'Cascadia Code', 'Source Code Pro', Menlo, Consolas, 'DejaVu Sans Mono', monospace;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/globals.css` at line 71, The CSS variable --font-mono currently
points to var(--font-lato) (a sans-serif) which is incorrect for monospaced
contexts; update the mapping for --font-mono in globals.css so it uses a proper
monospace stack (remove var(--font-lato) from the value) and include common
monospace fallbacks such as ui-monospace/SFMono-Regular/Menlo/Monaco/Roboto Mono
and final monospace generic; ensure code/pre elements that rely on --font-mono
will then render with fixed-width glyphs.
Comment on lines
+76
to
+79
| const jobDetailAd = await prisma.ad.findFirst({ | ||
| where: { isActive: true, position: AdPosition.JOB_DETAIL_TOP }, | ||
| orderBy: { createdAt: 'desc' } | ||
| }); |
There was a problem hiding this comment.
Ad query failure could break the entire job detail page.
The Prisma query for jobDetailAd is not wrapped in error handling. If the database is unavailable or the query fails, the entire page render will throw, preventing users from viewing the job details. Consider wrapping this in a try-catch to gracefully degrade when ad fetching fails.
🛡️ Proposed fix
- const jobDetailAd = await prisma.ad.findFirst({
- where: { isActive: true, position: AdPosition.JOB_DETAIL_TOP },
- orderBy: { createdAt: 'desc' }
- });
+ let jobDetailAd = null;
+ try {
+ jobDetailAd = await prisma.ad.findFirst({
+ where: { isActive: true, position: AdPosition.JOB_DETAIL_TOP },
+ orderBy: { createdAt: 'desc' }
+ });
+ } catch (error) {
+ console.error('Failed to fetch job detail ad:', error);
+ }📝 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 jobDetailAd = await prisma.ad.findFirst({ | |
| where: { isActive: true, position: AdPosition.JOB_DETAIL_TOP }, | |
| orderBy: { createdAt: 'desc' } | |
| }); | |
| let jobDetailAd = null; | |
| try { | |
| jobDetailAd = await prisma.ad.findFirst({ | |
| where: { isActive: true, position: AdPosition.JOB_DETAIL_TOP }, | |
| orderBy: { createdAt: 'desc' } | |
| }); | |
| } catch (error) { | |
| console.error('Failed to fetch job detail ad:', error); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/job/`[slug]/page.tsx around lines 76 - 79, The Prisma query for
jobDetailAd using prisma.ad.findFirst (with AdPosition.JOB_DETAIL_TOP) should be
wrapped in a try-catch so a DB failure doesn't crash the page; update the code
in page.tsx where jobDetailAd is fetched to use try { const jobDetailAd = await
prisma.ad.findFirst(...) } catch (err) { log the error (e.g., console.error or
existing logger) and set jobDetailAd = null } and ensure subsequent rendering
code tolerates a null/undefined jobDetailAd to gracefully degrade the UI.
Comment on lines
9
to
+21
| const jobs = await getJobs(); | ||
| const categories = await getCategories(); | ||
| const settings = await getSiteSettings(); | ||
|
|
||
| const headerAd = await prisma.ad.findFirst({ | ||
| where: { isActive: true, position: AdPosition.HEADER_TOP }, | ||
| orderBy: { createdAt: 'desc' } | ||
| }); | ||
|
|
||
| const inlineAd = await prisma.ad.findFirst({ | ||
| where: { isActive: true, position: AdPosition.INLINE_AFTER_3RD }, | ||
| orderBy: { createdAt: 'desc' } | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Parallelize independent data fetches to reduce homepage latency.
The calls on Line 9–Line 21 are independent but executed serially, so latency stacks. Running them in Promise.all keeps behavior the same and improves TTFB.
⚡ Suggested refactor
export default async function Home() {
- const jobs = await getJobs();
- const categories = await getCategories();
- const settings = await getSiteSettings();
-
- const headerAd = await prisma.ad.findFirst({
- where: { isActive: true, position: AdPosition.HEADER_TOP },
- orderBy: { createdAt: 'desc' }
- });
-
- const inlineAd = await prisma.ad.findFirst({
- where: { isActive: true, position: AdPosition.INLINE_AFTER_3RD },
- orderBy: { createdAt: 'desc' }
- });
+ const [jobs, categories, settings, headerAd, inlineAd] = await Promise.all([
+ getJobs(),
+ getCategories(),
+ getSiteSettings(),
+ prisma.ad.findFirst({
+ where: { isActive: true, position: AdPosition.HEADER_TOP },
+ orderBy: { createdAt: 'desc' },
+ }),
+ prisma.ad.findFirst({
+ where: { isActive: true, position: AdPosition.INLINE_AFTER_3RD },
+ orderBy: { createdAt: 'desc' },
+ }),
+ ]);📝 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 jobs = await getJobs(); | |
| const categories = await getCategories(); | |
| const settings = await getSiteSettings(); | |
| const headerAd = await prisma.ad.findFirst({ | |
| where: { isActive: true, position: AdPosition.HEADER_TOP }, | |
| orderBy: { createdAt: 'desc' } | |
| }); | |
| const inlineAd = await prisma.ad.findFirst({ | |
| where: { isActive: true, position: AdPosition.INLINE_AFTER_3RD }, | |
| orderBy: { createdAt: 'desc' } | |
| }); | |
| const [jobs, categories, settings, headerAd, inlineAd] = await Promise.all([ | |
| getJobs(), | |
| getCategories(), | |
| getSiteSettings(), | |
| prisma.ad.findFirst({ | |
| where: { isActive: true, position: AdPosition.HEADER_TOP }, | |
| orderBy: { createdAt: 'desc' }, | |
| }), | |
| prisma.ad.findFirst({ | |
| where: { isActive: true, position: AdPosition.INLINE_AFTER_3RD }, | |
| orderBy: { createdAt: 'desc' }, | |
| }), | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/page.tsx` around lines 9 - 21, The four independent fetches (getJobs,
getCategories, getSiteSettings, and the two prisma.ad.findFirst calls using
AdPosition.HEADER_TOP and AdPosition.INLINE_AFTER_3RD) are currently run
serially; change them to run in parallel with Promise.all so latency doesn't
stack. Collect promises for getJobs(), getCategories(), getSiteSettings(), plus
the two prisma.ad.findFirst(...) calls, await Promise.all on that array, then
assign results back to jobs, categories, settings, headerAd and inlineAd to
preserve behavior and ordering. Ensure you keep the same query options
(where/orderBy) for the prisma calls and handle any errors the same way as
before.
Comment on lines
+155
to
+173
| const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const file = e.target.files?.[0]; | ||
| if (file) { | ||
| if (file.size > 5 * 1024 * 1024) { | ||
| setSubmitError('Screenshot file too large (max 5MB)'); | ||
| e.target.value = ''; | ||
| return; | ||
| } | ||
| setScreenshotName(file.name); | ||
| const reader = new FileReader(); | ||
| reader.onloadend = () => { | ||
| setScreenshotBase64(reader.result as string); | ||
| }; | ||
| reader.onerror = () => { | ||
| setSubmitError('Failed to read screenshot file'); | ||
| }; | ||
| reader.readAsDataURL(file); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Potential state update on unmounted component.
If the popup closes while FileReader is still reading, the onloadend or onerror callbacks will fire and attempt setScreenshotBase64 or setSubmitError on an unmounted component. Consider aborting the read or guarding the state update.
🛡️ Suggested fix using abort and mounted check
+ const fileReaderRef = React.useRef<FileReader | null>(null);
+
const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const file = e.target.files?.[0];
if (file) {
if (file.size > 5 * 1024 * 1024) {
setSubmitError('Screenshot file too large (max 5MB)');
e.target.value = '';
return;
}
+ // Abort any in-progress read
+ fileReaderRef.current?.abort();
setScreenshotName(file.name);
const reader = new FileReader();
+ fileReaderRef.current = reader;
reader.onloadend = () => {
setScreenshotBase64(reader.result as string);
};
reader.onerror = () => {
setSubmitError('Failed to read screenshot file');
};
reader.readAsDataURL(file);
}
};Also abort on close in the reset effect:
useEffect(() => {
if (!open) {
+ fileReaderRef.current?.abort();
// ... reset state
}
}, [open]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/PostJobPopup.tsx` around lines 155 - 173, The FileReader in
handleFileChange can call setScreenshotBase64 or setSubmitError after the
component unmounts; to fix, store the FileReader in a ref (e.g., readerRef) or a
local variable accessible to cleanup, set a mounted flag (e.g., isMountedRef) in
a useEffect cleanup, and in reader.onloadend and reader.onerror check
isMountedRef.current before calling setScreenshotBase64, setSubmitError, or
setScreenshotName; additionally, on popup close/unmount abort any in-flight read
by calling readerRef.current?.abort() and clear readerRef to avoid dangling
callbacks.
Comment on lines
+175
to
+196
| const getDurationDays = () => { | ||
| if (!fromDate || !untilDate) return 8; | ||
| const start = new Date(fromDate); | ||
| const end = new Date(untilDate); | ||
| const diffTime = end.getTime() - start.getTime(); | ||
| const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24)) + 1; | ||
| return diffDays > 0 ? diffDays : 1; | ||
| }; | ||
|
|
||
| const durationDays = getDurationDays(); | ||
| const totalCost = durationDays * 9; | ||
|
|
||
| const isStep1Valid = | ||
| title.trim().length > 0 && | ||
| description.trim().length > 0 && | ||
| contactEmail.trim().length > 0 && | ||
| emailError === '' && | ||
| contactPhone.trim().length > 0 && | ||
| phoneError === '' && | ||
| fromDate !== '' && | ||
| untilDate !== ''; | ||
|
|
There was a problem hiding this comment.
Missing validation for date range where untilDate < fromDate.
The HTML min attribute on the date picker helps, but users can manually type an invalid range. When untilDate < fromDate, getDurationDays silently returns 1 instead of showing an error. This could confuse users about their actual posting duration.
🛡️ Suggested fix
+ const [dateError, setDateError] = useState('');
+
const getDurationDays = () => {
if (!fromDate || !untilDate) return 8;
const start = new Date(fromDate);
const end = new Date(untilDate);
const diffTime = end.getTime() - start.getTime();
const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24)) + 1;
- return diffDays > 0 ? diffDays : 1;
+ if (diffDays <= 0) {
+ setDateError('End date must be on or after start date');
+ return 1;
+ }
+ setDateError('');
+ return diffDays;
};
const isStep1Valid =
title.trim().length > 0 &&
description.trim().length > 0 &&
contactEmail.trim().length > 0 &&
emailError === '' &&
contactPhone.trim().length > 0 &&
phoneError === '' &&
fromDate !== '' &&
- untilDate !== '';
+ untilDate !== '' &&
+ dateError === '';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/PostJobPopup.tsx` around lines 175 - 196, The date-range
validation is missing: ensure the app detects when untilDate < fromDate (not
just relying on the date input's min). Update getDurationDays and the form
validation by adding an explicit check (e.g., compute start = new
Date(fromDate), end = new Date(untilDate) and set a dateRangeError flag when end
< start), make getDurationDays return the actual calculated days only when the
range is valid, and include this new dateRangeError (or the boolean check end >=
start) in isStep1Valid so the step is invalid when the range is backward; also
use the valid durationDays to compute totalCost only when the date range is
valid. Ensure you reference getDurationDays, durationDays, totalCost,
isStep1Valid, fromDate and untilDate when making changes.
Comment on lines
259
to
+274
| return ( | ||
| <> | ||
| {/* Backdrop */} | ||
| {open && ( | ||
| <div | ||
| className="fixed inset-0 z-40 bg-black opacity-50 transition-opacity" | ||
| onClick={onClose} | ||
| /> | ||
| )} | ||
| <div | ||
| className="fixed inset-0 z-50 bg-black/60 backdrop-blur-sm transition-opacity" | ||
| onClick={onClose} | ||
| /> | ||
|
|
||
| {/* Popup */} | ||
| <div | ||
| className={`fixed inset-0 z-50 flex items-center justify-center p-4 transition-opacity ${ | ||
| open ? 'opacity-100 pointer-events-auto' : 'opacity-0 pointer-events-none' | ||
| }`} | ||
| > | ||
| <div className={`bg-white rounded-2xl max-w-2xl w-full max-h-[90vh] overflow-y-auto shadow-2xl`}> | ||
| <div className="fixed inset-0 z-60 flex items-center justify-center p-4 sm:p-6 pointer-events-none"> | ||
| <div | ||
| role="dialog" | ||
| aria-modal="true" | ||
| aria-labelledby="modal-title" | ||
| className="bg-white rounded-3xl w-full max-w-2xl max-h-[90vh] overflow-y-auto [&::-webkit-scrollbar]:hidden [-ms-overflow-style:none] scrollbar-none shadow-2xl pointer-events-auto flex flex-col font-sans text-[#0F172A] border border-[#E2E8F0]" | ||
| > |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Consider adding focus trap for modal accessibility.
The modal has good ARIA attributes (role="dialog", aria-modal="true"), but lacks a focus trap. Keyboard users can Tab out of the modal into the backdrop or underlying content. Consider using a library like focus-trap-react or implementing manual focus management.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/PostJobPopup.tsx` around lines 259 - 274, The modal in
PostJobPopup (the returned JSX containing the div with role="dialog" and
aria-modal="true") lacks a focus trap, so keyboard users can tab out of the
dialog; wrap the dialog contents in a focus-trap (e.g., use FocusTrap from
focus-trap-react) or implement manual focus management: on open save
activeElement, move focus to the first focusable element in the dialog,
intercept Tab/Shift+Tab to cycle within the dialog, handle Escape to call
onClose, and restore focus to the previously active element on close; ensure the
focus-trap surrounds the same element currently rendered as the dialog (the
component function PostJobPopup and its onClose handler are the key symbols to
update).
| }`} | ||
| > | ||
| <div className={`bg-white rounded-2xl max-w-2xl w-full max-h-[90vh] overflow-y-auto shadow-2xl`}> | ||
| <div className="fixed inset-0 z-60 flex items-center justify-center p-4 sm:p-6 pointer-events-none"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for custom zIndex configuration in tailwind config files
echo "=== Checking Tailwind config files for zIndex customization ==="
fd -e js -e ts -e mjs 'tailwind.config' --exec cat {} 2>/dev/null | grep -A 10 -i 'zindex\|z-index' || echo "No zIndex customization found"
echo ""
echo "=== Checking for CSS-first `@theme` z-index definitions ==="
fd -e css . --exec grep -l '`@theme`\|zIndex' {} 2>/dev/null | head -5 | xargs cat 2>/dev/null | grep -A 5 -B 2 'z-\|zIndex' || echo "No `@theme` z-index found"Repository: Souvik65/job-father
Length of output: 240
Fix Tailwind z-index for the popup container
z-60 on the container (line 268) is not defined by this project’s Tailwind setup (no zIndex/z-index customization found), so it may not generate—use z-[60] (or extend theme.zIndex) to ensure it stays above the z-50 backdrop. Also add a focus trap for the modal dialog (ARIA is present).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/PostJobPopup.tsx` at line 268, The popup container in
PostJobPopup uses a nonstandard Tailwind class `z-60` which may not be
generated; change it to the arbitrary value syntax `z-[60]` (or update
theme.zIndex) on the div with className containing `z-60` to guarantee it
renders above the `z-50` backdrop, and add a keyboard focus trap for the modal
(e.g., integrate a FocusTrap/TabTrap around the modal content in the
PostJobPopup component and ensure initial focus is set to the dialog container
and focus is returned on close) to satisfy accessibility requirements while
keeping the existing ARIA attributes.
| <label className="font-bold text-[10px] text-[#475569] tracking-widest mb-1.5 uppercase block"> | ||
| <span>Upload Payment Screenshot</span> <span className="text-red-500">*</span> | ||
| </label> | ||
| <div className="border-2 border-dashed border-[#CBD5E1] bg-[#F8FAFC] hover:bg-[#F1F5F9] hover:border-[#94A3B8] rounded-2xl p-5 text-center cursor-pointer transition flex flex-col items-center justify-center gap-2 min-h-27.5 relative"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for these specific Tailwind classes to see if they're used elsewhere or configured
echo "=== Checking if min-h-27.5 and max-w-50 are used elsewhere ==="
rg -n 'min-h-27\.5|max-w-50' --type-add 'web:*.{tsx,jsx,ts,js,css}' --type web
echo ""
echo "=== Checking Tailwind config for custom spacing/sizing ==="
fd -e js -e ts -e mjs 'tailwind.config' --exec cat {} 2>/dev/null | grep -A 20 'extend\|spacing\|minHeight\|maxWidth' || echo "No custom spacing config found"Repository: Souvik65/job-father
Length of output: 756
🌐 Web query:
Tailwind CSS default maxWidth spacing scale: is max-w-50a valid built-in class? Tailwind minHeight: doesmin-h-27.5 exist in default spacing?
💡 Result:
Yes, max-w-50 is a valid built-in class in Tailwind CSS [1]. It is part of the default spacing scale, which maps to 12.5rem (200px) [1]. No, min-h-27.5 is not a valid class in the default Tailwind CSS spacing scale [1][2]. The default spacing scale includes specific increments such as 24, 28, and 32, but it does not include 27.5 [1][2]. While Tailwind allows you to use arbitrary values with square bracket notation (e.g., min-h-[27.5px] or min-h-[6.875rem]) for custom values not in the default scale, 27.5 is not part of the standard, out-of-the-box spacing configuration [3][4].
Citations:
- 1: https://v3.tailwindcss.com/docs/customizing-spacing
- 2: https://v3.tailwindcss.com/docs/min-height
- 3: https://tailwindcss.com/docs/max-width
- 4: https://tailwindcss.com/docs/min-height
Fix invalid Tailwind class min-h-27.5 (line 585)
min-h-27.5 isn’t in Tailwind’s default spacing scale, and there’s no custom spacing config in this repo, so Tailwind won’t generate it.
Suggestion
-min-h-27.5
+min-h-[6.875rem] // use arbitrary value syntax matching the intended sizemax-w-50 (lines 592, 672) is a valid default Tailwind class, so it doesn’t need changing.
📝 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="border-2 border-dashed border-[#CBD5E1] bg-[#F8FAFC] hover:bg-[#F1F5F9] hover:border-[#94A3B8] rounded-2xl p-5 text-center cursor-pointer transition flex flex-col items-center justify-center gap-2 min-h-27.5 relative"> | |
| <div className="border-2 border-dashed border-[`#CBD5E1`] bg-[`#F8FAFC`] hover:bg-[`#F1F5F9`] hover:border-[`#94A3B8`] rounded-2xl p-5 text-center cursor-pointer transition flex flex-col items-center justify-center gap-2 min-h-[6.875rem] relative"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/PostJobPopup.tsx` at line 585, The Tailwind class min-h-27.5
in the div inside PostJobPopup (the element with className starting "border-2
border-dashed ... min-h-27.5") is invalid; replace it with a valid Tailwind
utility such as min-h-28 or an arbitrary value like min-h-[110px] to achieve the
intended height and ensure the class is generated.
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
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.
…yout