Hacker application experience and background system#458
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 21 minutes and 50 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds DB fields/migrations and Zod validators for application backgrounds; implements background visual types, registry, and renderer; refactors hacker application into a multi-step wizard; adds officer-only HackathonManager UI and tRPC create/update mutations; hardens resume handling; extracts Providers and updates UI/branding. ChangesHackathon Custom Application Feature
Resume security & API
UI component & accessibility fixes
BloomKnights Site Branding & Layout Updates
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/blade/src/app/hacker/application/[hackathon-id]/page.tsx (1)
68-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate the hacker application UI on
hackathon.applicationOpen(not justapplicationDeadline)
apps/blade/src/app/hacker/application/[hackathon-id]/page.tsxonly blocks afterhackathon.applicationDeadlinehas passed; it doesn’t hide the form beforehackathon.applicationOpen. The server already rejects early submissions (now < hackathon.applicationOpen) inpackages/api/src/routers/hackers/mutations.ts, so users currently see a form they can’t successfully submit.Add a UI check before rendering
HackerFormPage, e.g.:if (hackathon.applicationOpen > new Date()) { return ( <div className="flex h-screen w-screen items-center justify-center text-center"> <div className="flex flex-col items-center justify-center"> <h1 className="text-2xl font-bold"> Applications for {hackathon.displayName} open on{" "} {hackathon.applicationOpen.toLocaleDateString()}. </h1> <p className="text-sm text-gray-500"> Stay on the lookout by joining our{" "} <Link href="https://discord.gg/blade" target="_blank" rel="noopener noreferrer" className="text-blue-600 hover:underline" > Discord </Link> . </p> </div> </div> ); }🤖 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 `@apps/blade/src/app/hacker/application/`[hackathon-id]/page.tsx around lines 68 - 90, The UI currently only checks hackathon.applicationDeadline and still renders HackerFormPage before hackathon.applicationOpen; add a pre-render guard that checks hackathon.applicationOpen against the current date (e.g., if (hackathon.applicationOpen > new Date()) ...) and return a message similar to the deadline block that tells users when applications open and links to Discord; update the conditional in apps/blade/src/app/hacker/application/[hackathon-id]/page.tsx (near the existing deadline check and before rendering HackerFormPage) to prevent showing the form when applications are not yet open.
🧹 Nitpick comments (5)
packages/ui/src/responsive-combo-box.tsx (1)
39-85: 💤 Low valueConsider updating JSDoc to document the new props.
The docblock currently omits the new API surface:
value,defaultValue,onValueChange, andtriggerClassName. Adding these would help consumers understand the controlled/uncontrolled behavior.🤖 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 `@packages/ui/src/responsive-combo-box.tsx` around lines 39 - 85, Update the component JSDoc for ResponsiveComboBox to include the new props: value (current selected value for controlled usage), defaultValue (initial value for uncontrolled usage), onValueChange ((newValue: string) => void) to notify changes, and triggerClassName (string) to allow custom styling of the trigger button; describe their types, default behavior and how controlled vs uncontrolled usage works (i.e., use value+onValueChange for controlled components or defaultValue for uncontrolled), and add brief examples/notes in the docblock so consumers see the new API surface.apps/bloomknights/src/app/_components/navbar/MLHBadge.tsx (1)
23-32: ⚡ Quick winConsider using Tailwind classes for consistency.
The inline styles deviate from the Tailwind-based styling pattern used throughout the codebase. While the fixed positioning might be intentional for the MLH badge, using Tailwind classes would maintain consistency and make responsive adjustments easier.
♻️ Refactor to Tailwind classes
- style={{ - display: "block", - maxWidth: "100px", - minWidth: "60px", - position: "fixed", - right: "50px", - top: 0, - width: "10%", - zIndex: 10000, - }} + className="fixed right-[50px] top-0 z-[10000] block w-[10%] min-w-[60px] max-w-[100px]" >For better mobile responsiveness, consider using responsive Tailwind classes:
+ className="fixed right-4 top-0 z-[10000] block w-[15%] min-w-[50px] max-w-[80px] sm:right-8 sm:w-[12%] md:right-[50px] md:w-[10%] md:max-w-[100px]"🤖 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 `@apps/bloomknights/src/app/_components/navbar/MLHBadge.tsx` around lines 23 - 32, The MLHBadge component uses inline styles; replace the style prop on the badge element inside MLHBadge with Tailwind utility classes to match project conventions. Remove the style object and add className equivalents (e.g., block, fixed, top-0, right-[50px], max-w-[100px], min-w-[60px], w-[10%], z-[10000]) so the positioning and sizing come from Tailwind; update the JSX in MLHBadge to use className and ensure responsive variants can be added later.apps/bloomknights/src/app/globals.css (1)
429-429: 💤 Low valueBackground stretching may distort mobile background image.
background-size: 100% 100%forces the image to stretch to fit both dimensions, which can distort the aspect ratio if the container's proportions don't match the image's natural aspect ratio. If the mobile background image is designed for a specific aspect ratio, consider usingbackground-size: coverinstead to maintain proportions while ensuring full coverage.If the current stretching is intentional (e.g., the image is designed to be flexible), this is fine—just confirming the choice.
♻️ Alternative using cover for aspect-ratio preservation
- background-size: 100% 100%; + background-size: cover;🤖 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 `@apps/bloomknights/src/app/globals.css` at line 429, The rule in globals.css currently uses background-size: 100% 100% which forces distortion on mobile; change the background sizing for the selector that contains background-size: 100% 100% (search for that exact token) to use background-size: cover to preserve the image aspect ratio while ensuring full coverage, or implement a mobile-specific override via a media query that applies background-size: cover for small viewports if the full-stretch behavior must be kept on larger screens.packages/db/src/schemas/knight-hacks.ts (1)
40-49: ⚡ Quick winMirror the background invariant in the database.
This schema still allows
applicationBackgroundEnabled = truewith aNULLapplicationBackgroundKey. The validators reject that state, but direct inserts/updates can still persist it and leave the app with inconsistent hackathon config. A DB check constraint here would keep the data contract aligned across layers.🤖 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 `@packages/db/src/schemas/knight-hacks.ts` around lines 40 - 49, Add a DB-level CHECK constraint to enforce the invariant that if applicationBackgroundEnabled is true then applicationBackgroundKey must be present; specifically modify the table schema definition to include a check (e.g., via t.check(...) or equivalent in this schema DSL) that requires (applicationBackgroundEnabled = false) OR (applicationBackgroundKey IS NOT NULL AND applicationBackgroundKey <> ''), referencing the existing columns applicationBackgroundEnabled and applicationBackgroundKey so direct inserts/updates cannot create the invalid state; add a descriptive constraint name similar to existing unique("knight_hacks_hackathon_name_unique").apps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsx (1)
785-810: 💤 Low valueRedundant
setActiveStepcall in transition effectBoth timeouts set
activeStepto the same value. WhenquestionDelay < stepTransitionMs, the first timeout sets the state, and the second timeout redundantly sets it again.This is harmless but slightly wasteful. You could simplify by only calling
setActiveStepin one place.♻️ Simplified approach
useEffect(() => { if (!isStepTransitioning || transitionStep === null) return; const questionDelay = Math.min( Math.max(questionTransitionMs, 0), stepTransitionMs, ); const questionTimeout = window.setTimeout(() => { setActiveStep(transitionStep); }, questionDelay); const transitionTimeout = window.setTimeout(() => { - setActiveStep(transitionStep); setTransitionStep(null); setIsStepTransitioning(false); }, stepTransitionMs); return () => { window.clearTimeout(questionTimeout); window.clearTimeout(transitionTimeout); }; }, [ isStepTransitioning, questionTransitionMs, stepTransitionMs, transitionStep, ]);🤖 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 `@apps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsx` around lines 785 - 810, The effect currently calls setActiveStep twice (in questionTimeout and transitionTimeout); remove the redundant call so activeStep is updated only once: choose to keep setActiveStep(transitionStep) in the earlier questionTimeout (computed via questionDelay) and in the later transitionTimeout only clear the transition by calling setTransitionStep(null) and setIsStepTransitioning(false), or conversely keep it only in the transitionTimeout and have the questionTimeout do nothing; update the useEffect accordingly while keeping the existing timers, cleanup (window.clearTimeout), and dependencies (isStepTransitioning, questionTransitionMs, stepTransitionMs, transitionStep) intact and referencing setActiveStep, setTransitionStep, and setIsStepTransitioning.
🤖 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 `@apps/blade/src/app/_components/admin/hackathon/manage/hackathon-manager.tsx`:
- Around line 222-223: The component currently defaults data to [] and can
render "No hackathons yet." even when getManagedHackathons failed; update the
rendering to gate on the query status: use the returned isLoading and isError
(and error) from api.hackathon.getManagedHackathons.useQuery() instead of
defaulting data to [] — show a unified loading state when isLoading is true and
surface an error state (using the error message) when isError is true before
rendering the table; apply the same pattern to the other hackathon query usage
around the 310-328 area so rendering only happens when all required queries have
succeeded.
In
`@apps/blade/src/app/_components/dashboard/hacker/hacker-application-background.tsx`:
- Around line 261-313: The video <source> elements still use the original
per-source src list so animatedSrc/idleSrc never loads; update the mapping in
the video branch to use the computed layerSrc for every rendered source.
Concretely, replace the current (layer.sources ?? [{ mimeType: layer.mimeType,
src: layerSrc }]).map(...) usage with a computed sourcesToRender that, when
layer.sources exists, maps each source to { mimeType: source.mimeType, src:
layerSrc } (otherwise use [{ mimeType: layer.mimeType, src: layerSrc }]) and
then map sourcesToRender to <source key={source.src} src={source.src}
type={source.mimeType} /> so the active layerSrc is loaded during transitions;
refer to symbols layerSrc, layer.sources, and the video element handling in
hacker-application-background.tsx.
In `@apps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsx`:
- Line 1186: The buttonPlaceholder is using field.value directly which can be
undefined and render the literal "undefined"; change the prop to use a safe
display fallback—either call the existing helper
getComboBoxDisplayValue(field.value) or provide an inline fallback like
field.value ?? "Select…" when setting buttonPlaceholder (also apply the same
change at the other occurrence around line 1211); update the component where
buttonPlaceholder is passed so it always receives a string.
In `@apps/bloomknights/src/app/_components/navbar/MLHBadge.tsx`:
- Line 20: The MLHBadge component has mismatched years: the trust-badge asset
URL is 2027 but the href utm_campaign and the img alt text still say 2026. In
the MLHBadge.tsx component update the img alt string and the href query param
utm_campaign to use "2027-season" (and any other occurrences of "2026" in that
component) so alt text, utm_campaign and the asset year are all consistently
2027.
In `@packages/api/src/routers/hackers/mutations.ts`:
- Around line 126-149: The two separate inserts (db.insert(Hacker)... returning
insertedHacker and db.insert(HackerAttendee)...) must be executed atomically to
avoid orphan Hacker rows if the second insert fails; wrap both operations in a
single database transaction (use the project's DB transaction API, e.g.,
db.transaction or similar) and perform the Hacker insert and the subsequent
HackerAttendee insert inside that transaction so any failure rolls back the
Hacker insert, still returning the Hacker.id from the transaction context;
ensure you reference Hacker, HackerAttendee, insertedHacker and use the
transaction client (tx) for both inserts so the whole operation is atomic.
In `@packages/db/drizzle/0006_serious_mikhail_rasputin.sql`:
- Line 1: Preflight the existing data for duplicate knight_hacks_hackathon.name
values and remediate them before adding the UNIQUE constraint
knight_hacks_hackathon_name_unique; run the provided verification query to find
duplicates, then choose a remediation strategy (delete or merge duplicate rows,
update names to be unique, or backfill unique identifiers) and perform that
one-time cleanup (or add a safe SQL migration that resolves duplicates
deterministically) so that adding UNIQUE("name") in the ALTER TABLE statement
(and the Drizzle schema uniqueName on t.name) will not fail during deploy.
In `@packages/validators/src/hackathons.ts`:
- Around line 26-29: The generic constraint on
createHackathonApplicationBackgroundKeySchema rejects readonly tuples from `as
const`; update the generic to accept readonly tuples by changing `T extends
[string, ...string[]]` to `T extends readonly [string, ...string[]]` so the
`backgroundKeys: T` matches the `z.enum` expected `readonly [string,
...string[]]` and keep the function returning
`z.enum(backgroundKeys).nullable().optional()`.
---
Outside diff comments:
In `@apps/blade/src/app/hacker/application/`[hackathon-id]/page.tsx:
- Around line 68-90: The UI currently only checks hackathon.applicationDeadline
and still renders HackerFormPage before hackathon.applicationOpen; add a
pre-render guard that checks hackathon.applicationOpen against the current date
(e.g., if (hackathon.applicationOpen > new Date()) ...) and return a message
similar to the deadline block that tells users when applications open and links
to Discord; update the conditional in
apps/blade/src/app/hacker/application/[hackathon-id]/page.tsx (near the existing
deadline check and before rendering HackerFormPage) to prevent showing the form
when applications are not yet open.
---
Nitpick comments:
In `@apps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsx`:
- Around line 785-810: The effect currently calls setActiveStep twice (in
questionTimeout and transitionTimeout); remove the redundant call so activeStep
is updated only once: choose to keep setActiveStep(transitionStep) in the
earlier questionTimeout (computed via questionDelay) and in the later
transitionTimeout only clear the transition by calling setTransitionStep(null)
and setIsStepTransitioning(false), or conversely keep it only in the
transitionTimeout and have the questionTimeout do nothing; update the useEffect
accordingly while keeping the existing timers, cleanup (window.clearTimeout),
and dependencies (isStepTransitioning, questionTransitionMs, stepTransitionMs,
transitionStep) intact and referencing setActiveStep, setTransitionStep, and
setIsStepTransitioning.
In `@apps/bloomknights/src/app/_components/navbar/MLHBadge.tsx`:
- Around line 23-32: The MLHBadge component uses inline styles; replace the
style prop on the badge element inside MLHBadge with Tailwind utility classes to
match project conventions. Remove the style object and add className equivalents
(e.g., block, fixed, top-0, right-[50px], max-w-[100px], min-w-[60px], w-[10%],
z-[10000]) so the positioning and sizing come from Tailwind; update the JSX in
MLHBadge to use className and ensure responsive variants can be added later.
In `@apps/bloomknights/src/app/globals.css`:
- Line 429: The rule in globals.css currently uses background-size: 100% 100%
which forces distortion on mobile; change the background sizing for the selector
that contains background-size: 100% 100% (search for that exact token) to use
background-size: cover to preserve the image aspect ratio while ensuring full
coverage, or implement a mobile-specific override via a media query that applies
background-size: cover for small viewports if the full-stretch behavior must be
kept on larger screens.
In `@packages/db/src/schemas/knight-hacks.ts`:
- Around line 40-49: Add a DB-level CHECK constraint to enforce the invariant
that if applicationBackgroundEnabled is true then applicationBackgroundKey must
be present; specifically modify the table schema definition to include a check
(e.g., via t.check(...) or equivalent in this schema DSL) that requires
(applicationBackgroundEnabled = false) OR (applicationBackgroundKey IS NOT NULL
AND applicationBackgroundKey <> ''), referencing the existing columns
applicationBackgroundEnabled and applicationBackgroundKey so direct
inserts/updates cannot create the invalid state; add a descriptive constraint
name similar to existing unique("knight_hacks_hackathon_name_unique").
In `@packages/ui/src/responsive-combo-box.tsx`:
- Around line 39-85: Update the component JSDoc for ResponsiveComboBox to
include the new props: value (current selected value for controlled usage),
defaultValue (initial value for uncontrolled usage), onValueChange ((newValue:
string) => void) to notify changes, and triggerClassName (string) to allow
custom styling of the trigger button; describe their types, default behavior and
how controlled vs uncontrolled usage works (i.e., use value+onValueChange for
controlled components or defaultValue for uncontrolled), and add brief
examples/notes in the docblock so consumers see the new API surface.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 8354bc70-74c4-467f-83a1-0688ca48fb87
⛔ Files ignored due to path filters (7)
.playwright-cli/console-2026-05-30T23-19-59-132Z.logis excluded by!**/*.log.playwright-cli/console-2026-05-30T23-30-06-367Z.logis excluded by!**/*.log.playwright-cli/console-2026-05-30T23-48-39-176Z.logis excluded by!**/*.logapps/blade/public/hackbackgrounds/khix/FLAT_khix-reg-bg.pngis excluded by!**/*.pngapps/blade/public/hackbackgrounds/khix/FOREGROUND_khix-reg-bg.pngis excluded by!**/*.pngapps/blade/public/hackbackgrounds/khix/lennyAnim_connected.pngis excluded by!**/*.pngapps/blade/public/hackbackgrounds/khix/lennyIdle.pngis excluded by!**/*.png
📒 Files selected for processing (32)
apps/blade/src/app/_components/admin/hackathon/manage/hackathon-manager.tsxapps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/bloomknights.tsapps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/index.tsapps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/khix.tsapps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/types.tsapps/blade/src/app/_components/dashboard/hacker/hacker-application-background.tsxapps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsxapps/blade/src/app/_components/providers.tsxapps/blade/src/app/_components/theme-toggle-route-guard.tsxapps/blade/src/app/admin/hackathon/manage/page.tsxapps/blade/src/app/admin/page.tsxapps/blade/src/app/hacker/application/[hackathon-id]/page.tsxapps/blade/src/app/layout.tsxapps/bloomknights/src/app/_components/about/about.tsxapps/bloomknights/src/app/_components/faq/faq.tsxapps/bloomknights/src/app/_components/footer/footer.tsxapps/bloomknights/src/app/_components/footer/footerContent.tsapps/bloomknights/src/app/_components/navbar/MLHBadge.tsxapps/bloomknights/src/app/_components/partners/partners.tsxapps/bloomknights/src/app/globals.csspackages/api/src/routers/hackathon.tspackages/api/src/routers/hackers/mutations.tspackages/consts/src/hackathons/index.tspackages/db/drizzle/0005_flaky_bushwacker.sqlpackages/db/drizzle/0006_serious_mikhail_rasputin.sqlpackages/db/drizzle/meta/0005_snapshot.jsonpackages/db/drizzle/meta/0006_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schemas/knight-hacks.tspackages/ui/src/responsive-combo-box.tsxpackages/validators/src/hackathons.tspackages/validators/src/index.ts
| @@ -0,0 +1 @@ | |||
| ALTER TABLE "knight_hacks_hackathon" ADD CONSTRAINT "knight_hacks_hackathon_name_unique" UNIQUE("name"); No newline at end of file | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show the migration SQL content
echo "---- Migration file: packages/db/drizzle/0006_serious_mikhail_rasputin.sql ----"
sed -n '1,200p' packages/db/drizzle/0006_serious_mikhail_rasputin.sql
# 2) Find any other constraints/indexes/constraints mentioning the table/name
echo
echo "---- Search: knight_hacks_hackathon name uniqueness ----"
rg -n "knight_hacks_hackathon|name_unique|UNIQUE\\(\"name\"\\)|UNIQUE\\s*\\(\\s*name\\s*\\)|create unique|unique index" packages/db/drizzle -S || true
# 3) Search for any prior backfill/dedupe migrations referencing name/duplicates
echo
echo "---- Search: backfill/dedupe for hackathon name ----"
rg -n "duplicate|dedupe|backfill|mikhail|rasputin|hackathon.*name|knight_hacks_hackathon.*name" packages/db/drizzle -S || true
# 4) Search for ORM/schema definition of the table/field (Drizzle)
echo
echo "---- Search: Drizzle schema for knight_hacks_hackathon ----"
rg -n "knight_hacks_hackathon" packages/db -S || trueRepository: KnightHacks/forge
Length of output: 50373
Preflight duplicate knight_hacks_hackathon.name rows before adding UNIQUE constraint
This migration adds UNIQUE("name") to "knight_hacks_hackathon" (knight_hacks_hackathon_name_unique), matching the Drizzle schema expectation (uniqueName ... on t.name). If any existing hackathons already share the same name, the deploy will fail when applying the constraint.
Run a one-time dedupe/backfill (or otherwise remediate duplicates) before shipping.
Verification query
SELECT name, COUNT(*) AS duplicate_count
FROM knight_hacks_hackathon
GROUP BY name
HAVING COUNT(*) > 1;Expected result: no rows.
🤖 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 `@packages/db/drizzle/0006_serious_mikhail_rasputin.sql` at line 1, Preflight
the existing data for duplicate knight_hacks_hackathon.name values and remediate
them before adding the UNIQUE constraint knight_hacks_hackathon_name_unique; run
the provided verification query to find duplicates, then choose a remediation
strategy (delete or merge duplicate rows, update names to be unique, or backfill
unique identifiers) and perform that one-time cleanup (or add a safe SQL
migration that resolves duplicates deterministically) so that adding
UNIQUE("name") in the ALTER TABLE statement (and the Drizzle schema uniqueName
on t.name) will not fail during deploy.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@apps/bloomknights/src/app/_components/navbar/MLHBadge.tsx`:
- Around line 14-24: The MLH badge link (motion.a in MLHBadge) is still
keyboard-focusable when isHidden is true; update the element to remove it from
the tab order and accessibility tree when hidden by setting tabIndex={isHidden ?
-1 : 0} (or omit tabIndex when visible) and adding aria-hidden={isHidden};
ensure the style pointerEvents logic remains for mouse events and restore normal
focusability/ARIA when isHidden is false so keyboard users cannot tab to the
off-screen link.
In `@apps/bloomknights/src/app/_components/navbar/Navbar.tsx`:
- Around line 48-61: The nav is visually hidden but still keyboard-focusable;
update the motion.nav/NavContent interaction so when isHidden is true the nav is
removed from keyboard navigation: add aria-hidden={isHidden} (and inert if
supported) to the motion.nav and/or pass isHidden into NavContent so NavContent
sets tabIndex={-1} (or disables focus) on its interactive elements (links,
buttons, mobile menu toggle) when isHidden is true; reference the motion.nav
block and the NavContent component and use the existing isHidden state to
conditionally apply aria-hidden/inert or set tabIndex=-1 on focusable elements
to prevent tab stops when the navbar is scrolled away.
- Around line 25-64: The mobile menu open state (isMobileMenuOpen) is not
cleared when the viewport moves to desktop, causing stale state that hides
MLHBadge and interferes scroll logic; add a side-effect in the Navbar component
that listens for breakpoint changes (e.g., window.matchMedia for the md
breakpoint or equivalent layout/breakpoint hook) and calls
setIsMobileMenuOpen(false) whenever the viewport enters the desktop range,
ensure the listener is registered once (on mount) and removed on cleanup, and
keep references to the state updater (setIsMobileMenuOpen) so NavContent,
MLHBadge and the scroll hide logic behave correctly when resizing.
In `@packages/api/src/resume-security.ts`:
- Around line 22-33: isResumeObjectOwnedByUser currently accepts any leaf under
getResumeUserPrefix(userId) but must only allow server-generated names created
by createResumeObjectName; update isResumeObjectOwnedByUser (or have
normalizeOwnedResumeObjectName call it) to validate that fileName exactly
matches the server filename pattern produced by createResumeObjectName (e.g.
"resume-<uuid>.pdf") instead of allowing arbitrary leaves — implement a strict
regex check against the expected "resume-<uuid>.pdf" format (UUID form with
hyphens) for the fileName after getResumeUserPrefix(userId) and return false if
it doesn't match.
In `@packages/api/src/routers/member.ts`:
- Around line 162-165: The code currently calls normalizeOwnedResumeObjectName
with a fallback to member.resumeUrl which causes legacy/malformed stored values
to be validated even when the client didn't send resumeUrl; change this so you
only call normalizeOwnedResumeObjectName when input.resumeUrl is explicitly
provided (e.g., input.resumeUrl !== undefined), otherwise leave resume set to
member.resumeUrl without normalization/validation; update the logic around
normalizeOwnedResumeObjectName, input.resumeUrl, member.resumeUrl and
ctx.session.user.id so validation/normalization runs only for client-supplied
changes.
In `@packages/api/src/routers/resume.ts`:
- Around line 48-76: Current flow deletes all existing resume objects
(existingResumes via s3Client.listObjects and removeObject) before calling
s3Client.putObject for the new filePath, risking dangling resumeUrl if putObject
or subsequent DB writes (in member.ts / hackers/mutations.ts) fail; change the
sequence to first upload the new object with s3Client.putObject (using either a
temporary/new key), persist the new filePath to the DB, and only after the DB
save succeeds remove the old objects via s3Client.removeObject (or prefer a
stable per-user key so writes are atomic from the DB perspective), updating code
around existingResumes, putObject and removeObject to implement
upload-then-cleanup (or switch to a deterministic user-specific key).
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 67457aa8-c3e0-4a51-93be-2b8f7cf9b89b
📒 Files selected for processing (19)
apps/blade/src/app/_components/dashboard/hacker-dashboard/confirm-button.tsxapps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/bloomknights.tsapps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/index.tsapps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/khix.tsapps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/types.tsapps/blade/src/app/_components/dashboard/hacker/hacker-application-background.tsxapps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsxapps/bloomknights/src/app/_components/footer/footerContent.tsapps/bloomknights/src/app/_components/navbar/MLHBadge.tsxapps/bloomknights/src/app/_components/navbar/NavContent.tsxapps/bloomknights/src/app/_components/navbar/Navbar.tsxapps/bloomknights/src/app/_components/navbar/motion.tsapps/bloomknights/src/app/_components/partners/partnerLogos.tspackages/api/src/resume-security.tspackages/api/src/routers/guild.tspackages/api/src/routers/hackers/mutations.tspackages/api/src/routers/member.tspackages/api/src/routers/resume.tspackages/ui/src/form.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/bloomknights/src/app/_components/partners/partnerLogos.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/types.ts
- packages/api/src/routers/hackers/mutations.ts
- apps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/index.ts
- apps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsx
- apps/blade/src/app/_components/dashboard/hacker/hacker-application-background.tsx
Contributor
Author
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
2 participants
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.
Closes #457
Why
The hackathon application flow did not fit the experience we wanted hackers to have. This PR rebuilds the hacker application frontend into a sleek, one-step-at-a-time experience while keeping the existing backend submission flow.
It also adds officer-managed hackathon setup so each hackathon can have its own route, dates, and optional application background preset. Alongside that, this PR includes BloomKnights site polish and general cleanup from the branch.
What
Hacker Application Experience
Application Background System
KHIX Forest Walk Background
BloomKnights Application Background Preset
Officer Hackathon Management
Backend and Database Support
applicationBackgroundEnabled.applicationBackgroundKey.knight_hacks_hackathon.name.createHackersubmissions.Shared Validation
@forge/validators.Shared UI Improvements
value/defaultValue/onValueChangesupport toResponsiveComboBox.BloomKnights Site Polish
Test Plan
Ran:
pnpm formatpnpm lintpnpm typecheckpnpm lint:wspnpm lint:wscompleted successfully with one existing warning.What You Should Test
Local Setup
Pull the branch and run the app locally.
Make sure your local user has admin/officer access so you can reach the admin hackathon manager.
Go to:
Create a test hackathon:
bloomknightsBloomKnightsbloomknightsVisit:
Application Flow
Fill out the full application from start to submit.
Confirm that:
Backgrounds
BloomKnights Preset
Confirm that:
KHIX Preset
Confirm that:
Stock/Default Background
Confirm that:
Admin/API
Confirm that:
/hacker/application/[hackathon-name]loads the correct hackathon by route name.Warning
After merge/deploy, an officer/admin should create the BloomKnights hackathon in production through the new admin manager and enable the BloomKnights background preset.
Video
view others locally pls my computer is exploding
it prefilled my info from a last application (It pulls your last saved hacker profile from the DB for the logged in user only)
Screen.Recording.2026-05-31.at.7.19.22.PM.mp4
Summary by CodeRabbit
New Features
UI/UX Improvements
Bug Fixes