feat: practical donate UX, profile popover readability, and self-host flow#22
feat: practical donate UX, profile popover readability, and self-host flow#22rohan-patnaik merged 10 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughRemoves the PREMIUM tier and adsFree flag; replaces premium gating with global pooled limits and global usage; adds theme context/toggle and theme init script; introduces a DonateBookmark (UPI/QR) component and env vars; large UI/design and CSS overhaul. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client (Browser)
participant UI as Web UI
participant Convex as Convex Backend
participant DB as Counters DB
Browser->>UI: open Usage page or submit job
UI->>Convex: capacity:getUsageSnapshot()
Convex->>DB: read per-user usage counters
Convex->>DB: read global/pool counters
Convex->>Convex: resolveGlobalLimits()
Convex-->>UI: return UsageSnapshot (user usage + globalLimits + globalUsage)
UI-->>Browser: render usage, pool metrics, and enforce submission limits
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@apps/web/.env.example`:
- Around line 26-29: The environment variable NEXT_PUBLIC_DONATE_UPI_NOTE
contains a space and should be quoted to avoid dotenv parsers truncating the
value; update the .env.example entry for NEXT_PUBLIC_DONATE_UPI_NOTE to wrap the
value in quotes (e.g., "Support ZenPDF") so parsers treat the entire string as
the value.
In `@apps/web/src/app/globals.css`:
- Around line 441-443: The dark-mode rule for .paper-button--danger only sets
color; update :root[data-theme="dark"] .paper-button--danger to also override
background and border-color to the same values used in the light-mode
`.paper-button--danger` rule so the button uses the intended light-pink
background and red border in dark theme; locate the dark selector and add
background and border-color properties matching the light-mode declarations in
the existing CSS.
In `@apps/web/src/app/page.tsx`:
- Around line 157-165: Replace the hardcoded "27 available" string in the
status-pill span with a computed value derived from your tools source of truth:
use the length/sum of items in toolShelves (or a shared tools registry constant)
to calculate the available count and render that value (e.g., `${count}
available`) inside the <span className="status-pill">; update the render logic
where the JSX block with className "paper-card" and the span with className
"status-pill" is defined so the displayed number stays in sync with toolShelves
or the chosen registry.
In `@apps/web/src/app/tools/page.tsx`:
- Around line 1033-1046: The button currently strips the browser focus ring via
the focus-visible:outline-none utility on the tool-card button (the element
using selectTool and aria-pressed with activeTool), leaving keyboard users
without any visible focus; remove that utility or replace it with accessible
focus styles (for example add focus-visible:ring-2,
focus-visible:ring-forest-600 and focus-visible:ring-offset-2 or another
clearly-contrasting focus-visible style) on the same button className so
keyboard focus is visibly indicated while preserving the rest of the tool-card
styling.
In `@apps/web/src/components/DonateBookmark.tsx`:
- Around line 93-103: The component leaks internal env var names to users by
rendering raw NEXT_PUBLIC_DONATE_* strings when not set; update the
DonateBookmark.tsx rendering logic around upiId (and the QR branch that uses
NEXT_PUBLIC_DONATE_UPI_QR_URL) so that if upiId is falsy you either hide the
donate UI entirely (remove the QR/image and donate button) or show a
non-developer-facing fallback like "Donations not configured" / "Donations
unavailable", and ensure you do not render the literal env var names (e.g.,
replace occurrences of {upiId || "Set NEXT_PUBLIC_DONATE_UPI_ID"} with the
generic fallback and conditionally render the QR block based on existence of
upiId/qrUrl).
- Around line 83-91: DonateBookmark.tsx renders an external QR image via the
Image component using NEXT_PUBLIC_DONATE_UPI_QR_URL; add the corresponding
domain (or a remotePatterns entry) to next.config.ts under images.remotePatterns
so Next.js allows that external URL at runtime. Update next.config.ts to include
the QR image host (or pattern) in images.remotePatterns (or images.domains) so
the Image in DonateBookmark.tsx can load qrUrl without 400 errors.
- Around line 56-61: The modal rendered by the DonateBookmark component lacks
keyboard and backdrop dismissal; add an Escape-key handler and backdrop
click-to-close: in DonateBookmark, when open is true register a useEffect that
listens for keydown and calls the component's close handler (e.g., onClose or
setOpen(false)) when event.key === 'Escape'; also add an onClick on the backdrop
element (the div with role="dialog" / surrounding backdrop) that calls the same
close handler and ensure the inner dialog panel stops propagation (e.g.,
onClick={e => e.stopPropagation()}) so clicks inside the dialog don't close it.
In `@apps/web/src/components/ThemeModeProvider.tsx`:
- Around line 17-22: getInitialTheme currently only reads
document.documentElement.dataset.theme so persisted preferences saved by
applyTheme are ignored; update getInitialTheme to first check localStorage for
the same key applyTheme writes (use the exact storage key used by applyTheme),
return that value ("dark" or "light") if present, and fall back to checking
document.documentElement.dataset.theme and then "light" if neither is set;
ensure you reference getInitialTheme and applyTheme so the storage key stays
consistent.
🧹 Nitpick comments (14)
apps/web/src/app/page.tsx (2)
73-92: Two of the three route cards link to the same page.Both "Check limits first" (line 83) and "Need unlimited usage" (line 89) resolve to
/usage-capacity. Users clicking two visually distinct CTAs and landing on the identical page may feel disoriented. Consider either linking the self-host card to a distinct anchor (e.g./usage-capacity#self-host) or to the repository/README, so each card has a unique destination.Suggested diff
{ title: "Need unlimited usage", description: "Run ZenPDF locally when you need full control and no cloud caps.", - href: "/usage-capacity", + href: "/usage-capacity#self-host", cta: "See self-host path", },
244-254: Step badge format is fine for the current data but fragile.
0{index + 1}produces"01","02","03"— correct for ≤ 9 steps. If the array ever grows past 9 items, badges would read"010". ApadStartcall would be more resilient, though this is purely a nitpick given the fixed 3-step array.apps/web/src/app/tools/page.tsx (3)
943-953: String-prefix status classification is fragile.
statusClassNamedetermines error/success styling by matching prefixes of the human-readablestatusstring. If any message wording changes (e.g.,"Unable"→"Failed"), the alert silently loses its error styling. This also forces every futuresetStatuscall site to be aware of these prefix conventions.Consider introducing a separate piece of state for the status kind:
♻️ Suggested approach
- const [status, setStatus] = useState<string | null>(null); + const [status, setStatus] = useState<{ message: string; kind: "error" | "success" | "info" } | null>(null);Then set it explicitly at each call site, e.g.:
setStatus({ message: "Upload exactly two PDFs to compare.", kind: "error" }); setStatus({ message: "Job queued. Results will appear below.", kind: "success" });And derive the class directly:
const statusClassName = status === null ? "" : status.kind === "error" ? "alert alert--error" : status.kind === "success" ? "alert alert--success" : "alert";
970-981: Hardcodedangleconstraints leak domain knowledge into a generic renderer.Lines 974–976 special-case
field.key === "angle"to setmin/max/step. This couplesrenderFieldto a specific tool's field. You can keep the renderer generic by extendingToolField:♻️ Suggested approach
type ToolField = { key: string; label: string; placeholder: string; helper?: string; type?: "text" | "number" | "password" | "textarea"; required?: boolean; + min?: number; + max?: number; + step?: number; };Then in the "Rotate PDF" field definition:
{ key: "angle", label: "Angle", placeholder: "90", helper: "Use 90, 180, or 270.", type: "number", min: 90, max: 270, step: 90 },And in
renderField:- min={field.key === "angle" ? 90 : undefined} - max={field.key === "angle" ? 270 : undefined} - step={field.key === "angle" ? 90 : undefined} + min={field.min} + max={field.max} + step={field.step}
685-1267: Consider extracting sub-modules to reduce file size.At ~1267 lines this file carries tool definitions, constants, helper utilities, a sub-component (
JobCard), and the full page component. ExtractingTOOLS/TOOL_GROUPSinto atools-data.tsmodule andJobCardinto its own file would improve navigability without changing behavior — but this can be deferred.apps/web/convex/lib/limits.ts (2)
3-3: Duplicate type definitions across server and client.
PlanTierandGlobalLimitsare defined identically here and inapps/web/src/lib/limits.ts. This is a common pattern in Convex projects (separate server/client boundaries), but if possible, consider a shared types module to avoid future drift.#!/bin/bash # Verify the duplicate type definitions echo "=== convex/lib/limits.ts PlanTier ===" rg -n "export type PlanTier" --type=ts echo "" echo "=== GlobalLimits definitions ===" rg -n "export type GlobalLimits" --type=tsAlso applies to: 13-20
39-46: Global daily-minutes cap of 120 is fairly tight.With PREMIUM removed, all users share this 120-minute global daily pool. If heavy tools (OCR, PDF/A) consume significant per-job minutes, this cap could be reached quickly during modest concurrent usage. Ensure monitoring/alerting is in place to detect when this limit is hit and consider whether the cap should be tunable at runtime (it already is via env override, which is good).
apps/web/src/app/usage-capacity/page.tsx (2)
308-313: "Pool concurrency cap" bar always renders at 100% — consider a different presentation.Both
valueandlimitareglobalLimits.maxConcurrentJobs, so the bar will always be completely full. Unlike the other bars that show usage against a ceiling, this one just displays a static configuration value. The filled bar could mislead users into thinking concurrency is maxed out.A simple info row or the
badgeprop might convey this more clearly — e.g.badge={String(globalLimits.maxConcurrentJobs)}with alimitof some reasonable upper bound, or just render it as static text outside ofUsageBar.
170-183: Default fallback values of0produce confusing pool metrics before the snapshot loads.When
snapshotisundefined,globalLimits.maxJobsPerDayandmaxDailyMinutesare0, so the pool section renders "0 of 0 used · 0 remaining". The "Loading latest snapshot…" pill on Line 230 partially mitigates this, but the pool section still shows zero-denominator metrics. Consider either hiding the pool section while!snapshot, or using more meaningful sentinel defaults.apps/web/src/app/sign-up/[[...sign-up]]/page.tsx (1)
9-17: Redundant appearance override — already set byClerkProvider.
providers.tsxalready configurescolorPrimary,colorBackground, andcolorTextwith identical values on the wrappingClerkProvider. The per-componentappearancehere is fully redundant and can be removed to keep a single source of truth.Proposed simplification
- <SignUp - appearance={{ - variables: { - colorPrimary: "rgb(var(--forest-600))", - colorBackground: "rgb(var(--paper-50))", - colorText: "rgb(var(--ink-900))", - }, - }} - /> + <SignUp />apps/web/src/components/DonateBookmark.tsx (1)
30-43: Timeout not cleared on unmount.If the component unmounts within the 2-second window after a copy,
setTimeoutwill firesetCopyStateon an unmounted component. While React 18+ tolerates this, it's a minor leak. AuseRef-based cleanup oruseEffectcleanup would be cleaner, but this is low priority.apps/web/src/app/layout.tsx (1)
57-61:DonateBookmarkis always rendered, even when donation is not configured.When
NEXT_PUBLIC_DONATE_UPI_IDis empty, the fixed "Support" button still appears and opens a modal showing raw env-var names. Consider conditionally rendering<DonateBookmark />only when the UPI ID is set, or guarding inside the component itself (the root cause is inDonateBookmark.tsx, flagged there).apps/web/convex/capacity.ts (1)
10-29: Consider parallelizing the three independent calls ingetCapacitySnapshot.
resolveBudgetState,resolveGlobalLimits, andresolveGlobalUsageCounterare independent of each other, yet they're awaited sequentially. This is already done correctly withPromise.allingetUsageSnapshotbelow.♻️ Proposed parallel fetch
export const getCapacitySnapshot = query({ args: {}, handler: async (ctx) => { const now = Date.now(); - const budget = await resolveBudgetState(ctx, now); - const limits = await resolveGlobalLimits(ctx); - const { counter, periodStart } = await resolveGlobalUsageCounter(ctx, now); + const [budget, limits, { counter, periodStart }] = await Promise.all([ + resolveBudgetState(ctx, now), + resolveGlobalLimits(ctx), + resolveGlobalUsageCounter(ctx, now), + ]); return { budget, limits, usage: counter ?? { periodStart, jobsUsed: 0, minutesUsed: 0, bytesProcessed: 0, }, }; }, });apps/web/src/lib/convex.ts (1)
4-4:GlobalLimits,PlanLimits, andPlanTierare duplicated across client and server boundaries.These types are defined identically in both
apps/web/src/lib/limits.tsandapps/web/convex/lib/limits.ts. If either copy drifts, the client type will silently mismatch the actual Convex return shape at runtime.Consider re-exporting from a single shared source (e.g.,
convex/lib/limits.ts→ re-exported via a shared barrel), or adding a compile-time assertion that both shapes are assignable to each other.
| <button | ||
| key={item.id} | ||
| type="button" | ||
| onClick={() => selectTool(item.id)} | ||
| aria-pressed={item.id === activeTool} | ||
| className={`tool-card w-full rounded-xl border p-3 text-left transition focus-visible:outline-none ${ | ||
| item.id === activeTool | ||
| ? "border-forest-600 bg-sage-200/45" | ||
| : "border-paper-200 bg-paper-50 hover:border-forest-500/50" | ||
| }`} | ||
| > | ||
| <h3 className="text-sm font-semibold text-ink-900">{item.label}</h3> | ||
| <p className="mt-1 text-xs text-ink-500">{item.description}</p> | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the file exists and verify the code snippet
head -1050 apps/web/src/app/tools/page.tsx | tail -30Repository: rohan-patnaik/ZenPDF
Length of output: 1620
🏁 Script executed:
#!/bin/bash
# Search for the tool-card CSS class definition
fd . --type f \( -name "*.css" -o -name "*.scss" -o -name "*.module.css" -o -name "*.module.scss" \) | xargs rg "tool-card" -nRepository: rohan-patnaik/ZenPDF
Length of output: 572
🏁 Script executed:
#!/bin/bash
# Also check for tool-card in any TSX/TS files that might contain CSS-in-JS
rg "tool-card" apps/web/src --type ts --type tsx -nRepository: rohan-patnaik/ZenPDF
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Read the tool-card CSS class definition from globals.css
sed -n '236,260p' apps/web/src/app/globals.cssRepository: rohan-patnaik/ZenPDF
Length of output: 529
focus-visible:outline-none removes the keyboard focus indicator without a visible replacement.
Line 1038 suppresses the browser's default focus ring. The tool-card CSS class and inline Tailwind classes provide no alternative focus style, leaving keyboard users without a visible indicator of which button is focused.
🤖 Prompt for AI Agents
In `@apps/web/src/app/tools/page.tsx` around lines 1033 - 1046, The button
currently strips the browser focus ring via the focus-visible:outline-none
utility on the tool-card button (the element using selectTool and aria-pressed
with activeTool), leaving keyboard users without any visible focus; remove that
utility or replace it with accessible focus styles (for example add
focus-visible:ring-2, focus-visible:ring-forest-600 and
focus-visible:ring-offset-2 or another clearly-contrasting focus-visible style)
on the same button className so keyboard focus is visibly indicated while
preserving the rest of the tool-card styling.
| {qrUrl ? ( | ||
| <div className="surface-muted mt-4 p-3"> | ||
| <Image | ||
| src={qrUrl} | ||
| alt="UPI QR code" | ||
| width={224} | ||
| height={224} | ||
| className="mx-auto h-56 w-56 rounded-md object-contain" | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and display next.config file
find . -name "next.config.*" -type f -exec echo "File: {}" \; -exec cat {} \;Repository: rohan-patnaik/ZenPDF
Length of output: 374
🏁 Script executed:
#!/bin/bash
# Search for NEXT_PUBLIC_DONATE_UPI_QR_URL in codebase
rg 'NEXT_PUBLIC_DONATE_UPI_QR_URL' -A 2 -B 2
# Also check for .env files
echo "=== .env files ==="
find . -name ".env*" -type f | head -20Repository: rohan-patnaik/ZenPDF
Length of output: 1334
🏁 Script executed:
#!/bin/bash
# Read the DonateBookmark.tsx file
cat -n apps/web/src/components/DonateBookmark.tsxRepository: rohan-patnaik/ZenPDF
Length of output: 4931
Add images.remotePatterns configuration to next.config.ts for the QR image domain.
The Image component in DonateBookmark.tsx (line 86) uses src={qrUrl} from NEXT_PUBLIC_DONATE_UPI_QR_URL, which can be an external URL. Next.js requires all external image domains to be listed in images.remotePatterns (or the legacy images.domains) in next.config.ts. Without this configuration, the image will fail at runtime with a 400 error when the environment variable is set to an external URL.
🤖 Prompt for AI Agents
In `@apps/web/src/components/DonateBookmark.tsx` around lines 83 - 91,
DonateBookmark.tsx renders an external QR image via the Image component using
NEXT_PUBLIC_DONATE_UPI_QR_URL; add the corresponding domain (or a remotePatterns
entry) to next.config.ts under images.remotePatterns so Next.js allows that
external URL at runtime. Update next.config.ts to include the QR image host (or
pattern) in images.remotePatterns (or images.domains) so the Image in
DonateBookmark.tsx can load qrUrl without 400 errors.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/app/globals.css`:
- Around line 502-512: The responsive rule makes .paper-button--ghost width:100%
on small screens and unintentionally stretches the icon .theme-toggle (which
relies on width:2rem); inside the existing `@media` (max-width: 640px) block
override the width for the theme toggle by adding a more specific selector such
as .theme-toggle.paper-button--ghost (or .theme-toggle) and reset its width back
to 2rem (or width:auto) so the icon button keeps its intended fixed size while
other .paper-button variants remain full-width.
🧹 Nitpick comments (1)
apps/web/src/app/globals.css (1)
114-128: Button variants as standalone classes — intentional but worth a brief note.
.paper-button--ghostand.paper-button--dangerare treated as independent classes rather than BEM modifiers applied alongside.paper-button. This works because Lines 114-128 share the base styles across all three selectors. Just be aware that any future base-only change (e.g., adding a new shared property to.paper-buttonalone) will silently skip the other two variants.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/app/globals.css`:
- Around line 165-173: The .paper-button--danger rule uses hardcoded rgb(...)
values which prevent dark-mode token swapping; update the background
declarations to use the CSS color tokens instead (use rgb(var(--rose-100)) for
the default background and rgb(var(--rose-200)) for the :hover background) in
the .paper-button--danger and .paper-button--danger:hover selectors so dark-mode
token values are applied automatically and you can remove/avoid explicit
dark-mode overrides.
🧹 Nitpick comments (3)
apps/web/src/components/DonateBookmark.tsx (2)
43-52: Fallback QR generation via quickchart.io is an uncontrolled external dependency.When
NEXT_PUBLIC_DONATE_UPI_QR_URLis not set, the component silently callsquickchart.ioto generate a QR code at runtime. This introduces an implicit third-party dependency that:
- May have rate limits or availability issues, causing a broken image in production.
- Sends the full UPI URI (containing the payee ID/name) to an external service.
Consider documenting this fallback behaviour (e.g., in the
.env.exampleor a comment) so deployers are aware, or generate the QR client-side using a library likeqrcode.react.
13-14: Hardcoded personal UPI credentials as defaults.The fallback values contain a real personal UPI ID and name. Anyone who forks the repo and forgets to set the env vars will unknowingly direct donations to the original author. This may be intentional, but it's unusual for open-source defaults — typically the fallback would be empty or clearly placeholder (e.g.,
yourname@upi), with the real values only in.env.apps/web/src/app/globals.css (1)
472-484: Dark-mode primary button flips to an outlined/ghost style — verify this is intentional.In light mode,
.paper-buttonis a filled green button (background: rgb(var(--forest-600)), white text). In dark mode it becomes a dark surface (background: rgb(var(--paper-100))) with green text — essentially ghost-like. This means primary and ghost buttons look very similar in dark mode (both are dark surfaces with green accents, differing mainly in border treatment).If the intent is to maintain a distinct filled CTA in dark mode, consider keeping a green (or slightly muted green) background instead.
rohan-patnaik
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/web/src/app/globals.css`:
- Around line 43-45: Dark-mode hover for .donate-fab is invisible because
--forest-700, --forest-600, and --forest-500 all resolve to the same value;
update the dark-mode token values so the hover color differs or change the hover
rule to use a darker token/adjust box-shadow. Specifically, modify the dark-mode
CSS variable definitions for --forest-700 (or --forest-600) to a slightly darker
RGB than 63 207 142, or alter the .donate-fab dark override hover rule to
reference a distinct variable (e.g., --forest-700-dark) or increase box-shadow
contrast so .donate-fab:hover produces a visible background change.
In `@apps/web/src/app/page.tsx`:
- Around line 87-92: The object with title "Need unlimited usage" currently
links to href "/usage-capacity" but the CTA and title indicate it should point
to the self-host guide; update that object's href value to "/self-host" (locate
the object literal in apps/web/src/app/page.tsx with title "Need unlimited
usage") so the CTA "See self-host path" and title navigate to the dedicated
self-hosting page.
In `@apps/web/src/components/DonateBookmark.tsx`:
- Around line 20-21: The component currently hardcodes personal payment details
in defaultUpiName and defaultUpiId; remove those literal defaults and instead
read values from environment/config (e.g., process.env or a passed prop) and
change the rendering logic in DonateBookmark (or its containing component) so
the Donate button is only shown when both UPI name and UPI id are explicitly
provided; if the env/config vars are missing, do not render the button (or
render a neutral disabled/placeholder message) so forks won't silently route
donations to the original author.
- Around line 362-368: The iframe in DonateBookmark.tsx currently sets
allow="payment *; clipboard-write", which grants the Payment Request API to any
origin; update the iframe usage (the element rendering resolvedCardEmbedUrl) to
scope permissions to the specific embed origin (e.g., allow="payment
https://embed.example.com; clipboard-write") or remove the permissive allow
attribute entirely so permissions default to the iframe origin; ensure the
change targets the iframe rendering resolvedCardEmbedUrl and adjust the origin
string to the actual payment provider domain.
🧹 Nitpick comments (5)
apps/web/src/app/page.tsx (1)
245-249: Step numbering format will break for 10+ items.
0{index + 1}produces"01","02", etc. but would give"010"for a tenth step. With only 3 steps this is fine, but apadStartapproach would be more robust:{String(index + 1).padStart(2, "0")}Low priority given the fixed step count.
apps/web/src/app/self-host/page.tsx (1)
49-52: URLs rendered as shell commands look misleading.Lines 50–51 contain URLs (
http://localhost:3000,https://dashboard.convex.dev) but they're in thecommandsarray, which renders inside<pre><code>. This makes them look like terminal commands rather than informational links.Consider either making them clickable
<a>elements or adding a separate field (e.g.,links) to theSetupSteptype for non-command references.apps/web/src/app/usage-capacity/page.tsx (1)
308-313: "Pool concurrency cap" bar is always at 100% — it shows a static limit, not usage.Both
valueandlimitareglobalLimits.maxConcurrentJobs, so the bar is perpetually full. Unlike the other UsageBars that display consumed vs allowed, this one just displays a configuration value.Consider either showing actual concurrent jobs in-flight (if that data is available in the snapshot) or rendering this as a simple label/badge instead of a progress bar:
- <UsageBar - label="Pool concurrency cap" - value={globalLimits.maxConcurrentJobs} - limit={globalLimits.maxConcurrentJobs} - helper={`${globalLimits.maxConcurrentJobs} jobs can run concurrently`} - /> + <div className="paper-card p-5"> + <div className="flex items-start justify-between gap-4"> + <div> + <p className="text-sm font-semibold text-ink-900">Pool concurrency cap</p> + <p className="mt-1 text-xs text-ink-500">{globalLimits.maxConcurrentJobs} jobs can run concurrently</p> + </div> + <span className="status-pill">{globalLimits.maxConcurrentJobs}</span> + </div> + </div>apps/web/src/app/globals.css (2)
304-308: Consider extracting warning/amber color tokens.The
.status-pill--warninglight-mode rule (Line 305, 307) and dark override (Line 664-665) use hardcoded amber/yellow RGB values (rgb(202 138 4 / 0.35),rgb(133 77 14),rgb(251 191 36 / 0.45),rgb(253 230 138)) rather than CSS custom properties. Unlike the green and rose palettes which have tokens, there are no--amber-*or--warn-*tokens. Adding them would improve consistency and make future dark-mode adjustments a single-point change, matching the approach used for--rose-100,--gold-200, and--forest-*.Also applies to: 663-666
507-511: Dark-mode body gradient uses a differentcircle atorigin than light mode.Light mode (Line 70) uses
circle at top right, while dark mode (Line 509) usescircle at top center. This is likely intentional for aesthetic reasons, but worth confirming the asymmetry is deliberate — it will cause a noticeable layout shift during theme transitions if animated.
| const defaultUpiName = "Rohan Patnaik"; | ||
| const defaultUpiId = "rohanpatnaik1997-1@okhdfcbank"; |
There was a problem hiding this comment.
Hardcoded personal payment details as defaults.
defaultUpiName and defaultUpiId contain the developer's real name and bank-linked UPI ID. Anyone deploying this open-source project without setting env vars will silently route donations to the original author. This is presumably intentional for the hosted instance, but for forks it's a subtle gotcha.
Consider guarding: if neither env var is set, hide the donate button entirely rather than silently defaulting to a specific person's account.
🤖 Prompt for AI Agents
In `@apps/web/src/components/DonateBookmark.tsx` around lines 20 - 21, The
component currently hardcodes personal payment details in defaultUpiName and
defaultUpiId; remove those literal defaults and instead read values from
environment/config (e.g., process.env or a passed prop) and change the rendering
logic in DonateBookmark (or its containing component) so the Donate button is
only shown when both UPI name and UPI id are explicitly provided; if the
env/config vars are missing, do not render the button (or render a neutral
disabled/placeholder message) so forks won't silently route donations to the
original author.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/app/api/download/route.ts`:
- Around line 41-44: The route currently sets disableAuth using the raw request
header which is spoofable; update the disableAuth computation in route.ts so the
header-based bypass is only honored when the env guard is true (i.e.
process.env.ZENPDF_DISABLE_AUTH === "1" && process.env.NODE_ENV !==
"production") before reading request.headers.get("x-zenpdf-auth-bypassed"), or
else rely solely on shouldBypassAuth() from the middleware; specifically modify
the disableAuth assignment (and any logic that sets allowAnonAccess) to require
the same environment check before trusting the "x-zenpdf-auth-bypassed" header.
In `@apps/web/src/proxy.ts`:
- Around line 22-30: The request header x-zenpdf-auth-bypassed is spoofable by
clients; in proxy() when shouldBypassAuth() is false (Clerk branch) remove/strip
that header from incoming requests before calling NextResponse.next to prevent
spoofing, and in the downstream download route handler (the function that reads
the header in route.ts) change the logic to only honor x-zenpdf-auth-bypassed
when shouldBypassAuth() returns true (i.e., gate acceptance of that header
behind the same environment check), ensuring both defense-in-depth in proxy()
and the authoritative check in the download route handler.
🧹 Nitpick comments (2)
docs/OWNER_ACTION_ITEMS.md (1)
42-42: Minor grammar nit: use a hyphen in "custom-hosted".Static analysis flagged this compound adjective.
Proposed fix
-- [ ] (Optional) Use a custom hosted QR image instead of generated QR. +- [ ] (Optional) Use a custom-hosted QR image instead of generated QR.apps/web/src/components/DonateBookmark.tsx (1)
8-19: Consider reducing state count with a reducer or a state machine.The component manages 7
useStatehooks plus 3 refs. Several of the states are tightly coupled (e.g.,showQr,showCardCheckout,activePaymentActionare always set together inhandleOpen,handleClose, and the button handlers). A singleuseReduceror a discriminated union state would reduce the risk of inconsistent state combinations and simplify the handlers.This is optional and can be deferred.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/proxy.ts`:
- Around line 22-24: Create a new middleware entry file that re-exports the
existing middleware function and its matcher so Next.js will load it: add a
`middleware.ts` in the project's src root that imports `proxy` and `config` from
the existing `proxy.ts` and re-exports `proxy` as the default export and
`config` as a named export (i.e., export `{ proxy as default, config } from
"./proxy"`), ensuring the middleware entrypoint is recognized and Clerk
auth/header sanitization runs.
🧹 Nitpick comments (4)
apps/web/src/app/page.tsx (1)
245-255: Step number format assumes ≤ 9 steps.
0{index + 1}renders "01", "02", "03" — fine for three steps. Ifstepsever grows past 9 items, this would produce "010", "011", etc. A trivial guard withString(index + 1).padStart(2, "0")would be more robust, but with only three steps this is purely cosmetic.Optional: future-proof step numbering
- <div className="flex h-8 w-8 items-center justify-center rounded-lg border border-paper-300 bg-paper-100 text-xs font-semibold text-forest-700"> - 0{index + 1} + <div className="flex h-8 w-8 items-center justify-center rounded-lg border border-paper-300 bg-paper-100 text-xs font-semibold text-forest-700"> + {String(index + 1).padStart(2, "0")}apps/web/src/app/api/download/route.ts (1)
88-89: Consider RFC 5987 encoding for non-ASCII filenames.
sanitizeFilenamestrips dangerous characters, but if a user uploads a file with non-ASCII characters (e.g., Unicode), the barefilename="…"inContent-Dispositionmay not render correctly in all browsers. Addingfilename*=UTF-8''…as a fallback would improve compatibility.Proposed fix
- headers.set("Content-Disposition", `attachment; filename="${sanitizeFilename(filename)}"`); + const safe = sanitizeFilename(filename); + headers.set( + "Content-Disposition", + `attachment; filename="${safe}"; filename*=UTF-8''${encodeURIComponent(safe)}`, + );apps/web/src/components/DonateBookmark.tsx (1)
8-11: Hide the donate FAB entirely when no donation method is configured.When both
upiIdandcardEmbedUrlare empty (i.e., the deployer hasn't configured any donation method), the component still renders a floating button that opens a non-functional modal. An early return would avoid confusing end users.Proposed fix
export default function DonateBookmark() { const [open, setOpen] = useState(false); const [showQr, setShowQr] = useState(true); const [showCardCheckout, setShowCardCheckout] = useState(false); + + // --- env reads (hoisted before early return so hooks above are always called) --- + const upiId = (process.env.NEXT_PUBLIC_DONATE_UPI_ID ?? "").trim(); + const cardEmbedUrl = (process.env.NEXT_PUBLIC_DONATE_CARD_EMBED_URL ?? "").trim(); + + if (!upiId && !cardEmbedUrl) { + return null; + }Note: hooks must remain above the early return to satisfy the Rules of Hooks.
apps/web/src/app/tools/page.tsx (1)
949-959: Status styling relies on fragile string-prefix matching.
statusClassNamechecks whetherstatusstarts with specific substrings like"Unable","Enter","Upload exactly", etc. Any future copy change silently breaks the styling. Consider tracking a status type alongside the message.Proposed approach
- const [status, setStatus] = useState<string | null>(null); + const [status, setStatus] = useState<{ message: string; tone: "error" | "success" | "info" } | null>(null);Then set it as:
setStatus({ message: "Add at least one file to continue.", tone: "error" }); setStatus({ message: "Job queued. Results will appear below.", tone: "success" });And derive className directly from
status.tone.
| export function proxy(request: Request, event: NextFetchEvent) { | ||
| const nextRequest = | ||
| request instanceof NextRequest ? request : new NextRequest(request); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if middleware.ts exists and how it relates to proxy.ts
echo "=== Looking for middleware.ts ==="
fd -t f 'middleware\.(ts|js)' apps/web/src/
echo ""
echo "=== Contents of middleware.ts (if found) ==="
fd -t f 'middleware\.(ts|js)' apps/web/src/ --exec cat {}
echo ""
echo "=== Check if proxy.ts is imported anywhere ==="
rg -n "proxy" apps/web/src/middleware.ts 2>/dev/null || echo "No middleware.ts found or no proxy import"
echo ""
echo "=== Check next.config for middleware config ==="
fd -t f 'next.config\.(ts|js|mjs)' apps/web/ --exec cat {}Repository: rohan-patnaik/ZenPDF
Length of output: 561
🏁 Script executed:
cat -n apps/web/src/proxy.tsRepository: rohan-patnaik/ZenPDF
Length of output: 1720
Create apps/web/src/middleware.ts to export proxy and config from proxy.ts.
Next.js middleware requires an entry point at middleware.ts (or middleware.js) in the project root or src/ root. The proxy.ts file contains complete middleware logic (Clerk auth protection, header sanitization) and a config.matcher export, but without a middleware.ts that imports and re-exports both proxy and config, Next.js will not recognize this as middleware and none of the code will execute. Clerk auth protection and header sanitization will silently stop working.
Create apps/web/src/middleware.ts:
export { proxy as default, config } from "./proxy";🤖 Prompt for AI Agents
In `@apps/web/src/proxy.ts` around lines 22 - 24, Create a new middleware entry
file that re-exports the existing middleware function and its matcher so Next.js
will load it: add a `middleware.ts` in the project's src root that imports
`proxy` and `config` from the existing `proxy.ts` and re-exports `proxy` as the
default export and `config` as a named export (i.e., export `{ proxy as default,
config } from "./proxy"`), ensuring the middleware entrypoint is recognized and
Clerk auth/header sanitization runs.
1 participant
Summary
Open UPI AppShow UPI QR,Open UPI App,Pay by card)NEXT_PUBLIC_DONATE_UPI_ID,NEXT_PUBLIC_DONATE_PAYEE_NAME, legacy name key fallback)middleware.tstoproxy.tsValidation
cd apps/web && npm run lintcd apps/web && npx tsc --noEmitcd apps/web && npm testcd apps/web && npm run buildcd apps/worker && python3.11 -m compileall zenpdf_workerCodeRabbit
coderabbit review --plain --type uncommitted --base mainproxy.ts+ download route guard pathScope
Summary by CodeRabbit
New Features
Changes
Documentation