-
Notifications
You must be signed in to change notification settings - Fork 0
feat: hackathon stamps page #83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Visit the preview URL for this PR (updated for commit f0ad21f): https://dev-nwplus-admin--pr83-kez-stampbook-hvuwn59k.web.app (expires Sun, 18 Jan 2026 05:09:20 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c290bf9e2fac0401389f751f415cce4267517b51 |
naijwu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks amazing, LFG 🚀
Left some comments but only for minor details--should be good to go after! TY 🐐
| ); | ||
| }, | ||
| }), | ||
| columnHelper.accessor("isQRUnlockable", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for isQRUnlockable column, do you think it'd be clearer if the header is just "has QR?" or the cell content is a clear binary value?
I do like the visual variance of the QR icon instead of a boring checkmark, but in the end this is a boolean column so just having a checkmark (similar to criteria and isHidden) might be clearer?
src/lib/utils.ts
Outdated
| * Parse hackathon ID (e.g. "nwHacks2026", "cmd-f2021", "HackCamp2025") | ||
| * into base hackathon type/slug | ||
| */ | ||
| export function getHackathonType(hackathonId: string): "cmd-f" | "hackcamp" | "nwhacks" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could use HackathonType here
| export function getHackathonType(hackathonId: string): "cmd-f" | "hackcamp" | "nwhacks" { | |
| export function getHackathonType(hackathonId: string): HackathonType { |
| <div className="flex items-center gap-2"> | ||
| <Button type="button" variant="outline" className="w-full justify-between font-normal"> | ||
| <span> | ||
| {criteria.length > 0 ? ( | ||
| <span className="font-medium text-neutral-700"> | ||
| {criteria.length} condition{criteria.length !== 1 ? "s" : ""} set | ||
| </span> | ||
| ) : ( | ||
| <span className="text-muted-foreground">Add unlock criteria...</span> | ||
| )} | ||
| </span> | ||
| </Button> | ||
| {criteria.length > 0 && ( | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Badge variant="secondary" className="cursor-pointer"> | ||
| {criteria.length} | ||
| </Badge> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <div className="space-y-1"> | ||
| <p className="font-medium">Unlock Criteria:</p> | ||
| <div className="font-mono text-xs">{getFilterLogicDisplay()}</div> | ||
| </div> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <div className="flex items-center gap-2"> | |
| <Button type="button" variant="outline" className="w-full justify-between font-normal"> | |
| <span> | |
| {criteria.length > 0 ? ( | |
| <span className="font-medium text-neutral-700"> | |
| {criteria.length} condition{criteria.length !== 1 ? "s" : ""} set | |
| </span> | |
| ) : ( | |
| <span className="text-muted-foreground">Add unlock criteria...</span> | |
| )} | |
| </span> | |
| </Button> | |
| {criteria.length > 0 && ( | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Badge variant="secondary" className="cursor-pointer"> | |
| {criteria.length} | |
| </Badge> | |
| </TooltipTrigger> | |
| <TooltipContent> | |
| <div className="space-y-1"> | |
| <p className="font-medium">Unlock Criteria:</p> | |
| <div className="font-mono text-xs">{getFilterLogicDisplay()}</div> | |
| </div> | |
| </TooltipContent> | |
| </Tooltip> | |
| )} | |
| </div> | |
| <div className="relative flex items-center gap-2"> | |
| <Button type="button" variant="outline" className="w-full justify-between font-normal"> | |
| <span> | |
| {criteria.length > 0 ? ( | |
| <span className="font-medium text-neutral-700"> | |
| {criteria.length} condition{criteria.length !== 1 ? "s" : ""} set | |
| </span> | |
| ) : ( | |
| <span className="text-muted-foreground">Add unlock criteria...</span> | |
| )} | |
| </span> | |
| </Button> | |
| <div className="absolute right-2"> | |
| {criteria.length > 0 && ( | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <Badge variant="secondary" className="cursor-pointer"> | |
| {criteria.length} | |
| </Badge> | |
| </TooltipTrigger> | |
| <TooltipContent> | |
| <div className="space-y-1"> | |
| <p className="font-medium">Unlock Criteria:</p> | |
| <div className="font-mono text-xs">{getFilterLogicDisplay()}</div> | |
| </div> | |
| </TooltipContent> | |
| </Tooltip> | |
| )} | |
| </div> | |
| </div> |
This suggestion positions the criteria badge to be within the popover trigger button, in order to prevent unwanted parent container x-overflow. Only changes 3 lines: lines 137 and the lines surrounding 150 and 163; makes the parent relative positioned and wraps the Badge component to be absolute positioned, right-2 of the parent div.
| before | after |
|---|---|
![]() |
![]() |
martincai8
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it lgtjwu it lgtm thank you legends
|
Oh I forgot to squash... mbs |


Reason
Explanation