This repository was archived by the owner on May 19, 2026. It is now read-only.
refactor(ui): improve ui, ux, styling and responsiveness #143
Merged
Conversation
- Increate content density - Updated PendingOrderCard.svelte to enhance layout and typography for better mobile experience. - Adjusted padding and margins in customer event pages for consistent spacing across devices. - Introduced EventTimeline component to encapsulate event itinerary rendering logic. - Improved accessibility and visual hierarchy in ticket history and pending orders sections. - Hide "Continue to seat selection" button when it reaches footer.
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR refreshes theme tokens and global CSS, centralizes logout and redirect handling, adds animated/decorative components (theme toggler, dot/grid patterns, number ticker), introduces several new pages (about, careers, contact, FAQ, privacy), refactors many UI components/layouts, improves touch interactions (pan/pinch, swipe-to-dismiss), and hardens the contact action with validation and rate limiting. ChangesTheme System, Configuration & Authentication
Component Library & Animations
New Customer Pages & Content
Event Pages & Timeline
Account & Order Pages
Navigation, Layout & Footer
Touch Interactions & Data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
…sion to Web3Forms
- Implemented a new FAQ page with categorized questions and search functionality. - Added a privacy policy page detailing data protection practices and user rights. - Updated the terms page title for clarity. - Refined contact form validation to streamline error handling.
DungxND
force-pushed
the
feat/ui-improvement-2
branch
from
ea68ae6 to
605cd11
Compare
- add swipe down to dismiss functionality in summary bar
- Bump @eslint/compat from ^2.0.5 to ^2.1.0 - Bump @lucide/svelte from ^0.561.0 to ^1.14.0 - Bump @sveltejs/vite-plugin-svelte from ^7.1.1 to ^7.1.2 - Bump @types/node from ^25.6.0 to ^25.6.2 - Bump vite from ^8.0.10 to ^8.0.11 - Bump amqplib from ^1.0.5 to ^1.0.7
Member
Author
|
Cho user token luôn khi register, không quay lại trang đăng nhập |
Member
Author
|
Cần xem xét ux để tăng tốc độ flow quan trọng |
DungxND
changed the title
DungxND
changed the title
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/routes/(customer)/me/profile/+page.svelte (2)
1-43:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
invalidateAllis referenced but not imported.
handleProfileSubmit(line 93) andhandleEmailSubmit(line 152) callawait invalidateAll(), butinvalidateAllis not present in the updated import block. As soon as either form succeeds you'll hit aReferenceError: invalidateAll is not definedand the success path will throw.🛠️ Proposed fix
- import { enhance } from '$app/forms'; + import { enhance } from '$app/forms'; + import { invalidateAll } from '$app/navigation'; import { resolve } from '$app/paths';Also applies to: 93-93, 152-152
🤖 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/routes/`(customer)/me/profile/+page.svelte around lines 1 - 43, handleProfileSubmit and handleEmailSubmit call await invalidateAll() but invalidateAll was not imported, causing a ReferenceError; add the missing import for invalidateAll from the appropriate SvelteKit kit (import { invalidateAll } from '$app/navigation') at the top of the module so both functions can call it successfully (search for handleProfileSubmit and handleEmailSubmit to locate usage and add the import near other $app/* imports).
153-170:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrong error state assigned in email/password 400 branches.
Both
handleEmailSubmit(line 158) andhandlePasswordSubmit(line 213) write server-side validation errors intoprofileErrorsinstead of their respective state variables. Server errors for the Email card and the Password card will be rendered under the Profile form's fields (and the actual input that failed will look fine), while their own forms silently appear successful from the user's perspective.🛠️ Proposed fix
} else if (res.status === 400) { const json = await res.json(); if (json.errors?.details) { emailErrors = json.errors.details; } else if (json.errors) { - profileErrors = json.errors; + emailErrors = json.errors; } else { toast.error('Dữ liệu không hợp lệ'); }} else if (res.status === 400) { const json = await res.json(); if (json.errors?.details) { passwordErrors = json.errors.details; } else if (json.errors) { - profileErrors = json.errors; + passwordErrors = json.errors; } else { toast.error('Dữ liệu không hợp lệ'); }Also applies to: 208-225
🤖 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/routes/`(customer)/me/profile/+page.svelte around lines 153 - 170, The bug is that server-side 400 validation errors in handleEmailSubmit and handlePasswordSubmit are being assigned to profileErrors instead of their specific error state; update the error handling in both functions so when res.status === 400 you assign json.errors.details to emailErrors in handleEmailSubmit (or to passwordErrors in handlePasswordSubmit) and assign other json.errors to the corresponding emailErrors/passwordErrors variable rather than profileErrors, leaving profileErrors untouched for the Profile form.
🟡 Minor comments (12)
src/routes/+error.svelte-46-50 (1)
46-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake role-to-destination mapping explicit instead of “non-customer ⇒ admin”.
Current ternary sends every non-
customerrole to/admin. If additional roles exist, this can route users to unauthorized screens and degrade recovery UX on error pages.Safer role mapping
- href={resolve(role === 'customer' ? '/' : '/admin')} + href={resolve(role === 'admin' ? '/admin' : '/')}🤖 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/routes/`+error.svelte around lines 46 - 50, The current ternary in the href (using resolve(role === 'customer' ? '/' : '/admin')) routes any non-customer role to /admin which is unsafe; replace this implicit fallback with an explicit role-to-path mapping (e.g., a small map/object or switch) and use resolve(mappedPath) instead, referencing the role variable and the href usage in +error.svelte (the element rendering <House /> and the button text) so each known role (customer, admin, etc.) has an explicit destination and an explicit default (e.g., fallback to '/login' or '/') for unknown roles.src/routes/(customer)/events/[id]/+page.svelte-397-401 (1)
397-401:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpose selected state for show-picker buttons to assistive tech.
These act like toggle options, but selected state is only visual. Add
aria-pressed={isActive}so screen readers announce which show is currently selected.Suggested patch
<button onclick={() => selectShow(show.id)} + aria-pressed={isActive} class="flex min-w-[85px] shrink-0 cursor-pointer flex-col items-center gap-1 rounded-lg px-2.5 py-2 transition-all duration-300 md:min-w-[150px] md:gap-2 md:rounded-xl md:p-5 {isActive ? 'scale-105 bg-primary-container text-white shadow-lg ring-2 ring-primary-container/20 md:ring-4' : 'bg-surface-container text-foreground hover:bg-surface-container-high active:scale-95'}" >🤖 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/routes/`(customer)/events/[id]/+page.svelte around lines 397 - 401, The show-picker button currently only indicates selection visually; update the button element used with selectShow and isActive to expose its selected state to assistive tech by adding aria-pressed={isActive} (so screen readers announce the toggled state). Ensure the attribute is placed on the same <button> that calls selectShow(show.id) and uses the isActive boolean for styling.src/lib/components/seat-map/SeatMap.svelte-138-141 (1)
138-141:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRe-seed pan state when pinch ends with one finger remaining
Currently,
handleTouchEnd()unconditionally resetsisTouchPanningandlastPinchDist, preventing smooth gesture transition when a user lifts one finger during a two-finger pinch while the other finger remains on screen. The remaining touch should resume single-touch panning without requiring a newtouchstart.Add the
TouchEventparameter and check remaining touches:Suggested fix
-function handleTouchEnd() { - isTouchPanning = false; - lastPinchDist = 0; +function handleTouchEnd(e: TouchEvent) { + if (e.touches.length === 1) { + isTouchPanning = true; + lastTouchX = e.touches[0].clientX; + lastTouchY = e.touches[0].clientY; + lastPinchDist = 0; + return; + } + isTouchPanning = false; + lastPinchDist = 0; }🤖 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/lib/components/seat-map/SeatMap.svelte` around lines 138 - 141, handleTouchEnd currently wipes pan state unconditionally; update the signature to accept the TouchEvent, check event.touches.length and if one touch remains re-seed single-touch pan state instead of turning off panning. Specifically, in handleTouchEnd(event: TouchEvent) keep isTouchPanning = true when event.touches.length === 1 and set your single-touch last-pan coordinates (e.g., lastPanX/lastPanY or the equivalent vars used for panning) from event.touches[0].clientX/clientY, and always reset lastPinchDist = 0; if no touches remain set isTouchPanning = false and lastPinchDist = 0. Ensure you reference handleTouchEnd, isTouchPanning and lastPinchDist when making the change.src/routes/(customer)/contact/+page.svelte-166-170 (1)
166-170:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the development note from user-facing text.
The parenthetical comment "(Đây là form liên hệ thật, chúng tôi sẽ nhận được email!)" appears to be an internal note about the form functionality and should be removed before production deployment.
🧹 Proposed fix
<p class="mt-0.5 text-xs text-muted-foreground"> Điền thông tin bên dưới, chúng tôi sẽ liên hệ lại sớm nhất. - <br /> - (Đây là form liên hệ thật, chúng tôi sẽ nhận được email!) </p>🤖 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/routes/`(customer)/contact/+page.svelte around lines 166 - 170, Remove the internal development note from the user-facing paragraph in src/routes/(customer)/contact/+page.svelte: edit the <p class="mt-0.5 text-xs text-muted-foreground"> element to delete the parenthetical sentence "(Đây là form liên hệ thật, chúng tôi sẽ nhận được email!)" so only the customer-facing instruction "Điền thông tin bên dưới, chúng tôi sẽ liên hệ lại sớm nhất." remains.src/lib/components/customer/order/EventTicketCard.svelte-153-154 (1)
153-154:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winText size of 8px may be too small for comfortable reading.
The
text-[8px]class results in 8-pixel text for the "E-TICKET" badge on mobile, which is extremely small and may be difficult to read, especially for users with visual impairments or on lower-resolution displays.While the PR aims to "increase content density," text below 10-11px can significantly impact readability on mobile devices. Consider using at least
text-[10px](10px) for better legibility.📱 Suggested minimum text size
<span - class="rounded bg-primary/10 px-1.5 py-0.5 text-[8px] font-bold tracking-wider text-primary uppercase md:px-2 md:text-[9px]" + class="rounded bg-primary/10 px-1.5 py-0.5 text-[10px] font-bold tracking-wider text-primary uppercase md:px-2 md:text-xs" > E-TICKET </span>Consider applying similar size increases to other very small text (9px labels) for improved mobile readability.
🤖 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/lib/components/customer/order/EventTicketCard.svelte` around lines 153 - 154, The "E-TICKET" badge in EventTicketCard.svelte uses a very small font size (text-[8px]) which harms readability; update the badge's class string (the element rendering the E-TICKET badge) to use at least text-[10px] on mobile (e.g., replace text-[8px] with text-[10px]) and keep or adjust the responsive size (md:text-[9px] -> consider md:text-[11px] or similar) so the badge remains legible across breakpoints.src/lib/components/customer/order/PendingOrderCard.svelte-88-93 (1)
88-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConsider adding
aria-livefor timer accessibility.The countdown timer updates every second but doesn't announce changes to screen readers. Users relying on assistive technology won't know how much time remains unless they manually check.
Adding an
aria-live="polite"attribute to the timer badge would make it accessible, though you may want to throttle announcements (e.g., only announce every 10-30 seconds) to avoid overwhelming screen reader users.♿ Proposed accessibility enhancement
<div - class="flex shrink-0 items-center gap-1.5 rounded-full border border-cta/40 bg-cta-muted/20 px-3 py-1 text-sm font-extrabold text-cta-muted-foreground tabular-nums md:px-4 md:py-1.5 md:text-lg" + class="flex shrink-0 items-center gap-1.5 rounded-full border border-cta/40 bg-cta-muted/20 px-3 py-1 text-sm font-extrabold text-cta-muted-foreground tabular-nums md:px-4 md:py-1.5 md:text-lg" + aria-live="polite" + aria-label="Thời gian còn lại: {formattedTime}" > <Clock class="h-3.5 w-3.5 animate-pulse md:h-4 md:w-4" /> <span>{formattedTime}</span>Alternatively, consider announcing only at specific intervals (e.g., when minutes change) to reduce verbosity.
🤖 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/lib/components/customer/order/PendingOrderCard.svelte` around lines 88 - 93, The timer badge containing the Clock component and the {formattedTime} span should expose changes to assistive tech: add aria-live="polite" and aria-atomic="true" to the surrounding div (the same element with the rounded-full/timer classes) so screen readers announce updates; to avoid overwhelming users, implement throttling for announcements by only updating the accessible text every N seconds or when minutes change (e.g., maintain a separate visually-hidden element that is updated less frequently while the visible {formattedTime} continues to tick).src/lib/components/magic/animated-theme-toggler/animated-theme-toggler.svelte-28-28 (1)
28-28:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
MouseEventparameter to satisfy lint.The
eparameter is unused intoggleand only forwarded byhandleClick.Cleanup
- async function toggle(e: MouseEvent) { + async function toggle() { @@ - function handleClick(e: MouseEvent) { - toggle(e); + function handleClick() { + toggle(); }Also applies to: 60-62
🤖 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/lib/components/magic/animated-theme-toggler/animated-theme-toggler.svelte` at line 28, The toggle function currently declares an unused parameter (MouseEvent e); remove the unused parameter from the toggle function signature (function toggle()) and update any callers (e.g., handleClick) so they no longer forward the event argument when invoking toggle; also remove the unused parameter from the other similar toggle declaration noted around lines 60-62 to satisfy the linter.src/routes/(customer)/me/tickets/+page.svelte-108-110 (1)
108-110:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win“Xem lịch sử” is rendered as an actionable button but has no action.
This creates a dead control in a key user flow. Wire it to navigation (or render non-interactive text until destination exists).
🤖 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/routes/`(customer)/me/tickets/+page.svelte around lines 108 - 110, The "Xem lịch sử ›" button in +page.svelte is an inactive actionable control; either wire it to navigation or make it non-interactive: replace the button or add an on:click that calls SvelteKit's goto('/me/tickets/history') (import { goto } from '$app/navigation') to navigate to the ticket history page, or change it to a semantic anchor (<a href="/me/tickets/history">) for natural navigation; if the destination doesn't exist yet, render plain text or an element with aria-disabled="true" and remove button semantics to avoid a dead control.src/lib/components/magic/number-ticker/number-ticker.svelte-2-2 (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused import to clear lint error.
onMountis imported but never used.Quick cleanup
- import { onMount } from 'svelte';🤖 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/lib/components/magic/number-ticker/number-ticker.svelte` at line 2, Remove the unused import onMount from the module import list in number-ticker.svelte: locate the import line that references "onMount" and delete "onMount" (or the entire import statement if nothing else is imported) so the file no longer imports an unused symbol.src/lib/components/magic/animated-theme-toggler/animated-theme-toggler.svelte-43-46 (1)
43-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCompute transition radius from the button center, not top-left.
maxRadiuscurrently usesleft/top, while the circle origin isx/y(center). This can leave slight uncovered edges in some placements.Fix
const maxRadius = Math.hypot( - Math.max(left, window.innerWidth - left), - Math.max(top, window.innerHeight - top), + Math.max(x, window.innerWidth - x), + Math.max(y, window.innerHeight - y), );🤖 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/lib/components/magic/animated-theme-toggler/animated-theme-toggler.svelte` around lines 43 - 46, maxRadius is computed from the button's top-left coordinates (left/top) but the circle is centered at x/y, causing potential uncovered edges; update the calculation in animated-theme-toggler.svelte to use x and y instead of left and top and compute distances to edges with Math.max(x, window.innerWidth - x) and Math.max(y, window.innerHeight - y) so maxRadius = Math.hypot( maxHorizontalDistance, maxVerticalDistance ); adjust the reference to the variables named maxRadius, x, and y in that block accordingly.src/routes/(customer)/me/profile/+page.svelte-234-238 (1)
234-238:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
isLoggingOutis never flipped, so the logout button's loading UI is dead.
handleLogoutneither setsisLoggingOut = truebefore submit nor toggles it in the post-submit callback, so the disabled state and the spinner branch on the destructive button (lines 646–655) never activate. Mirror the pattern used inAdminHeader.svelte:🛠️ Proposed fix
function handleLogout() { + isLoggingOut = true; return () => sharedLogout(); }Also applies to: 642-657
🤖 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/routes/`(customer)/me/profile/+page.svelte around lines 234 - 238, The logout loading UI never activates because isLoggingOut is never updated; update handleLogout to set isLoggingOut to true immediately before invoking sharedLogout and ensure it is reset to false after the logout completes or fails (use a .finally or post-submit callback) so the destructive button's disabled/spinner branches can react; mirror the pattern used in AdminHeader.svelte by toggling the isLoggingOut state around the sharedLogout call and ensuring any navigation or error paths also clear isLoggingOut.src/layout.css-542-547 (1)
542-547:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stylelint violation in view-transition block.
Line 546 has an empty line before
mix-blend-mode, which violates the configured Stylelint rule and can fail CI lint checks.Suggested patch
::view-transition-old(root), ::view-transition-new(root) { animation: none; - mix-blend-mode: normal; }🤖 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/layout.css` around lines 542 - 547, The view-transition CSS rule for the selectors ::view-transition-old(root) and ::view-transition-new(root) contains an extra blank line before the mix-blend-mode declaration which triggers a Stylelint rule; remove that empty line so the property lines are contiguous (i.e., have animation: none; followed immediately by mix-blend-mode: normal;) and ensure no trailing blank lines inside that block to satisfy the linter.
🧹 Nitpick comments (3)
src/routes/(customer)/careers/+page.svelte (1)
282-290: ⚡ Quick winExpose department chip state with
aria-pressed.These controls are toggle-like filters; adding pressed state improves screen-reader UX.
💡 Proposed fix
<button onclick={() => (activeDept = dept)} + aria-pressed={activeDept === dept} class="rounded-full border px-4 py-1.5 text-xs font-bold tracking-wide transition-all duration-200 {activeDept === dept ? 'border-primary bg-primary text-primary-foreground shadow-sm shadow-primary/20' : 'border-border bg-surface text-muted-foreground hover:border-primary/40 hover:text-foreground'}" >🤖 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/routes/`(customer)/careers/+page.svelte around lines 282 - 290, The department chip buttons are toggle-like controls but lack an accessible pressed state; update the button element that sets activeDept (onclick={() => (activeDept = dept)}) to include an aria-pressed attribute whose boolean value reflects whether activeDept === dept so screen readers get the toggle state; ensure aria-pressed updates when activeDept changes and keep the existing visual classes and onclick behavior unchanged.src/routes/(customer)/faq/+page.svelte (1)
172-180: ⚡ Quick winExpose chip toggle state with
aria-pressed.These buttons behave like single-select toggles; adding pressed semantics improves assistive feedback.
💡 Proposed fix
<button onclick={() => (activeCategory = cat.id)} + aria-pressed={activeCategory === cat.id} class="rounded-full border px-4 py-1.5 text-xs font-bold tracking-wide transition-all duration-200 {activeCategory === cat.id ? 'border-primary bg-primary text-primary-foreground shadow-sm shadow-primary/20' : 'border-border bg-surface text-muted-foreground hover:border-primary/40 hover:text-foreground'}" >🤖 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/routes/`(customer)/faq/+page.svelte around lines 172 - 180, The chip buttons act as single-select toggles but lack pressed semantics; update the button in +page.svelte to include aria-pressed bound to the toggle state (use aria-pressed={activeCategory === cat.id}) so assistive tech receives true/false when a chip is selected; apply this to the same button element that uses onclick/on:click and references activeCategory and cat.id.src/routes/(customer)/contact/+page.svelte (1)
179-238: ⚡ Quick winConsider preserving form values after validation errors.
Currently, when server-side validation fails, the form fields are cleared and users must re-enter all their information. Standard UX practice is to preserve the submitted values so users only need to correct invalid fields.
♻️ Proposed enhancement to preserve form values
<Input id="name" name="name" + value={page.form?.name ?? ''} placeholder="Nguyễn Văn A" class={page.form?.errors?.name ? 'border-destructive focus-visible:ring-destructive/20' : ''} /> <Input id="email" name="email" type="email" + value={page.form?.email ?? ''} placeholder="name@example.com" class={page.form?.errors?.email ? 'border-destructive focus-visible:ring-destructive/20' : ''} /> <Input id="subject" name="subject" + value={page.form?.subject ?? ''} placeholder="Tôi cần hỗ trợ về..." class={page.form?.errors?.subject ? 'border-destructive focus-visible:ring-destructive/20' : ''} /> <Textarea id="message" name="message" + value={page.form?.message ?? ''} placeholder="Mô tả chi tiết vấn đề của bạn..." rows={5} class={page.form?.errors?.message ? 'border-destructive focus-visible:ring-destructive/20' : ''} />🤖 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/routes/`(customer)/contact/+page.svelte around lines 179 - 238, The form currently doesn't repopulate fields after server-side validation because the Input and Textarea components aren't given the submitted values; update each field (Input id/name="name", "email", "subject" and Textarea id/name="message") to pass the previous submission values from page.form (e.g. value={page.form?.values?.name}, value={page.form?.values?.email}, value={page.form?.values?.subject}, value={page.form?.values?.message}) so that when page.form contains errors the components render the submitted values and users only need to correct invalid fields; ensure the prop names match the custom Input/Textarea API (value) used elsewhere in the code.
🤖 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 `@src/lib/components/customer/event/CategoryFilterBar.svelte`:
- Around line 56-63: handleSelectCategory currently creates a fresh
SvelteURLSearchParams and drops any existing query params; instead build the
params from the current URL, update/delete the category key, and then navigate.
Concretely: in handleSelectCategory, replace new SvelteURLSearchParams() with
one constructed from the current search string (e.g. new
SvelteURLSearchParams(window.location.search) or from page.url.searchParams),
call params.set('category', slug) when slug is present or
params.delete('category') when empty, then call
goto(resolve(`/search?${params.toString()}`), { keepFocus: true }); keep the
function name handleSelectCategory and existing goto/resolve usage.
In `@src/lib/components/customer/layout/CustomerNavbar.svelte`:
- Around line 92-102: bottomTabs.findIndex(...) can return -1 which sets
--active-idx to -1 and moves the sliding pill off-screen; change the derived
activeTabIndex logic to compute both the raw index and a clamped index (e.g.,
rawIndex = bottomTabs.findIndex(...); clampedIndex = rawIndex < 0 ? 0 :
rawIndex) and expose a boolean like hasActive = rawIndex >= 0; then use the
clampedIndex for the CSS variable (instead of rawIndex) and conditionally hide
the pill when hasActive is false (add a class or bind a hidden attribute).
Update the references to activeTabIndex in CustomerNavbar.svelte to use the
clamped value for layout and the hasActive flag to toggle visibility so routes
not present in bottomTabs don't push the pill off-canvas.
In `@src/lib/components/customer/order/PendingOrderCard.svelte`:
- Line 77: The Tailwind v4 breaking change requires the "!" important modifier
after the utility name; update the class attributes on CardHeader, CardContent,
and CardFooter to move the "!" after the utility (replace occurrences of
"md:!px-6" with "md:px-6!" in the PendingOrderCard.svelte file), ensuring any
similar uses of the "!"" modifier follow the pattern utility! (e.g., md:px-6!)
so Tailwind v4 parses them correctly.
In `@src/lib/components/magic/dot-pattern/dot-pattern.svelte`:
- Around line 37-38: The x and y props are declared but never used when
computing dot positions, causing the offsets to be ignored and ESLint to flag
unused variables; locate where dot coordinates are created (the code that
computes each dot's cx/cy or position in the dot generation/mapping function)
and add the offsets by adding x to the computed X coordinate and y to the
computed Y coordinate (e.g., cx = baseX + x, cy = baseY + y or include x/y in
the position object), ensuring x and y are referenced so the props actually
shift the pattern.
In `@src/lib/components/magic/number-ticker/number-ticker.svelte`:
- Around line 37-79: The animateValue function currently mutates
spanRef.textContent and never cancels requestAnimationFrame which causes
lifecycle leaks and reactivity desync; refactor animateValue to drive a reactive
numeric store/variable (e.g., animatedValue) instead of writing DOM directly,
keep the active rAF id in a variable (e.g., activeRafId) so you can cancel it
with cancelAnimationFrame before starting a new animation and in the $: /
onDestroy cleanup, update the existing effect cleanup (the $effect block that
clears the timeout) to also cancel activeRafId; remove the unused onMount import
and change the template to render {formatValue(animatedValue)} rather than
static text so prop changes and unmount will be handled reactively.
In `@src/lib/components/seat-map/SeatMap.svelte`:
- Around line 109-123: The touch handlers currently call e.preventDefault()
unconditionally which suppresses the browser-synthesized click events and breaks
seat selection; update handleTouchStart and handleTouchMove so they only call
e.preventDefault() for multi-touch gestures (e.touches.length >= 2) or when you
are actively handling a pinch/zoom (use
isTouchPanning/lastPinchDist/getPinchDist to detect state), and allow
single-touch events to proceed so onclick on seat circle elements still fires.
In `@src/lib/components/seat-map/SummaryBar.svelte`:
- Around line 93-113: The swipe-to-dismiss handlers (handleTouchStart,
handleTouchMove, handleTouchEnd) currently track swipes regardless of the cart
list scroll position, causing downward scrolling to register as a dismiss;
modify handleTouchStart to only begin a swipe when the scroll container is at
the top (scrollTop === 0) or when the gesture starts inside the header/drag
handle area, and in handleTouchMove keep the existing guard but also ignore
motion unless the initial start was allowed; ensure you still reset swipeOffset,
swipeStartY and isSwiping consistently and only set expanded = false when
swipeOffset > SWIPE_THRESHOLD as before.
In `@src/lib/utils/api.ts`:
- Line 86: The client utility in src/lib/utils/api.ts should not call
SvelteKit's server-side redirect(): in the code path where redirect(302,
`/login?redirect=${encodeURIComponent(window.location.pathname)}`) is used,
replace that call with a hard client navigation using window.location.assign and
include both window.location.pathname and window.location.search when building
the redirect query so the original search params are preserved; update the call
site (the function containing the redirect invocation) to use
window.location.assign(`/login?redirect=${encodeURIComponent(window.location.pathname
+ window.location.search)}`) instead of redirect(...) so the navigation happens
client-side and is not swallowed by the outer catch block.
In `@src/routes/`(customer)/careers/+page.svelte:
- Around line 155-160: The {`#each` ... as s} block rendering quick-stats is
missing a key which triggers svelte/require-each-key; update the each block to
include a stable key (for example use s.value or s.label) by changing the block
to {`#each` [{ value: '20+', label: 'Thành viên' }, ...] as s (s.value)} so each
item has a unique key and Svelte can track list changes correctly.
In `@src/routes/`(customer)/contact/+page.server.ts:
- Around line 41-51: The fetch to 'https://api.web3forms.com/submit' has no
timeout; wrap it with an AbortController and a timer (e.g., 5s) and pass
controller.signal into the fetch call (the call that currently assigns to res).
Clear the timeout after fetch completes; on timeout abort, handle the thrown
error (AbortError) and return/log a friendly timeout error so the action doesn't
hang. Ensure you still include WEB3FORMS_KEY and the same JSON body when
invoking fetch.
- Around line 21-24: The current client-supplied _time check in the POST handler
(variable _time in +page.server.ts) is insecure because clients can forge it;
replace this with a server-side rate-limit check: record recent submission
timestamps per user/session/IP on the server (e.g., in server session data, a
short-lived store, or an in-memory/redis rate-limiter) and validate Date.now()
against that server-side timestamp instead of data.get('_time'), rejecting
too-frequent requests; update the handler logic that currently uses _time to
consult this server-side store (and clear/update the stored timestamp on
success).
- Line 4: Remove the hardcoded WEB3FORMS_KEY constant and read the secret from
the app's private environment instead (e.g., replace uses of WEB3FORMS_KEY with
process.env.WEB3FORMS_KEY or SvelteKit's
$env/static/private/$env/dynamic/private value). Update the module that declares
const WEB3FORMS_KEY to import the environment variable, throw or log a clear
error if the env var is missing, and ensure any callers (e.g., the handler in
+page.server.ts that sends the form) use the env-backed value; also rotate the
leaked key and add the new secret to your deployment secrets store.
In `@src/routes/`(customer)/faq/+page.svelte:
- Around line 155-160: The search input currently uses only a placeholder which
is not a reliable accessible name for assistive tech; update the input to have
an explicit accessible label by either adding an aria-label (e.g.,
aria-label="Search FAQs") or by creating a visible or screen-reader-only <label>
tied to the input via an id (match the id to the input bound to query) so the
element bound to query has a proper accessible name for screen readers.
In `@src/routes/`+error.svelte:
- Around line 34-41: The UI is directly rendering the backend error variable
message in +error.svelte (inside the {`#if` status === 404} ... {:else} block),
which can leak internal details; remove or replace the raw {message} in the
customer-facing text with a generic/internalized message (e.g., "An unexpected
error occurred. Our team has been notified.") and, if you still want detailed
output for debugging, only render message behind a dev-only guard (e.g.,
import.meta.env.DEV) or log the full message server-side instead; update the
else branch that references message so end users never see raw backend content.
In `@vite.config.ts`:
- Around line 14-16: Remove the entire hmr configuration block from the Vite
config (the object named hmr with keys host and protocol) so Vite can
auto-detect protocol/host from the request; specifically delete the hmr: { host:
'localhost', protocol: 'wss' } block and rely on the existing server.host
setting instead to avoid protocol/host mismatches that break HMR.
---
Outside diff comments:
In `@src/routes/`(customer)/me/profile/+page.svelte:
- Around line 1-43: handleProfileSubmit and handleEmailSubmit call await
invalidateAll() but invalidateAll was not imported, causing a ReferenceError;
add the missing import for invalidateAll from the appropriate SvelteKit kit
(import { invalidateAll } from '$app/navigation') at the top of the module so
both functions can call it successfully (search for handleProfileSubmit and
handleEmailSubmit to locate usage and add the import near other $app/* imports).
- Around line 153-170: The bug is that server-side 400 validation errors in
handleEmailSubmit and handlePasswordSubmit are being assigned to profileErrors
instead of their specific error state; update the error handling in both
functions so when res.status === 400 you assign json.errors.details to
emailErrors in handleEmailSubmit (or to passwordErrors in handlePasswordSubmit)
and assign other json.errors to the corresponding emailErrors/passwordErrors
variable rather than profileErrors, leaving profileErrors untouched for the
Profile form.
---
Minor comments:
In `@src/layout.css`:
- Around line 542-547: The view-transition CSS rule for the selectors
::view-transition-old(root) and ::view-transition-new(root) contains an extra
blank line before the mix-blend-mode declaration which triggers a Stylelint
rule; remove that empty line so the property lines are contiguous (i.e., have
animation: none; followed immediately by mix-blend-mode: normal;) and ensure no
trailing blank lines inside that block to satisfy the linter.
In `@src/lib/components/customer/order/EventTicketCard.svelte`:
- Around line 153-154: The "E-TICKET" badge in EventTicketCard.svelte uses a
very small font size (text-[8px]) which harms readability; update the badge's
class string (the element rendering the E-TICKET badge) to use at least
text-[10px] on mobile (e.g., replace text-[8px] with text-[10px]) and keep or
adjust the responsive size (md:text-[9px] -> consider md:text-[11px] or similar)
so the badge remains legible across breakpoints.
In `@src/lib/components/customer/order/PendingOrderCard.svelte`:
- Around line 88-93: The timer badge containing the Clock component and the
{formattedTime} span should expose changes to assistive tech: add
aria-live="polite" and aria-atomic="true" to the surrounding div (the same
element with the rounded-full/timer classes) so screen readers announce updates;
to avoid overwhelming users, implement throttling for announcements by only
updating the accessible text every N seconds or when minutes change (e.g.,
maintain a separate visually-hidden element that is updated less frequently
while the visible {formattedTime} continues to tick).
In
`@src/lib/components/magic/animated-theme-toggler/animated-theme-toggler.svelte`:
- Line 28: The toggle function currently declares an unused parameter
(MouseEvent e); remove the unused parameter from the toggle function signature
(function toggle()) and update any callers (e.g., handleClick) so they no longer
forward the event argument when invoking toggle; also remove the unused
parameter from the other similar toggle declaration noted around lines 60-62 to
satisfy the linter.
- Around line 43-46: maxRadius is computed from the button's top-left
coordinates (left/top) but the circle is centered at x/y, causing potential
uncovered edges; update the calculation in animated-theme-toggler.svelte to use
x and y instead of left and top and compute distances to edges with Math.max(x,
window.innerWidth - x) and Math.max(y, window.innerHeight - y) so maxRadius =
Math.hypot( maxHorizontalDistance, maxVerticalDistance ); adjust the reference
to the variables named maxRadius, x, and y in that block accordingly.
In `@src/lib/components/magic/number-ticker/number-ticker.svelte`:
- Line 2: Remove the unused import onMount from the module import list in
number-ticker.svelte: locate the import line that references "onMount" and
delete "onMount" (or the entire import statement if nothing else is imported) so
the file no longer imports an unused symbol.
In `@src/lib/components/seat-map/SeatMap.svelte`:
- Around line 138-141: handleTouchEnd currently wipes pan state unconditionally;
update the signature to accept the TouchEvent, check event.touches.length and if
one touch remains re-seed single-touch pan state instead of turning off panning.
Specifically, in handleTouchEnd(event: TouchEvent) keep isTouchPanning = true
when event.touches.length === 1 and set your single-touch last-pan coordinates
(e.g., lastPanX/lastPanY or the equivalent vars used for panning) from
event.touches[0].clientX/clientY, and always reset lastPinchDist = 0; if no
touches remain set isTouchPanning = false and lastPinchDist = 0. Ensure you
reference handleTouchEnd, isTouchPanning and lastPinchDist when making the
change.
In `@src/routes/`(customer)/contact/+page.svelte:
- Around line 166-170: Remove the internal development note from the user-facing
paragraph in src/routes/(customer)/contact/+page.svelte: edit the <p
class="mt-0.5 text-xs text-muted-foreground"> element to delete the
parenthetical sentence "(Đây là form liên hệ thật, chúng tôi sẽ nhận được
email!)" so only the customer-facing instruction "Điền thông tin bên dưới, chúng
tôi sẽ liên hệ lại sớm nhất." remains.
In `@src/routes/`(customer)/events/[id]/+page.svelte:
- Around line 397-401: The show-picker button currently only indicates selection
visually; update the button element used with selectShow and isActive to expose
its selected state to assistive tech by adding aria-pressed={isActive} (so
screen readers announce the toggled state). Ensure the attribute is placed on
the same <button> that calls selectShow(show.id) and uses the isActive boolean
for styling.
In `@src/routes/`(customer)/me/profile/+page.svelte:
- Around line 234-238: The logout loading UI never activates because
isLoggingOut is never updated; update handleLogout to set isLoggingOut to true
immediately before invoking sharedLogout and ensure it is reset to false after
the logout completes or fails (use a .finally or post-submit callback) so the
destructive button's disabled/spinner branches can react; mirror the pattern
used in AdminHeader.svelte by toggling the isLoggingOut state around the
sharedLogout call and ensuring any navigation or error paths also clear
isLoggingOut.
In `@src/routes/`(customer)/me/tickets/+page.svelte:
- Around line 108-110: The "Xem lịch sử ›" button in +page.svelte is an inactive
actionable control; either wire it to navigation or make it non-interactive:
replace the button or add an on:click that calls SvelteKit's
goto('/me/tickets/history') (import { goto } from '$app/navigation') to navigate
to the ticket history page, or change it to a semantic anchor (<a
href="/me/tickets/history">) for natural navigation; if the destination doesn't
exist yet, render plain text or an element with aria-disabled="true" and remove
button semantics to avoid a dead control.
In `@src/routes/`+error.svelte:
- Around line 46-50: The current ternary in the href (using resolve(role ===
'customer' ? '/' : '/admin')) routes any non-customer role to /admin which is
unsafe; replace this implicit fallback with an explicit role-to-path mapping
(e.g., a small map/object or switch) and use resolve(mappedPath) instead,
referencing the role variable and the href usage in +error.svelte (the element
rendering <House /> and the button text) so each known role (customer, admin,
etc.) has an explicit destination and an explicit default (e.g., fallback to
'/login' or '/') for unknown roles.
---
Nitpick comments:
In `@src/routes/`(customer)/careers/+page.svelte:
- Around line 282-290: The department chip buttons are toggle-like controls but
lack an accessible pressed state; update the button element that sets activeDept
(onclick={() => (activeDept = dept)}) to include an aria-pressed attribute whose
boolean value reflects whether activeDept === dept so screen readers get the
toggle state; ensure aria-pressed updates when activeDept changes and keep the
existing visual classes and onclick behavior unchanged.
In `@src/routes/`(customer)/contact/+page.svelte:
- Around line 179-238: The form currently doesn't repopulate fields after
server-side validation because the Input and Textarea components aren't given
the submitted values; update each field (Input id/name="name", "email",
"subject" and Textarea id/name="message") to pass the previous submission values
from page.form (e.g. value={page.form?.values?.name},
value={page.form?.values?.email}, value={page.form?.values?.subject},
value={page.form?.values?.message}) so that when page.form contains errors the
components render the submitted values and users only need to correct invalid
fields; ensure the prop names match the custom Input/Textarea API (value) used
elsewhere in the code.
In `@src/routes/`(customer)/faq/+page.svelte:
- Around line 172-180: The chip buttons act as single-select toggles but lack
pressed semantics; update the button in +page.svelte to include aria-pressed
bound to the toggle state (use aria-pressed={activeCategory === cat.id}) so
assistive tech receives true/false when a chip is selected; apply this to the
same button element that uses onclick/on:click and references activeCategory and
cat.id.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e90a396-f394-4f9c-81b4-17f5453bdcc7
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (54)
package.jsonsrc/layout.csssrc/lib/components/ToastContainer.sveltesrc/lib/components/admin/layout/AdminHeader.sveltesrc/lib/components/customer/event/AmenityBadge.sveltesrc/lib/components/customer/event/CategoryFilterBar.sveltesrc/lib/components/customer/event/EmptyState.sveltesrc/lib/components/customer/event/EventBannerCarousel.sveltesrc/lib/components/customer/event/EventCard.sveltesrc/lib/components/customer/event/EventTimeline.sveltesrc/lib/components/customer/event/PaginationNav.sveltesrc/lib/components/customer/event/SectionHeader.sveltesrc/lib/components/customer/layout/CustomerFooter.sveltesrc/lib/components/customer/layout/CustomerNavbar.sveltesrc/lib/components/customer/order/EventTicketCard.sveltesrc/lib/components/customer/order/PendingOrderCard.sveltesrc/lib/components/magic/animated-grid-pattern/animated-grid-pattern.sveltesrc/lib/components/magic/animated-grid-pattern/index.tssrc/lib/components/magic/animated-theme-toggler/animated-theme-toggler.sveltesrc/lib/components/magic/animated-theme-toggler/index.tssrc/lib/components/magic/dot-pattern/dot-pattern.sveltesrc/lib/components/magic/dot-pattern/index.tssrc/lib/components/magic/number-ticker/index.tssrc/lib/components/magic/number-ticker/number-ticker.sveltesrc/lib/components/seat-map/SeatMap.sveltesrc/lib/components/seat-map/SummaryBar.sveltesrc/lib/components/ui/bento/BentoStat.sveltesrc/lib/server/services/event.service.tssrc/lib/utils/api.tssrc/lib/utils/auth.tssrc/routes/(admin)/admin/+layout.server.tssrc/routes/(admin)/admin/events/+page.sveltesrc/routes/(auth)/+layout.sveltesrc/routes/(auth)/login/+page.sveltesrc/routes/(auth)/register/+page.sveltesrc/routes/(customer)/+page.server.tssrc/routes/(customer)/+page.sveltesrc/routes/(customer)/about/+page.sveltesrc/routes/(customer)/careers/+page.sveltesrc/routes/(customer)/contact/+page.server.tssrc/routes/(customer)/contact/+page.sveltesrc/routes/(customer)/events/+page.sveltesrc/routes/(customer)/events/[id]/+page.sveltesrc/routes/(customer)/faq/+page.sveltesrc/routes/(customer)/me/profile/+page.server.tssrc/routes/(customer)/me/profile/+page.sveltesrc/routes/(customer)/me/tickets/+page.server.tssrc/routes/(customer)/me/tickets/+page.sveltesrc/routes/(customer)/privacy/+page.sveltesrc/routes/(customer)/terms/+page.sveltesrc/routes/+error.sveltesrc/routes/api/orders/[id]/checkout/+server.tssvelte.config.jsvite.config.ts
💤 Files with no reviewable changes (1)
- src/routes/(customer)/+page.server.ts
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
…BentoStat text size
This comment has been minimized.
This comment has been minimized.
…ed seat data handling and layout; update AdminSidebar version display
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (5)
src/lib/components/customer/layout/ProfileSidebar.svelte (1)
47-51:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRace condition: unawaited logout conflicts with navigation.
The
api.postcall on line 48 is not awaited, sogotoon line 50 executes before the server clears the session cookie. This can causeinvalidateAllto re-fetch user data while still authenticated, leaving stale UI state.Use the
sharedLogoututility from$lib/utils/authwith a SvelteKit form action (as done in+page.sveltelines 653-664).🔧 Recommended fix
Replace the button with a form-based approach:
- function handleLogout() { - api.post('/auth/logout', {}).catch(() => {}); - toast.success('Đăng xuất thành công!'); - goto(resolve('/'), { invalidateAll: true }); - } + import { enhance } from '$app/forms'; + import { handleLogout as sharedLogout } from '$lib/utils/auth'; + + function handleLogout() { + return () => sharedLogout(); + }Then update the logout button to use a form:
<form action={resolve('/api/auth/logout')} method="POST" use:enhance={handleLogout}> <button type="submit" class="flex w-full items-center gap-2.5 rounded-lg px-3 py-2 text-[13px] font-medium text-muted-foreground transition-colors duration-150 hover:bg-destructive/8 hover:text-destructive" > <LogOut class="size-3.5 shrink-0" /> Đăng xuất </button> </form>🤖 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/lib/components/customer/layout/ProfileSidebar.svelte` around lines 47 - 51, The handleLogout implementation causes a race: api.post('/auth/logout') is not awaited so goto(resolve('/'), { invalidateAll: true }) can run before the server clears the session; replace the button-driven logout with the sharedLogout form-action flow used elsewhere by using the sharedLogout utility and a POST form action (e.g., action={resolve('/api/auth/logout')} method="POST" with use:enhance={handleLogout}) instead of calling api.post directly, and remove the unawaited api.post call from handleLogout so navigation happens after the server-side logout completes and invalidateAll will see the logged-out state.src/lib/components/customer/event/CategoryFilterBar.svelte (1)
52-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset pagination when switching categories to avoid empty results pages.
When users switch categories while on a later page (e.g., page 5), they may land on an empty results page if the new category has fewer total pages. The page parameter should be deleted to reset pagination to page 1.
🔧 Proposed fix
function buildHref(slug: string) { const params = new SvelteURLSearchParams(page.url.searchParams); if (slug) { params.set('category', slug); } else { params.delete('category'); } + params.delete('page'); return resolve(`/search?${params.toString()}`); }🤖 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/lib/components/customer/event/CategoryFilterBar.svelte` around lines 52 - 60, The buildHref function currently preserves the current pagination when changing categories; update it so that when modifying the 'category' param you also remove the 'page' param to reset to page 1. In the buildHref function (which uses SvelteURLSearchParams and page.url.searchParams and returns resolve(`/search?${params.toString()}`)), after setting or deleting the 'category' key ensure params.delete('page') is called so switching categories clears pagination and avoids landing on empty result pages.src/lib/components/magic/number-ticker/number-ticker.svelte (2)
38-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard
decimalPlacesbeforetoFixedto avoid runtime crashes.
toFixed(decimalPlaces)throws aRangeErrorwhendecimalPlacesis outside the valid range of 0–100 or is non-integer. Since the prop is unconstrained and defaults are not enforced at runtime, invalid values will crash the component.🛡️ Proposed fix
function formatValue(val: number): string { + const sanitized = Math.max(0, Math.min(100, Math.trunc(decimalPlaces ?? 0))); + return `${prefix}${Intl.NumberFormat('en-US', { - minimumFractionDigits: decimalPlaces, - maximumFractionDigits: decimalPlaces, - }).format(Number(val.toFixed(decimalPlaces)))}${suffix}`; + minimumFractionDigits: sanitized, + maximumFractionDigits: sanitized, + }).format(Number(val.toFixed(sanitized)))}${suffix}`; }🤖 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/lib/components/magic/number-ticker/number-ticker.svelte` around lines 38 - 43, The formatValue function uses decimalPlaces directly in val.toFixed(decimalPlaces), which can throw a RangeError if decimalPlaces is non-integer or outside 0–100; guard and normalize it first (e.g., compute a safeDecimalPlaces by coercing decimalPlaces to an integer with Math.round or Math.floor, clamp it between 0 and 100, and fallback to a sensible default if undefined/NaN), then use safeDecimalPlaces in both toFixed and the Intl.NumberFormat options; update references inside formatValue to use safeDecimalPlaces instead of decimalPlaces so the component cannot crash at runtime.
31-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInitialize
animatedValueto match the starting position.Currently
animatedValuealways starts at0, causing a brief flash before the animation begins. Initialize to the correct starting value based ondirection.✨ Proposed fix
- let animatedValue = $state(0); + let animatedValue = $state(direction === 'down' ? value : startValue);🤖 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/lib/components/magic/number-ticker/number-ticker.svelte` at line 31, animatedValue is initialized to 0 which causes a visual flash; instead initialize it to the component's starting position derived from the current value and direction. Update the initialization of animatedValue (the $state wrapper) so it uses the current numeric value (e.g., the prop/state value the ticker should start from) adjusted for direction, and ensure you also sync it when the input value or direction changes (in the component's setup or onMount/update logic) so animatedValue starts aligned with the visible starting position rather than 0.src/lib/server/config.ts (1)
29-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the hardcoded
WEB3FORMS_KEYdefault and require env injection.Line 29 currently embeds a concrete key in source. This reintroduces the previously flagged credential-hardcoding risk and can silently pass misconfigured environments.
💡 Minimal fix
- WEB3FORMS_KEY: z.string().default('2fe33d39-dd60-4a96-8beb-3b807daf571e'), + WEB3FORMS_KEY: z.string().min(1, 'WEB3FORMS_KEY is required'),Also applies to: 50-50, 99-100
🤖 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/lib/server/config.ts` at line 29, The config currently hardcodes WEB3FORMS_KEY by using z.string().default('2fe33d39-...'), so remove the default and make the key required from the environment by changing the schema entry for WEB3FORMS_KEY to a required string (e.g., z.string() or z.string().nonempty()), and ensure any code that reads the parsed config (the symbol WEB3FORMS_KEY in your config/schema object) expects it to be present and fails fast if missing so the value must be injected via env rather than falling back to a literal default.
🧹 Nitpick comments (16)
src/lib/components/admin/layout/AdminSidebar.svelte (1)
105-109: ⚡ Quick winUse
<span>instead of<p>for semantic correctness.A
<p>tag is a block-level element semantically intended for paragraphs, whereas this version badge is styled and positioned as an inline component. Using a<span>better reflects the semantic role of a small inline label.♻️ Suggested semantic fix
- <p + <span class="rounded-md bg-muted/60 px-1.5 py-0.5 text-[9px] font-semibold tracking-wider text-muted-foreground/50 uppercase" > v{version} - </p> + </span>🤖 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/lib/components/admin/layout/AdminSidebar.svelte` around lines 105 - 109, The version badge is currently rendered as a paragraph element showing the version variable; replace that paragraph element with an inline span element (keeping the class and the v{version} binding intact) so the markup is semantically correct for a small inline label, and verify the visual styling and accessibility remain unchanged after swapping the element.src/lib/components/customer/event/EventBannerCarousel.svelte (3)
142-143: ⚡ Quick winConsider UX impact of truncating venue names.
The
line-clamp-1on the venue text will truncate long venue names without providing a way for users to see the full location. While venue information is also available on the detail page, truncation here might hide important location details that influence a user's decision to click through.Consider:
- Adding a tooltip on hover to show the full venue name
- Using a two-line clamp (
line-clamp-2) to show more context- Testing with your longest venue names to ensure critical info isn't lost
🤖 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/lib/components/customer/event/EventBannerCarousel.svelte` around lines 142 - 143, The venue text currently uses the CSS utility class line-clamp-1 which can hide important location details; update the EventBannerCarousel component to make the full venue discoverable by either replacing line-clamp-1 with line-clamp-2 on the span that renders {event.venue} or add a hover tooltip (e.g., title attribute or a Tooltip component) on the same span so users can view the full venue string, referencing the MapPin element and the span that contains event.venue for where to apply the change; test with longest venue names to confirm readability.
153-155: ⚡ Quick winHardcoded Vietnamese strings limit internationalization.
The component uses hardcoded Vietnamese text ("Giá vé từ", "Xem thêm") that will prevent localization if the app needs to support multiple languages in the future.
If internationalization is planned, consider extracting these strings to a localization system. If the app is intended to be Vietnam-only, this can be safely ignored.
Also applies to: 169-169
🤖 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/lib/components/customer/event/EventBannerCarousel.svelte` around lines 153 - 155, The component EventBannerCarousel.svelte contains hardcoded Vietnamese strings ("Giá vé từ" and "Xem thêm") which should be extracted for localization; replace these literals in the template with i18n keys (e.g., use your project's translation helper like $t('event.price_from') or i18n.t('event.view_more')), add corresponding entries to the translations resource (Vietnamese and other locales), and update any tests/props that expect the raw text so the display uses the translation function instead of the hardcoded strings.
166-173: ⚖️ Poor tradeoffConsider CTA effectiveness for conversion optimization.
The button text "Xem thêm" (View details) is accurate but less action-oriented than purchase-focused CTAs like "Đặt vé ngay" (Buy tickets now). While this may be intentional given the admin buy prevention mentioned in commits, the less direct CTA could slow the user conversion funnel.
This relates to PR comment
#2about accelerating important user flows. For non-admin users on the primary discovery path, a more direct purchase CTA might improve conversion rates.🤖 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/lib/components/customer/event/EventBannerCarousel.svelte` around lines 166 - 173, EventBannerCarousel currently renders a neutral CTA text "Xem thêm"; update the component to render a more action-oriented CTA for conversion by conditionally switching the label to "Đặt vé ngay" for non-admin users on the primary discovery path (keep "Xem thêm" for admins or non-primary flows). Locate the CTA span in EventBannerCarousel (the element containing the ArrowRight icon) and implement a concise conditional using the existing user/context flags (e.g., isAdmin, isPrimaryPath or similar props/state) to choose the label, ensure accessibility by updating the aria-label accordingly, and wire any tracking/event handlers to the new CTA without changing styling or the ArrowRight component usage.src/routes/(customer)/me/profile/+page.svelte (1)
454-479: ⚖️ Poor tradeoffConsider extracting password input to a reusable component.
The password input with visibility toggle pattern is repeated 5 times (email current password, and 3 password fields). Extracting this to a reusable component would reduce duplication and improve maintainability.
💡 Example component structure
<!-- PasswordInput.svelte --> <script lang="ts"> import { Eye, EyeOff, Lock, Key } from 'lucide-svelte'; let { id, value = $bindable(''), placeholder = '••••••••', icon = 'lock' }: { id: string; value: string; placeholder?: string; icon?: 'lock' | 'key'; } = $props(); let show = $state(false); const IconComponent = icon === 'key' ? Key : Lock; </script> <div class="flex h-10 w-full rounded-md border border-input bg-transparent ring-offset-background focus-within:ring-2 focus-within:ring-ring focus-within:ring-offset-2"> <div class="flex items-center pl-3 text-muted-foreground"> <IconComponent class="size-4 shrink-0" /> </div> <input {id} type={show ? 'text' : 'password'} bind:value {placeholder} class="h-full flex-1 bg-transparent px-3 text-sm outline-none placeholder:text-muted-foreground" /> <button type="button" onclick={() => (show = !show)} class="flex items-center pr-3 text-muted-foreground transition-colors hover:text-foreground" aria-label={show ? 'Ẩn mật khẩu' : 'Hiện mật khẩu'} > {`#if` show} <EyeOff class="size-4 shrink-0" /> {:else} <Eye class="size-4 shrink-0" /> {/if} </button> </div>Usage:
<PasswordInput id="email_curpwd" bind:value={emailForm.current_password} /> <PasswordInput id="new_pw" bind:value={passwordForm.new_password} icon="key" />Also applies to: 521-546, 558-583, 594-619
🤖 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/routes/`(customer)/me/profile/+page.svelte around lines 454 - 479, The repeated password-field pattern (seen in the block using showEmailPwd, emailForm.current_password, the Lock/Eye/EyeOff icons and input id "email_curpwd") should be extracted into a reusable PasswordInput Svelte component (e.g., PasswordInput.svelte) that accepts props id, bind:value, placeholder, and icon (lock|key) and manages its own internal show state and toggle button; replace each in this file (and at the other occurrences around lines 521-546, 558-583, 594-619) with <PasswordInput id="..." bind:value={...} placeholder="…" icon="lock|key" /> so you remove duplicated markup and keep the same aria-label, icon components (Lock, Key), and behavior for toggling between text/password and updating the bound form fields.src/layout.css (3)
106-106: 💤 Low valueInconsistent OKLCH lightness format across color tokens.
Lines 106 and 205 use percent notation (
94.431%,80.576%) for lightness, while the rest of the file uses decimal format (0.98,0.2, etc.). Both formats are valid CSS, but mixing them reduces consistency and may confuse maintainers.Consider normalizing to decimal format to match the established pattern.
♻️ Proposed normalization
- --info-muted: oklch(94.431% 0.01395 219.971); + --info-muted: oklch(0.944 0.014 220);- --info: oklch(80.576% 0.08457 237.984); + --info: oklch(0.806 0.085 238);Also applies to: 205-205
🤖 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/layout.css` at line 106, Two OKLCH color tokens use percent lightness (e.g., --info-muted: oklch(94.431% 0.01395 219.971);) while the rest use decimal lightness; update those tokens to use decimal lightness (convert 94.431% → 0.94431, 80.576% → 0.80576) so the OKLCH calls follow the same decimal format across the file, and ensure you update every occurrence that uses percent notation.
547-547: 💤 Low valueMinor formatting issue flagged by Stylelint.
Remove the empty line before the
mix-blend-modedeclaration to satisfy the linter rule.♻️ Proposed fix
::view-transition-old(root), ::view-transition-new(root) { animation: none; - mix-blend-mode: normal; }🤖 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/layout.css` at line 547, Remove the extra blank line immediately preceding the mix-blend-mode declaration so the rule appears directly under its selector/declaration block; edit the mix-blend-mode line (mix-blend-mode: normal;) to have no empty line above it in src/layout.css to satisfy Stylelint formatting rules.
353-353: ⚡ Quick winRespect user motion preferences for smooth scrolling.
Global
scroll-behavior: smoothcan cause discomfort for users with motion sensitivity. Wrap it in a media query to respect theprefers-reduced-motionsetting.♻️ Proposed fix
html { `@apply` font-sans; -webkit-font-smoothing: antialiased; -moz-osx-font-smoothing: grayscale; + } + + `@media` (prefers-reduced-motion: no-preference) { + html { + scroll-behavior: smooth; + } - scroll-behavior: smooth; }🤖 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/layout.css` at line 353, The global scroll-behavior: smooth rule should respect user motion preferences: wrap the existing scroll-behavior: smooth declaration inside a media query that only applies when the user has not requested reduced motion (use `@media` (prefers-reduced-motion: no-preference)) and target the same element (e.g., html or :root) so smooth scrolling remains the default for users without reduced-motion, while users who prefer reduced motion are not affected; update the existing scroll-behavior: smooth line accordingly.src/lib/components/seat-map/SummaryBar.svelte (1)
97-99: ⚡ Quick winFragile scroll container selection.
The
querySelector('[class*="overflow-y-auto"]')selector relies on substring matching and will break silently if the scroll container's class names change. Consider using abind:thisreference instead.♻️ Refactor to use a direct reference
Add a binding at the top of the script:
+ let scrollContainer: HTMLElement;Then bind the scroll container element (around line 218):
- <div class="flex-1 space-y-3 overflow-y-auto px-4 pb-20 sm:px-6 lg:px-8"> + <div class="flex-1 space-y-3 overflow-y-auto px-4 pb-20 sm:px-6 lg:px-8" bind:this={scrollContainer}>Update the handler:
function handleTouchStart(e: TouchEvent) { if (e.touches.length === 1) { - const scrollContainer = (e.currentTarget as HTMLElement).querySelector( - '[class*="overflow-y-auto"]', - ); const isAtTop = !scrollContainer || scrollContainer.scrollTop === 0;🤖 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/lib/components/seat-map/SummaryBar.svelte` around lines 97 - 99, The current handler uses a fragile querySelector('[class*="overflow-y-auto"]') to find the scroll container; replace it with a direct Svelte element reference by declaring a local variable (e.g., let scrollContainerEl) in the script, binding that variable to the scroll container element with bind:this on the DOM node (the element around where the scrollable area is rendered), and update the handler to use that bound variable (scrollContainerEl) instead of querying via e.currentTarget; remove the substring class selector logic so the handler directly reads from the bound element.src/lib/components/magic/number-ticker/number-ticker.svelte (1)
58-68: Use actual frame delta for smoother, device-independent animation.The fixed
dt = 16.67 / 1000assumes a constant 60fps, but actual frame rates vary. This causes the animation to run faster on high-refresh displays and stutter during frame drops.[medium_effort_and_high_reward]
⚡ Proposed refactor for variable time step
function animateValue(from: number, to: number) { if (activeRafId !== null) { cancelAnimationFrame(activeRafId); } const startTime = performance.now(); const duration = 2000; // Duration in ms + let lastTime = startTime; let velocity = 0; let position = from; const target = to; function step(currentTime: number) { const elapsed = currentTime - startTime; - const dt = 16.67 / 1000; // ~60fps in seconds + const dt = (currentTime - lastTime) / 1000; // Actual delta in seconds + lastTime = currentTime; // Spring physics const springForce = -stiffness * (position - target); const dampingForce = -damping * velocity; const acceleration = springForce + dampingForce; velocity += acceleration * dt; position += velocity * dt; // Update animated value animatedValue = position; // Continue animation if not settled const isSettled = Math.abs(velocity) < 0.01 && Math.abs(position - target) < 0.01; if (!isSettled && elapsed < duration * 2) { activeRafId = requestAnimationFrame(step); } else { // Ensure final value is exact animatedValue = target; activeRafId = null; } } activeRafId = requestAnimationFrame(step); }🤖 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/lib/components/magic/number-ticker/number-ticker.svelte` around lines 58 - 68, The step function uses a fixed dt of 16.67/1000 which makes animation frame-rate dependent; replace that with the actual frame delta computed from timestamps (e.g., dt = (currentTime - lastTime) / 1000 where lastTime is initialized before the first frame and updated each call) and clamp dt to a sensible max (to prevent huge jumps after tab switch/frame drop). Update references to startTime/currentTime handling so you use lastTime for per-frame integration, and ensure position/velocity are integrated using this variable dt inside step.src/routes/(customer)/events/[id]/shows/[showId]/seats/+page.svelte (1)
701-712: ⚡ Quick winRespect reduced-motion preferences for the countdown pulse animation.
The new critical pulse is useful, but it should be disabled for users with reduced-motion settings.
Suggested CSS adjustment
.animate-countdown-pulse { animation: countdown-pulse 1.4s ease-in-out infinite; } + +@media (prefers-reduced-motion: reduce) { + .animate-countdown-pulse { + animation: none; + } +}🤖 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/routes/`(customer)/events/[id]/shows/[showId]/seats/+page.svelte around lines 701 - 712, The countdown pulse animation (keyframes named countdown-pulse and the class animate-countdown-pulse) doesn’t respect users’ reduced-motion preference; update the CSS to disable the animation for users who set prefers-reduced-motion: reduce by adding a media query that sets animation: none (and/or removes box-shadow transitions) for .animate-countdown-pulse under `@media` (prefers-reduced-motion: reduce) so the pulse is effectively turned off while keeping the original keyframes/class for users without that preference.src/routes/+layout.svelte (1)
2-2: ⚡ Quick winRemove unused favicon import if line 25 is removed.
The
faviconasset import is only used on line 25. If you consolidate the favicon links as suggested above and remove line 25, this import should also be removed.🤖 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/routes/`+layout.svelte at line 2, Remove the unused import favicon from the top of +layout.svelte if you also remove the favicon link usage (previously referenced where favicon was used); locate the import statement "import favicon from '$lib/assets/favicon.svg';" and delete it so there are no unused imports or linter warnings after consolidating favicon links.src/app.html (1)
2-2: ⚖️ Poor tradeoffConsider dynamic locale if internationalization is planned.
The
langattribute is hardcoded to Vietnamese. If the application plans to support multiple languages, this should be made dynamic using SvelteKit's data loading or a similar approach.If Vietnamese is the only supported language, this is fine as-is.
🤖 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.html` at line 2, The HTML lang attribute is hardcoded ("<html lang=\"vi\">"); make it dynamic so the root element reflects the active locale (e.g., derive a locale variable via SvelteKit load or your i18n store and bind it into the html tag) — update the <html lang="..."> usage in src/app.html to read the runtime locale value instead of the literal "vi" and ensure the loader/store that provides the locale (e.g., a page load returning locale or an i18n getLocale function) is wired to supply that value.src/routes/(customer)/me/tickets/+page.svelte (1)
47-77: ⚡ Quick winConsider adding ARIA attributes for better screen reader experience.
The tab interface works well for keyboard and mouse users, but adding ARIA tab pattern attributes would improve the experience for screen reader users by properly announcing the tab role and selected state.
♿ Proposed ARIA enhancements
Wrap the buttons in a
role="tablist"container and addrole="tab"+aria-selectedto each button:- <div + <div + role="tablist" + aria-label="Lọc vé theo trạng thái" class="flex w-fit items-center rounded-full border border-border bg-surface-container-low p-1" > <button + role="tab" + aria-selected={activeTab === 'all'} onclick={() => (activeTab = 'all')} class="rounded-full px-3 py-1 text-xs font-semibold transition-all md:px-5 md:py-1.5 md:text-sm {activeTab === 'all' ? 'bg-background text-foreground shadow-sm' : 'text-muted-foreground hover:text-foreground'}" > Tất cả </button> <button + role="tab" + aria-selected={activeTab === 'pending'} onclick={() => (activeTab = 'pending')} class="rounded-full px-2.5 py-1 text-xs font-semibold transition-all md:px-4 md:py-1.5 md:text-sm {activeTab === 'pending' ? 'bg-background text-foreground shadow-sm' : 'text-muted-foreground hover:text-foreground'}" > Chờ thanh toán </button> <button + role="tab" + aria-selected={activeTab === 'paid'} onclick={() => (activeTab = 'paid')} class="rounded-full px-2.5 py-1 text-xs font-semibold transition-all md:px-4 md:py-1.5 md:text-sm {activeTab === 'paid' ? 'bg-background text-foreground shadow-sm' : 'text-muted-foreground hover:text-foreground'}" > Đã thanh toán </button> </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 `@src/routes/`(customer)/me/tickets/+page.svelte around lines 47 - 77, Wrap the tab buttons container (the div holding the three buttons) with role="tablist" and mark each button with role="tab" plus aria-selected that reflects the current activeTab value ('all', 'pending', 'paid'); ensure the button that sets activeTab also updates aria-selected to true for the selected button and false for others, and add tabindex="0" for the selected tab and tabindex="-1" for non-selected tabs to follow the ARIA tab pattern (use the existing activeTab variable and the three buttons that set it to locate where to add these attributes).src/lib/components/admin/event/SectionDetail.svelte (1)
11-11: 💤 Low value
SvelteSetis unnecessary here; a plainSetsuffices.
gridColsis built and consumed entirely inside$derived.by(). The reactive boundary is the derived itself (re-runs onsection.seatGridchanges), so per-addreactive bookkeeping fromSvelteSetprovides no benefit and the result is immediately spread into a plain array anyway.♻️ Proposed simplification
- import { SvelteSet } from 'svelte/reactivity'; @@ const gridCols = $derived.by(() => { - const cols = new SvelteSet<number>(); + const cols = new Set<number>(); for (const row of Object.values(section.seatGrid ?? {})) { for (const c of Object.keys(row)) cols.add(Number(c)); } return [...cols].sort((a, b) => a - b); });Also applies to: 113-119
🤖 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/lib/components/admin/event/SectionDetail.svelte` at line 11, Remove the unnecessary SvelteSet usage: delete the import of SvelteSet and replace any SvelteSet construction used to build gridCols inside the $derived.by() computation with a plain JS Set (or simple array) since gridCols is computed and consumed entirely within the derived reactive boundary (see symbols: SvelteSet import, gridCols, $derived.by(), section.seatGrid, add). Ensure the derived still returns a plain array (the spread/result currently used) and that any calls to add on SvelteSet are replaced with Set.prototype.add (or array push) so the rest of the component continues to consume the same array shape.src/lib/components/seat-map/SectionBlock.svelte (1)
7-7: 💤 Low valueSwitch from
SvelteMapto plainMapin the derived block.
seatLookupis recomputed wheneversection.seatschanges, creating a fresh map each time. Since the map is never mutated after creation and the template only reads it via.get(), plainMapis both cheaper and sufficient here.SvelteMapprovides per-set reactive tracking, but there's no consumer benefit when the entire map is recreated on each derived update.♻️ Suggested refactor
- import { SvelteMap } from 'svelte/reactivity'; import SeatItem from './SeatItem.svelte'; @@ let seatLookup = $derived.by(() => { - const map = new SvelteMap<string, SeatMapSeat>(); + const map = new Map<string, SeatMapSeat>(); for (const seat of section.seats) { map.set(`${seat.row_label}-${seat.col_number}`, seat); } return map; });Also applies to: 22-22
🤖 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/lib/components/seat-map/SectionBlock.svelte` at line 7, Replace the use of SvelteMap with a plain Map in the derived computation that builds seatLookup: remove the SvelteMap import and instantiate a new Map instead when mapping over section.seats (the derived block that defines seatLookup), and update any lookups that call .get() to continue working unchanged; do the same replacement for the other occurrence referencing SvelteMap (the second instance around the code that also derives from section.seats) so the map is created immutably on each recompute and no per-set Svelte reactivity is used.
🤖 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 `@src/lib/components/admin/event/SectionDetail.svelte`:
- Around line 121-130: The component currently allows rendering grid data even
when seatCfg is null which risks label mismatches; update the derived guard so
hasGridData requires seatCfg to be present and valid (e.g., change hasGridData
to check seatCfg && gridKeys.length>0 && gridCols.length>0) and ensure dependent
derived values (sectionRows, sectionCols, sectionStartRowIndex,
sectionStartColIndex) still use seatCfg when present; reference the symbols
seatCfg, seatGrid, hasGridData, sectionRows, sectionCols, sectionStartRowIndex,
sectionStartColIndex and the getRowLabel usage in the template to locate and fix
the condition.
In `@src/lib/components/seat-map/SummaryBar.svelte`:
- Around line 114-119: In handleTouchMove, when you pass the guard (isSwiping &&
swipeAllowed && e.touches.length === 1) you must call e.preventDefault() to stop
native scrolling while tracking swipeOffset; update the function (referencing
handleTouchMove, isSwiping, swipeAllowed, swipeStartY, and swipeOffset) to call
e.preventDefault() (guarding by e.cancelable if needed) immediately before
computing dy and setting swipeOffset so the browser doesn’t scroll the cart list
during the swipe gesture.
In `@src/routes/`(customer)/contact/+page.server.ts:
- Around line 60-77: The timeout timer created with AbortController and
timeoutId may not be cleared if fetch throws; wrap the fetch call in a
try/finally (or try/catch/finally) so clearTimeout(timeoutId) always runs in the
finally block and the controller is still available; update the block around the
AbortController, timeoutId, and fetch invocation in the +page.server.ts code
(references: controller, timeoutId, fetch('https://api.web3forms.com/submit'))
to ensure clearTimeout(timeoutId) is executed in finally and the abort timer is
cleaned up regardless of errors.
- Around line 16-23: Wrap the external rate-limit call to
contactFormLimiter.limit(ip) in a try/catch so Upstash/Redis failures don't
throw and abort the server action: call getClientAddress(), then try { const {
success: allowed } = await contactFormLimiter.limit(ip); } catch (err) { log the
error (e.g., console.error or your app logger) and set allowed = true /* or
fallback policy to not block on backend errors */ } and only return fail(429,
...) when allowed is strictly false; reference the getClientAddress,
contactFormLimiter.limit, allowed variable and the existing fail(...) call when
making this change.
In `@src/routes/`(customer)/events/[id]/shows/[showId]/seats/+page.svelte:
- Around line 97-103: The timeout calling queueStore.leave() is scheduled
without tracking its timer, risking a stale teardown if state/session changes;
modify the logic around remaining/hasExpired to store the timer id when calling
setTimeout (e.g., a local variable like leaveTimer), clear any existing timer
before scheduling a new one, and ensure the timer is cleared on relevant state
transitions and component teardown (onDestroy) so queueStore.leave() cannot run
for a changed/new session context.
In `@src/routes/`(customer)/me/+layout.server.ts:
- Line 6: The current throw redirect(302, '/login') call should preserve the
original destination so users return after auth; update the redirect target to
include a redirect query param containing the encoded original URL (use
event.url.pathname plus event.url.search) and call throw redirect(302,
'/login?redirect=' + encodeURIComponent(originalPathAndQuery)) so the layout's
redirect matches the profile-page pattern.
In `@src/routes/`+layout.svelte:
- Around line 24-27: Remove the duplicate favicon links by consolidating to a
single dynamic favicon: in src/routes/+layout.svelte remove the static imported
asset link element that uses href={favicon} and keep only the dynamic <link
rel="icon" type="image/svg+xml" href={faviconHref} /> inside <svelte:head>; also
delete the static <link rel="icon"> in app.html so only the dynamic faviconHref
is emitted. Alternatively, if you prefer to keep the app.html entry, revert to a
single static <link id="favicon"> there and update its href at runtime (instead
of adding new <link> tags) — locate the relevant <svelte:head> block and the
variables favicon and faviconHref to make the change.
---
Duplicate comments:
In `@src/lib/components/customer/event/CategoryFilterBar.svelte`:
- Around line 52-60: The buildHref function currently preserves the current
pagination when changing categories; update it so that when modifying the
'category' param you also remove the 'page' param to reset to page 1. In the
buildHref function (which uses SvelteURLSearchParams and page.url.searchParams
and returns resolve(`/search?${params.toString()}`)), after setting or deleting
the 'category' key ensure params.delete('page') is called so switching
categories clears pagination and avoids landing on empty result pages.
In `@src/lib/components/customer/layout/ProfileSidebar.svelte`:
- Around line 47-51: The handleLogout implementation causes a race:
api.post('/auth/logout') is not awaited so goto(resolve('/'), { invalidateAll:
true }) can run before the server clears the session; replace the button-driven
logout with the sharedLogout form-action flow used elsewhere by using the
sharedLogout utility and a POST form action (e.g.,
action={resolve('/api/auth/logout')} method="POST" with
use:enhance={handleLogout}) instead of calling api.post directly, and remove the
unawaited api.post call from handleLogout so navigation happens after the
server-side logout completes and invalidateAll will see the logged-out state.
In `@src/lib/components/magic/number-ticker/number-ticker.svelte`:
- Around line 38-43: The formatValue function uses decimalPlaces directly in
val.toFixed(decimalPlaces), which can throw a RangeError if decimalPlaces is
non-integer or outside 0–100; guard and normalize it first (e.g., compute a
safeDecimalPlaces by coercing decimalPlaces to an integer with Math.round or
Math.floor, clamp it between 0 and 100, and fallback to a sensible default if
undefined/NaN), then use safeDecimalPlaces in both toFixed and the
Intl.NumberFormat options; update references inside formatValue to use
safeDecimalPlaces instead of decimalPlaces so the component cannot crash at
runtime.
- Line 31: animatedValue is initialized to 0 which causes a visual flash;
instead initialize it to the component's starting position derived from the
current value and direction. Update the initialization of animatedValue (the
$state wrapper) so it uses the current numeric value (e.g., the prop/state value
the ticker should start from) adjusted for direction, and ensure you also sync
it when the input value or direction changes (in the component's setup or
onMount/update logic) so animatedValue starts aligned with the visible starting
position rather than 0.
In `@src/lib/server/config.ts`:
- Line 29: The config currently hardcodes WEB3FORMS_KEY by using
z.string().default('2fe33d39-...'), so remove the default and make the key
required from the environment by changing the schema entry for WEB3FORMS_KEY to
a required string (e.g., z.string() or z.string().nonempty()), and ensure any
code that reads the parsed config (the symbol WEB3FORMS_KEY in your
config/schema object) expects it to be present and fails fast if missing so the
value must be injected via env rather than falling back to a literal default.
---
Nitpick comments:
In `@src/app.html`:
- Line 2: The HTML lang attribute is hardcoded ("<html lang=\"vi\">"); make it
dynamic so the root element reflects the active locale (e.g., derive a locale
variable via SvelteKit load or your i18n store and bind it into the html tag) —
update the <html lang="..."> usage in src/app.html to read the runtime locale
value instead of the literal "vi" and ensure the loader/store that provides the
locale (e.g., a page load returning locale or an i18n getLocale function) is
wired to supply that value.
In `@src/layout.css`:
- Line 106: Two OKLCH color tokens use percent lightness (e.g., --info-muted:
oklch(94.431% 0.01395 219.971);) while the rest use decimal lightness; update
those tokens to use decimal lightness (convert 94.431% → 0.94431, 80.576% →
0.80576) so the OKLCH calls follow the same decimal format across the file, and
ensure you update every occurrence that uses percent notation.
- Line 547: Remove the extra blank line immediately preceding the mix-blend-mode
declaration so the rule appears directly under its selector/declaration block;
edit the mix-blend-mode line (mix-blend-mode: normal;) to have no empty line
above it in src/layout.css to satisfy Stylelint formatting rules.
- Line 353: The global scroll-behavior: smooth rule should respect user motion
preferences: wrap the existing scroll-behavior: smooth declaration inside a
media query that only applies when the user has not requested reduced motion
(use `@media` (prefers-reduced-motion: no-preference)) and target the same element
(e.g., html or :root) so smooth scrolling remains the default for users without
reduced-motion, while users who prefer reduced motion are not affected; update
the existing scroll-behavior: smooth line accordingly.
In `@src/lib/components/admin/event/SectionDetail.svelte`:
- Line 11: Remove the unnecessary SvelteSet usage: delete the import of
SvelteSet and replace any SvelteSet construction used to build gridCols inside
the $derived.by() computation with a plain JS Set (or simple array) since
gridCols is computed and consumed entirely within the derived reactive boundary
(see symbols: SvelteSet import, gridCols, $derived.by(), section.seatGrid, add).
Ensure the derived still returns a plain array (the spread/result currently
used) and that any calls to add on SvelteSet are replaced with Set.prototype.add
(or array push) so the rest of the component continues to consume the same array
shape.
In `@src/lib/components/admin/layout/AdminSidebar.svelte`:
- Around line 105-109: The version badge is currently rendered as a paragraph
element showing the version variable; replace that paragraph element with an
inline span element (keeping the class and the v{version} binding intact) so the
markup is semantically correct for a small inline label, and verify the visual
styling and accessibility remain unchanged after swapping the element.
In `@src/lib/components/customer/event/EventBannerCarousel.svelte`:
- Around line 142-143: The venue text currently uses the CSS utility class
line-clamp-1 which can hide important location details; update the
EventBannerCarousel component to make the full venue discoverable by either
replacing line-clamp-1 with line-clamp-2 on the span that renders {event.venue}
or add a hover tooltip (e.g., title attribute or a Tooltip component) on the
same span so users can view the full venue string, referencing the MapPin
element and the span that contains event.venue for where to apply the change;
test with longest venue names to confirm readability.
- Around line 153-155: The component EventBannerCarousel.svelte contains
hardcoded Vietnamese strings ("Giá vé từ" and "Xem thêm") which should be
extracted for localization; replace these literals in the template with i18n
keys (e.g., use your project's translation helper like $t('event.price_from') or
i18n.t('event.view_more')), add corresponding entries to the translations
resource (Vietnamese and other locales), and update any tests/props that expect
the raw text so the display uses the translation function instead of the
hardcoded strings.
- Around line 166-173: EventBannerCarousel currently renders a neutral CTA text
"Xem thêm"; update the component to render a more action-oriented CTA for
conversion by conditionally switching the label to "Đặt vé ngay" for non-admin
users on the primary discovery path (keep "Xem thêm" for admins or non-primary
flows). Locate the CTA span in EventBannerCarousel (the element containing the
ArrowRight icon) and implement a concise conditional using the existing
user/context flags (e.g., isAdmin, isPrimaryPath or similar props/state) to
choose the label, ensure accessibility by updating the aria-label accordingly,
and wire any tracking/event handlers to the new CTA without changing styling or
the ArrowRight component usage.
In `@src/lib/components/magic/number-ticker/number-ticker.svelte`:
- Around line 58-68: The step function uses a fixed dt of 16.67/1000 which makes
animation frame-rate dependent; replace that with the actual frame delta
computed from timestamps (e.g., dt = (currentTime - lastTime) / 1000 where
lastTime is initialized before the first frame and updated each call) and clamp
dt to a sensible max (to prevent huge jumps after tab switch/frame drop). Update
references to startTime/currentTime handling so you use lastTime for per-frame
integration, and ensure position/velocity are integrated using this variable dt
inside step.
In `@src/lib/components/seat-map/SectionBlock.svelte`:
- Line 7: Replace the use of SvelteMap with a plain Map in the derived
computation that builds seatLookup: remove the SvelteMap import and instantiate
a new Map instead when mapping over section.seats (the derived block that
defines seatLookup), and update any lookups that call .get() to continue working
unchanged; do the same replacement for the other occurrence referencing
SvelteMap (the second instance around the code that also derives from
section.seats) so the map is created immutably on each recompute and no per-set
Svelte reactivity is used.
In `@src/lib/components/seat-map/SummaryBar.svelte`:
- Around line 97-99: The current handler uses a fragile
querySelector('[class*="overflow-y-auto"]') to find the scroll container;
replace it with a direct Svelte element reference by declaring a local variable
(e.g., let scrollContainerEl) in the script, binding that variable to the scroll
container element with bind:this on the DOM node (the element around where the
scrollable area is rendered), and update the handler to use that bound variable
(scrollContainerEl) instead of querying via e.currentTarget; remove the
substring class selector logic so the handler directly reads from the bound
element.
In `@src/routes/`(customer)/events/[id]/shows/[showId]/seats/+page.svelte:
- Around line 701-712: The countdown pulse animation (keyframes named
countdown-pulse and the class animate-countdown-pulse) doesn’t respect users’
reduced-motion preference; update the CSS to disable the animation for users who
set prefers-reduced-motion: reduce by adding a media query that sets animation:
none (and/or removes box-shadow transitions) for .animate-countdown-pulse under
`@media` (prefers-reduced-motion: reduce) so the pulse is effectively turned off
while keeping the original keyframes/class for users without that preference.
In `@src/routes/`(customer)/me/profile/+page.svelte:
- Around line 454-479: The repeated password-field pattern (seen in the block
using showEmailPwd, emailForm.current_password, the Lock/Eye/EyeOff icons and
input id "email_curpwd") should be extracted into a reusable PasswordInput
Svelte component (e.g., PasswordInput.svelte) that accepts props id, bind:value,
placeholder, and icon (lock|key) and manages its own internal show state and
toggle button; replace each in this file (and at the other occurrences around
lines 521-546, 558-583, 594-619) with <PasswordInput id="..." bind:value={...}
placeholder="…" icon="lock|key" /> so you remove duplicated markup and keep the
same aria-label, icon components (Lock, Key), and behavior for toggling between
text/password and updating the bound form fields.
In `@src/routes/`(customer)/me/tickets/+page.svelte:
- Around line 47-77: Wrap the tab buttons container (the div holding the three
buttons) with role="tablist" and mark each button with role="tab" plus
aria-selected that reflects the current activeTab value ('all', 'pending',
'paid'); ensure the button that sets activeTab also updates aria-selected to
true for the selected button and false for others, and add tabindex="0" for the
selected tab and tabindex="-1" for non-selected tabs to follow the ARIA tab
pattern (use the existing activeTab variable and the three buttons that set it
to locate where to add these attributes).
In `@src/routes/`+layout.svelte:
- Line 2: Remove the unused import favicon from the top of +layout.svelte if you
also remove the favicon link usage (previously referenced where favicon was
used); locate the import statement "import favicon from
'$lib/assets/favicon.svg';" and delete it so there are no unused imports or
linter warnings after consolidating favicon links.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 518e53af-0e7a-4279-941a-8697013d075a
⛔ Files ignored due to path filters (6)
bun.lockis excluded by!**/*.locksrc/lib/assets/favicon-active.svgis excluded by!**/*.svgsrc/lib/assets/favicon.svgis excluded by!**/*.svgstatic/favicon-active.svgis excluded by!**/*.svgstatic/favicon.svgis excluded by!**/*.svgstatic/og-image.pngis excluded by!**/*.png
📒 Files selected for processing (34)
.env.exampleREADME.mdpackage.jsonsrc/app.htmlsrc/layout.csssrc/lib/components/ToastContainer.sveltesrc/lib/components/admin/event/SectionDetail.sveltesrc/lib/components/admin/layout/AdminHeader.sveltesrc/lib/components/admin/layout/AdminSidebar.sveltesrc/lib/components/customer/event/CategoryFilterBar.sveltesrc/lib/components/customer/event/EventBannerCarousel.sveltesrc/lib/components/customer/layout/CustomerNavbar.sveltesrc/lib/components/customer/layout/ProfileSidebar.sveltesrc/lib/components/customer/order/EventTicketCard.sveltesrc/lib/components/magic/number-ticker/number-ticker.sveltesrc/lib/components/seat-map/SectionBlock.sveltesrc/lib/components/seat-map/SummaryBar.sveltesrc/lib/components/ui/bento/BentoStat.sveltesrc/lib/server/config.tssrc/lib/server/rate-limiter.tssrc/lib/utils/api.tssrc/routes/(admin)/admin/events/+page.sveltesrc/routes/(admin)/admin/events/[id]/+page.sveltesrc/routes/(customer)/+page.sveltesrc/routes/(customer)/contact/+page.server.tssrc/routes/(customer)/events/[id]/+page.sveltesrc/routes/(customer)/events/[id]/shows/[showId]/seats/+page.sveltesrc/routes/(customer)/me/+layout.server.tssrc/routes/(customer)/me/profile/+page.server.tssrc/routes/(customer)/me/profile/+page.sveltesrc/routes/(customer)/me/tickets/+page.server.tssrc/routes/(customer)/me/tickets/+page.sveltesrc/routes/+error.sveltesrc/routes/+layout.svelte
✅ Files skipped from review due to trivial changes (5)
- .env.example
- README.md
- package.json
- src/routes/(customer)/+page.svelte
- src/routes/(admin)/admin/events/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (9)
- src/routes/(customer)/me/tickets/+page.server.ts
- src/lib/utils/api.ts
- src/routes/(customer)/events/[id]/+page.svelte
- src/lib/components/ui/bento/BentoStat.svelte
- src/lib/components/ToastContainer.svelte
- src/routes/+error.svelte
- src/lib/components/admin/layout/AdminHeader.svelte
- src/lib/components/customer/layout/CustomerNavbar.svelte
- src/lib/components/customer/order/EventTicketCard.svelte
Comment on lines
+114
to
+119
| function handleTouchMove(e: TouchEvent) { | ||
| if (!isSwiping || !swipeAllowed || e.touches.length !== 1) return; | ||
| const dy = e.touches[0].clientY - swipeStartY; | ||
| // Only track downward swipes | ||
| swipeOffset = Math.max(0, dy); | ||
| } |
Contributor
There was a problem hiding this comment.
Missing preventDefault() causes scroll conflict.
When isSwiping is true and offset is being tracked, native scroll events are not prevented. This allows the browser to scroll the cart list while simultaneously translating the panel, creating confusing dual behavior. Call e.preventDefault() after the guard to suppress native scrolling during swipe gestures.
🛠️ Proposed fix
function handleTouchMove(e: TouchEvent) {
if (!isSwiping || !swipeAllowed || e.touches.length !== 1) return;
+ e.preventDefault();
const dy = e.touches[0].clientY - swipeStartY;
// Only track downward swipes
swipeOffset = Math.max(0, dy);
}📝 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 handleTouchMove(e: TouchEvent) { | |
| if (!isSwiping || !swipeAllowed || e.touches.length !== 1) return; | |
| const dy = e.touches[0].clientY - swipeStartY; | |
| // Only track downward swipes | |
| swipeOffset = Math.max(0, dy); | |
| } | |
| function handleTouchMove(e: TouchEvent) { | |
| if (!isSwiping || !swipeAllowed || e.touches.length !== 1) return; | |
| e.preventDefault(); | |
| const dy = e.touches[0].clientY - swipeStartY; | |
| // Only track downward swipes | |
| swipeOffset = Math.max(0, dy); | |
| } |
🤖 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/lib/components/seat-map/SummaryBar.svelte` around lines 114 - 119, In
handleTouchMove, when you pass the guard (isSwiping && swipeAllowed &&
e.touches.length === 1) you must call e.preventDefault() to stop native
scrolling while tracking swipeOffset; update the function (referencing
handleTouchMove, isSwiping, swipeAllowed, swipeStartY, and swipeOffset) to call
e.preventDefault() (guarding by e.cancelable if needed) immediately before
computing dy and setting swipeOffset so the browser doesn’t scroll the cart list
during the swipe gesture.
Comment on lines
+24
to
+27
| <svelte:head> | ||
| <link rel="icon" href={favicon} /> | ||
| <link rel="icon" type="image/svg+xml" href={faviconHref} /> | ||
| </svelte:head> |
Contributor
There was a problem hiding this comment.
Consolidate redundant favicon definitions.
The current implementation creates three <link rel="icon"> elements in the final HTML:
- The imported asset on line 25
- The dynamic favicon on line 26
- The static link in
src/app.htmlline 44
This redundancy can cause browser confusion about which favicon to use. Consider one of these approaches:
Option 1 (recommended): Remove line 25 and the static link in app.html, keeping only the dynamic favicon:
<svelte:head>
- <link rel="icon" href={favicon} />
<link rel="icon" type="image/svg+xml" href={faviconHref} />
</svelte:head>Option 2: Keep the static link in app.html with id="favicon" and update its href attribute via JavaScript instead of adding new links:
onMount(() => {
+ const faviconLink = document.getElementById('favicon');
faviconHref = document.hidden ? '/favicon.svg' : '/favicon-active.svg';
+ if (faviconLink) faviconLink.setAttribute('href', faviconHref);
const handler = () => {
faviconHref = document.hidden ? '/favicon.svg' : '/favicon-active.svg';
+ if (faviconLink) faviconLink.setAttribute('href', faviconHref);
};📝 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
| <svelte:head> | |
| <link rel="icon" href={favicon} /> | |
| <link rel="icon" type="image/svg+xml" href={faviconHref} /> | |
| </svelte:head> | |
| <svelte:head> | |
| <link rel="icon" type="image/svg+xml" href={faviconHref} /> | |
| </svelte:head> |
🤖 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/routes/`+layout.svelte around lines 24 - 27, Remove the duplicate favicon
links by consolidating to a single dynamic favicon: in src/routes/+layout.svelte
remove the static imported asset link element that uses href={favicon} and keep
only the dynamic <link rel="icon" type="image/svg+xml" href={faviconHref} />
inside <svelte:head>; also delete the static <link rel="icon"> in app.html so
only the dynamic faviconHref is emitted. Alternatively, if you prefer to keep
the app.html entry, revert to a single static <link id="favicon"> there and
update its href at runtime (instead of adding new <link> tags) — locate the
relevant <svelte:head> block and the variables favicon and faviconHref to make
the change.
…unctionality - Updated SeatMap.svelte to refine touch panning and zooming behavior. - Modified SectionBlock.svelte for better quantity selector styling and layout. - Improved SectionLegend.svelte to sort sections and enhance availability display. - Refactored queue page (+page.svelte) for better countdown handling and UI responsiveness. - Enhanced seats page (+page.svelte) with improved header and timer display.
This comment has been minimized.
This comment has been minimized.
…g and consistency
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/lib/components/seat-map/SectionBlock.svelte (1)
206-207:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce global ticket limit in both “+” controls to prevent silent failure.
At Line 206 and Line 273, increment stays enabled after global max is reached (
store.isAtLimit), so users can still click+with no effective change. This was already reported earlier and remains unresolved.💡 Suggested fix
function increment() { - if (quantity < availableCount) { + if (store.isAtLimit) return; + if (quantity < availableCount) { store.setGeneralQuantity(section.id, quantity + 1); } } ... - disabled={quantity >= availableCount} + disabled={quantity >= availableCount || store.isAtLimit} ... - disabled={quantity >= availableCount} + disabled={quantity >= availableCount || store.isAtLimit}Also applies to: 273-274
🤖 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/lib/components/seat-map/SectionBlock.svelte` around lines 206 - 207, Both "+" controls still allow clicks after the global limit (store.isAtLimit) is reached, causing silent no-ops; update the disable logic and the click handlers so both controls are disabled when either quantity >= availableCount OR store.isAtLimit, and also guard the increment action itself by checking store.isAtLimit before performing the increment inside the increment handler (and any other increment-like handler used by the second control) to ensure no action occurs when at the global limit.src/routes/(customer)/events/[id]/queue/+page.svelte (1)
21-37:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
holdingcountdown still gets stuck at 0:00 with no recovery.The expiration check at line 29 only transitions to
missedwhenstatus === 'ready'. Because polling is intentionally disabled forholding(lines 99 and 101), a user sitting on this page in theholdingstate (e.g., after navigating back from the seats page via the "Tiếp tục chọn ghế" button at lines 391-398) will hittimeLeft === 0and remain on a frozen 0:00 screen with no way out — the seats-page expiry handler can't fire when the user isn't on that page.Either include
holdingin the local expiry handler here, or re-enable a lightweight server check while holding so the client converges.🐛 Proposed fix
const update = () => { timeLeft = Math.max(0, Math.floor((queueStore.expiresAt! - Date.now()) / 1000)); - if (timeLeft === 0 && queueStore.status === 'ready') { + if (timeLeft === 0 && (queueStore.status === 'ready' || queueStore.status === 'holding')) { queueStore.status = 'missed'; queueStore.leave(); } };🤖 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/routes/`(customer)/events/[id]/queue/+page.svelte around lines 21 - 37, The countdown handler in the $effect uses update() to mark expiry only for status 'ready', which causes a stuck 0:00 when queueStore.status === 'holding'; modify the update logic inside the $effect so that when timeLeft === 0 it also handles the 'holding' state (e.g., treat 'holding' like 'ready' and set queueStore.status = 'missed' and call queueStore.leave()), or alternatively re-enable a lightweight server poll while status === 'holding' so the client converges; update the conditions referencing queueStore.status, queueStore.expiresAt, timeLeft, and the update() function accordingly.
🧹 Nitpick comments (2)
src/routes/(customer)/events/[id]/queue/+page.svelte (2)
112-122: 💤 Low valueUnused noise filter — looks like dead markup.
This top-level
<svg>defines<filter id="noise">but no element in this file referencesurl(#noise). The actual noise overlays at lines 237 and 328 embed their ownid="n"filter inside an inlinedata:URI, so this block is never used and can be removed.♻️ Proposed cleanup
-<!-- Noise texture filter (invisible, referenced via CSS) --> -<svg class="pointer-events-none absolute size-0" aria-hidden="true"> - <defs> - <filter id="noise"> - <feTurbulence type="fractalNoise" baseFrequency="0.65" numOctaves="3" stitchTiles="stitch" /> - <feColorMatrix type="saturate" values="0" /> - <feBlend in="SourceGraphic" mode="overlay" result="blend" /> - <feComposite in="blend" in2="SourceGraphic" operator="in" /> - </filter> - </defs> -</svg> -🤖 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/routes/`(customer)/events/[id]/queue/+page.svelte around lines 112 - 122, The top-level unused SVG filter <filter id="noise"> is dead markup and should be removed; delete the entire <svg class="pointer-events-none absolute size-0">…</svg> block (the one that defines <filter id="noise">) so the file no longer contains an orphaned noise filter, since the actual noise overlays use inline filters (id="n") elsewhere.
234-238: 💤 Low valueExtract the duplicated noise overlay into a shared style.
The inline
data:image/svg+xml,...stylestring here is byte-identical to the one at line 328 in theholdingblock. Extracting it (e.g., into a<style>block or a CSS custom property on the page-level container) avoids drift if the noise pattern is ever tweaked, and keeps the markup readable.♻️ Sketch
<style> .noise-overlay { background: url("data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' width='200' height='200'><filter id='n'><feTurbulence type='fractalNoise' baseFrequency='0.65' numOctaves='3' stitchTiles='stitch'/></filter><rect width='200' height='200' filter='url(%23n)' opacity='1'/></svg>"); background-size: 200px; } </style>Then replace the two inline
style="background: ..."overlays withclass="pointer-events-none absolute inset-0 opacity-[0.035] noise-overlay".🤖 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/routes/`(customer)/events/[id]/queue/+page.svelte around lines 234 - 238, The duplicated inline SVG noise background in +page.svelte (present in the noise overlay div and again in the holding block) should be moved into a shared CSS rule or custom property (e.g., create a .noise-overlay class or --noise-bg variable in the component's <style>) and both divs updated to use class="pointer-events-none absolute inset-0 opacity-[0.035] noise-overlay" (or reference the custom property) so the SVG data URI is maintained in one place and the inline style removed.
🤖 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 `@src/layout.css`:
- Around line 541-543: Remove the blank line separating declarations in the CSS
rule that contains the properties animation and mix-blend-mode: delete the empty
line between the "animation" declaration and the "mix-blend-mode" declaration so
the two properties are adjacent in the same rule block to satisfy Stylelint's
declaration-empty-line-before rule.
In `@src/lib/components/seat-map/SectionBlock.svelte`:
- Around line 192-209: The icon-only stepper buttons in SectionBlock.svelte (the
button elements that call decrement and increment and render <Minus /> and <Plus
/>) need accessible names: add aria-label attributes to the decrement and
increment buttons (e.g., "Decrease quantity" and "Increase quantity" or a
dynamically composed label using the section/seat context and the current
quantity) so assistive tech can identify their actions; ensure the attributes
are present alongside the existing disabled logic that references quantity and
availableCount so behavior doesn't change.
---
Duplicate comments:
In `@src/lib/components/seat-map/SectionBlock.svelte`:
- Around line 206-207: Both "+" controls still allow clicks after the global
limit (store.isAtLimit) is reached, causing silent no-ops; update the disable
logic and the click handlers so both controls are disabled when either quantity
>= availableCount OR store.isAtLimit, and also guard the increment action itself
by checking store.isAtLimit before performing the increment inside the increment
handler (and any other increment-like handler used by the second control) to
ensure no action occurs when at the global limit.
In `@src/routes/`(customer)/events/[id]/queue/+page.svelte:
- Around line 21-37: The countdown handler in the $effect uses update() to mark
expiry only for status 'ready', which causes a stuck 0:00 when queueStore.status
=== 'holding'; modify the update logic inside the $effect so that when timeLeft
=== 0 it also handles the 'holding' state (e.g., treat 'holding' like 'ready'
and set queueStore.status = 'missed' and call queueStore.leave()), or
alternatively re-enable a lightweight server poll while status === 'holding' so
the client converges; update the conditions referencing queueStore.status,
queueStore.expiresAt, timeLeft, and the update() function accordingly.
---
Nitpick comments:
In `@src/routes/`(customer)/events/[id]/queue/+page.svelte:
- Around line 112-122: The top-level unused SVG filter <filter id="noise"> is
dead markup and should be removed; delete the entire <svg
class="pointer-events-none absolute size-0">…</svg> block (the one that defines
<filter id="noise">) so the file no longer contains an orphaned noise filter,
since the actual noise overlays use inline filters (id="n") elsewhere.
- Around line 234-238: The duplicated inline SVG noise background in
+page.svelte (present in the noise overlay div and again in the holding block)
should be moved into a shared CSS rule or custom property (e.g., create a
.noise-overlay class or --noise-bg variable in the component's <style>) and both
divs updated to use class="pointer-events-none absolute inset-0 opacity-[0.035]
noise-overlay" (or reference the custom property) so the SVG data URI is
maintained in one place and the inline style removed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39eff0ca-f662-4f0f-b4d1-22a624afe78c
📒 Files selected for processing (6)
src/layout.csssrc/lib/components/seat-map/SeatMap.sveltesrc/lib/components/seat-map/SectionBlock.sveltesrc/lib/components/seat-map/SectionLegend.sveltesrc/routes/(customer)/events/[id]/queue/+page.sveltesrc/routes/(customer)/events/[id]/shows/[showId]/seats/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/components/seat-map/SeatMap.svelte
- src/routes/(customer)/events/[id]/shows/[showId]/seats/+page.svelte
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| export const actions = { | ||
| default: async ({ request, getClientAddress }) => { | ||
| const ip = getClientAddress(); | ||
| let allowed = true; |
HungND-flocus
approved these changes
May 9, 2026
Comment on lines
+5
to
+7
| if (!locals.user) { | ||
| throw redirect(302, `/login?redirect=${encodeURIComponent(url.pathname)}`); | ||
| } |
There was a problem hiding this comment.
Redirect URL construction omits url.search, dropping state parameters like ?page=2 or ?tab=history and causing users to lose context after login. Include url.pathname + url.search to preserve the full query string in the redirect payload.
if (!locals.user) {
throw redirect(302, `/login?redirect=${encodeURIComponent(url.pathname + url.search)}`);
}Prompt for LLM
File src/routes/(customer)/me/+layout.server.ts:
Line 5 to 7:
WHAT: The redirect URL construction omits `url.search`, dropping any query string parameters. WHY: If an unauthenticated user attempts to access a URL with state parameters (e.g., `/me/orders?page=2` or `?tab=history`), those parameters are lost in the `redirect` payload, sending them back to the base path after login. HOW: Include both the pathname and the search string when building the redirect URL: `url.pathname + url.search`.
Suggested Code:
if (!locals.user) {
throw redirect(302, `/login?redirect=${encodeURIComponent(url.pathname + url.search)}`);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This pull request introduces significant improvements to the user interface, user experience, styling, and responsiveness across the application, along with new content pages and refactoring.
Key changes include:
Enhanced UI/UX & Responsiveness:
backdrop-filtersupport, and adjusted globalinfocolor variables.New Features & Content Pages:
AnimatedThemeTogglerfor a smooth theme switch with view transitions,AnimatedGridPatternandDotPatternfor decorative animated backgrounds, andNumberTickerfor dynamic number animations.EventTimelinecomponent to display detailed event itineraries within event detail pages./about), "Careers" (/careers) with job listings and filtering, "Contact Us" (/contact) with a functional submission form, "Frequently Asked Questions" (/faq) with search and category filters, and a comprehensive "Privacy Policy" (/privacy) compliant with local regulations.Refactoring & Code Quality:
src/lib/utils/auth.ts) for consistent behavior across admin and customer interfaces.?redirect=query parameter, allowing users to return to their intended page after authentication.svelte.config.jsto includeworker-srcin CSP for development andvite.config.tswith server warmup and dependency optimization.ToastContainerandBentoStat./searchpage.Summary by CodeRabbit
New Features
Bug Fixes
UI/UX