Skip to content

Conversation

@jasukej
Copy link
Contributor

@jasukej jasukej commented Jan 4, 2026

Reason

  • Allow admins to add, view, and modify stamps for the portal stampbook feature.

Explanation

  • CRUD functionality for hackathon stamps.
  • Define how stamps should be unlocked by hackers (firebase query vs. qr code) as detailed here + other criteria such as user-facing visibility
Screenshot 2026-01-04 at 8 30 37 AM Screenshot 2026-01-04 at 8 28 43 AM

@jasukej jasukej requested a review from martincai8 January 4, 2026 16:31
@jasukej jasukej changed the title feat: add hackathon stamps page feat: hackathon stamps page Jan 4, 2026
@github-actions
Copy link

github-actions bot commented Jan 4, 2026

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

@jasukej jasukej requested a review from naijwu January 4, 2026 16:32
Copy link
Contributor

@naijwu naijwu left a 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", {
Copy link
Contributor

@naijwu naijwu Jan 6, 2026

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" {
Copy link
Contributor

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

Suggested change
export function getHackathonType(hackathonId: string): "cmd-f" | "hackcamp" | "nwhacks" {
export function getHackathonType(hackathonId: string): HackathonType {

Comment on lines 137 to 164
<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>
Copy link
Contributor

@naijwu naijwu Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<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
Screenshot 2026-01-06 at 10 58 15 AM Screenshot 2026-01-06 at 10 57 39 AM

Copy link
Contributor

@martincai8 martincai8 left a 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

@jasukej jasukej merged commit 65cc695 into dev Jan 8, 2026
3 checks passed
@jasukej
Copy link
Contributor Author

jasukej commented Jan 8, 2026

Oh I forgot to squash... mbs

@jasukej jasukej deleted the kez/stampbook branch January 8, 2026 05:53
@jasukej jasukej mentioned this pull request Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants