v-3388: Update all skeleton loaders on mobile and desktop#127
Open
v-3388: Update all skeleton loaders on mobile and desktop#127
Conversation
Replace generic pulse placeholders with structure-matching skeletons that mirror each real component's heights, spacing and markup — so transitions from skeleton to loaded state don't cause layout shift. Extract every skeleton into its own file under `src/components/**/` for reuse between inline loading states, Next.js `loading.tsx` Suspense fallbacks, and layout-level dispatch in `account/layout.tsx`. Also split the login form out of `/account` into a dedicated `/account/login` route: `/account` is now dashboard-only (with an auth redirect to `/account/login`), and the account-button links in Header / Footer / MobileMenu route anonymous users straight to `/account/login` via a new `AccountLink` client component. Pages covered: homepage featured products, cart (2-item scenario), checkout, order-placed, category listing + filter bar, and every account sub-page (dashboard, profile, orders, gift cards, credit cards, addresses, login, register). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 46 seconds. ⌛ 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
WalkthroughThis PR reorganizes the storefront's account and authentication flows by creating a dedicated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
src/components/account/LoginFormSkeleton.tsx (1)
16-42: Consider adding accessibility attributes for screen readers.Skeleton loaders should include accessibility attributes to inform assistive technology users that content is loading. Consider adding
aria-busy="true"andaria-labelto the wrapper div.♿ Proposed accessibility enhancement
- <div className="max-w-md mx-auto px-4 sm:px-6 lg:px-8 py-16"> + <div + className="max-w-md mx-auto px-4 sm:px-6 lg:px-8 py-16" + aria-busy="true" + aria-label="Loading login form" + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/account/LoginFormSkeleton.tsx` around lines 16 - 42, The LoginFormSkeleton wrapper div lacks accessibility attributes to indicate loading state; update the outermost div in the LoginFormSkeleton component to include aria-busy="true" and a descriptive aria-label (e.g., "Signing in, content loading") and consider adding role="status" or aria-live="polite" on the same wrapper so screen readers are informed the skeleton is a loading placeholder.src/components/account/AuthFallbackSkeleton.tsx (1)
9-13: Consider honoring reduced-motion preferences for the pulse effect.Nice skeleton structure. As an accessibility polish, add
motion-reduce:animate-nonewhereanimate-pulseis used.Suggested tweak
- <div className="animate-pulse space-y-4"> + <div className="animate-pulse motion-reduce:animate-none space-y-4">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/account/AuthFallbackSkeleton.tsx` around lines 9 - 13, The skeleton uses a pulsing animation that should respect users' reduced-motion preferences; update the JSX in AuthFallbackSkeleton (the div with className "animate-pulse space-y-4") to include the Tailwind utility for reduced motion (e.g., add "motion-reduce:animate-none") so the pulse stops for users who prefer reduced motion while keeping the existing layout and classes intact.src/app/[country]/[locale]/(storefront)/account/gift-cards/loading.tsx (1)
3-5: Add an explicit return type to the loading component.Line 3 should declare a return type (for example
: JSX.Element) to match the repository’s strict TS typing style.♻️ Proposed change
-export default function GiftCardsLoading() { +export default function GiftCardsLoading(): JSX.Element { return <GiftCardsSkeleton />; }As per coding guidelines, "Use strict TypeScript type checking. Always define explicit return types for functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/account/gift-cards/loading.tsx around lines 3 - 5, The loading component GiftCardsLoading is missing an explicit return type; update its declaration to include a strict TypeScript return type (for example add ": JSX.Element" to the GiftCardsLoading function signature) so the function signature explicitly declares the returned JSX element, matching the repository's strict TS typing rules.src/components/order/OrderPlacedSkeleton.tsx (1)
11-11: Add an explicit return type on the skeleton component.Line 11 should include a return type (
: JSX.Element) to align with repository TS typing rules.As per coding guidelines, "Use strict TypeScript type checking. Always define explicit return types for functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/order/OrderPlacedSkeleton.tsx` at line 11, Add an explicit return type to the skeleton component function: update the OrderPlacedSkeleton declaration to include ": JSX.Element" as its return type (i.e., function OrderPlacedSkeleton(): JSX.Element) so it conforms to the repository's strict TypeScript typing rules and explicit return-type guideline.src/components/account/ProfileSkeleton.tsx (1)
7-60: Add an explicit return type to the new component.This repo’s TS guideline expects new functions/components to declare their return type explicitly. As per coding guidelines, "Use strict TypeScript type checking. Always define explicit return types for functions, use 'satisfies' for type checking object literals, and avoid 'any' (use 'unknown' instead)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/account/ProfileSkeleton.tsx` around lines 7 - 60, The ProfileSkeleton component is missing an explicit return type; update the function signature for ProfileSkeleton to declare its return type (e.g., : JSX.Element or React.ReactElement) so it conforms to the repo's strict TS guidelines—locate the export function ProfileSkeleton and add the explicit return type to the signature without changing the implementation body.src/app/[country]/[locale]/(storefront)/account/login/page.tsx (1)
1-23: Keeppage.tsxserver-side so this route can export metadata.Because this route file is marked
"use client", it can't definegenerateMetadata. The new dedicated login page will miss its page-level title/description unless the interactive form moves into a client child component andpage.tsxstays server-rendered.As per coding guidelines, "Use Server Components by default. Only add 'use client' when you need event handlers, hooks like useState/useReducer/useEffect/useContext, browser-only APIs, or custom hooks that use state/effects." and "Use the Metadata API with generateMetadata function to set page titles, descriptions, and Open Graph images for SEO."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/account/login/page.tsx around lines 1 - 23, The file is incorrectly marked "use client" which prevents exporting generateMetadata; remove the top-level "use client" and keep LoginPage as a server component that exports generateMetadata, then extract the interactive form and any client-only hooks/usages (useState, useEffect, useRouter, usePathname, useSearchParams, useAuth, Eye/EyeOff handlers, etc.) into a new client component (e.g., LoginForm) and import that into LoginPage; ensure LoginPage remains server-side, defines generateMetadata for title/description, and renders the client LoginForm component where the form UI (Input, Button, stateful logic) previously lived.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`[country]/[locale]/(storefront)/account/layout.tsx:
- Around line 163-167: The redirect currently sends anonymous users to
`${basePath}/account/login` without preserving the originally requested account
route; update the useEffect redirect (the useEffect that checks loading,
isAuthenticated, isAuthPage and calls router.replace) to append a `redirect`
query parameter with the current requested path (use router.asPath or
router.pathname+router.query) encoded (e.g., via encodeURIComponent) so
LoginPage can read it and return the user to their original page; keep the same
guard conditions and dependencies (loading, isAuthenticated, isAuthPage,
basePath, router) and ensure the constructed URL uses basePath then
`/account/login?redirect=<encoded-original-path>`.
In `@src/app/`[country]/[locale]/(storefront)/account/login/page.tsx:
- Around line 51-64: handleSubmit currently awaits login(email, password)
without a try/catch/finally, so if login throws the error path is skipped and
setLoading(false) never runs; update handleSubmit to wrap the await login(...)
in try/catch/finally: in try call login and handle success
(router.push(redirectUrl ?? `${basePath}/account`)), in catch setError to a
generic message or the caught error, and in finally always call
setLoading(false) to ensure the submit state is cleared even on exceptions.
- Around line 31-32: The redirect query value (redirectUrl) is used directly in
router.push causing open-redirect/XSS risk; validate and sanitize it by
implementing/using safeRedirect to ensure the value is an app-local path under
the current basePath and explicitly reject protocol-relative and non-relative
schemes (e.g., javascript:, //, http(s) outside basePath). Replace direct uses
of redirectUrl ?? `${basePath}/account` in both router.push calls with
safeRedirect(redirectUrl, `${basePath}/account`) and ensure safeRedirect returns
only validated paths or the provided fallback.
In `@src/components/account/AddressesSkeleton.tsx`:
- Around line 24-27: In AddressesSkeleton change the action placeholders used
for "Edit (link sm) + Delete (destructive sm)" so their height matches the real
buttons to avoid layout shift: locate the two divs in the AddressesSkeleton
component (the flex gap-2 container with the h-5 placeholders) and update their
className heights to the actual control height used for small buttons (e.g.,
h-8/h-9), keeping widths, rounded-md and animate-pulse intact; ensure both
placeholders use the same height as the real Edit/Delete controls so the card
size doesn't change when real buttons render.
In `@src/components/account/ContentSkeleton.tsx`:
- Line 5: The ContentSkeleton function lacks an explicit return type; update the
function signature for ContentSkeleton to include an explicit JSX return type
(for example: JSX.Element) to satisfy strict TS rules—locate the ContentSkeleton
declaration and annotate its return type accordingly while keeping the
implementation unchanged.
In `@src/components/account/CreditCardsSkeleton.tsx`:
- Around line 22-23: The remove-button placeholder in CreditCardsSkeleton uses
class "h-5" causing layout jump when the real small button mounts; update the
placeholder div in the CreditCardsSkeleton component (the remove-action
placeholder div) to use the small-button height class (e.g., "h-9") and keep the
rest of its classes (w-20 bg-gray-200 rounded-md animate-pulse) so the skeleton
matches the real control size.
In `@src/components/account/LoginFormSkeleton.tsx`:
- Line 14: The LoginFormSkeleton function lacks an explicit return-type
annotation; update its signature (the LoginFormSkeleton function) to include a
JSX return type (e.g., add : JSX.Element or React.ReactElement) so the
declaration becomes an explicit typed function returning JSX, ensuring the
component's output type is clear to TypeScript.
- Line 35: The skeleton uses a non-existent Tailwind class `h-13`; in the
LoginFormSkeleton component replace `h-13` on the skeleton div with a standard
Tailwind height (e.g., `h-14` or `h-12`) to match intended sizing—update the div
with className "h-14 bg-gray-200 rounded-lg w-full animate-pulse" (or `h-12` if
the smaller size is desired) so the utility is valid and the lg button sizing
aligns.
In `@src/components/layout/AccountLink.tsx`:
- Around line 7-12: AccountLink currently defines a narrow props interface and
drops forwarded props used by Slot consumers like Button asChild and SheetClose
asChild; update the AccountLink component to accept all props a Link supports by
replacing its props type with Omit<ComponentProps<typeof Link>, "href"> (or
equivalent) and spread the incoming props onto the Link element (while still
constructing the href from basePath or combining basePath into the href as
needed), ensuring children are preserved and className/aria-label/data-*
attributes from parent Slot compositions are forwarded; modify the AccountLink
function signature and the JSX return to spread ...props into Link so Button
asChild / SheetClose asChild compositions receive their injected attributes.
In `@src/components/order/OrderPlacedSkeleton.tsx`:
- Around line 40-55: OrderPlacedSkeleton currently hardcodes too few placeholder
rows causing layout shifts when OrderTotals or AddressBlock render more lines;
update OrderPlacedSkeleton to reserve space equal to the maximum possible rows
(or render dynamic placeholders) by either 1) matching the placeholder count to
the max rows used by OrderTotals and AddressBlock (render arrays of skeleton
rows instead of hardcoded three/four), or 2) applying a stable min-height via
CSS classes on the totals and address container elements so height doesn’t jump;
locate and change the JSX in OrderPlacedSkeleton where the totals/address
skeleton rows are created and make the count/dimensions mirror OrderTotals and
AddressBlock.
---
Nitpick comments:
In `@src/app/`[country]/[locale]/(storefront)/account/gift-cards/loading.tsx:
- Around line 3-5: The loading component GiftCardsLoading is missing an explicit
return type; update its declaration to include a strict TypeScript return type
(for example add ": JSX.Element" to the GiftCardsLoading function signature) so
the function signature explicitly declares the returned JSX element, matching
the repository's strict TS typing rules.
In `@src/app/`[country]/[locale]/(storefront)/account/login/page.tsx:
- Around line 1-23: The file is incorrectly marked "use client" which prevents
exporting generateMetadata; remove the top-level "use client" and keep LoginPage
as a server component that exports generateMetadata, then extract the
interactive form and any client-only hooks/usages (useState, useEffect,
useRouter, usePathname, useSearchParams, useAuth, Eye/EyeOff handlers, etc.)
into a new client component (e.g., LoginForm) and import that into LoginPage;
ensure LoginPage remains server-side, defines generateMetadata for
title/description, and renders the client LoginForm component where the form UI
(Input, Button, stateful logic) previously lived.
In `@src/components/account/AuthFallbackSkeleton.tsx`:
- Around line 9-13: The skeleton uses a pulsing animation that should respect
users' reduced-motion preferences; update the JSX in AuthFallbackSkeleton (the
div with className "animate-pulse space-y-4") to include the Tailwind utility
for reduced motion (e.g., add "motion-reduce:animate-none") so the pulse stops
for users who prefer reduced motion while keeping the existing layout and
classes intact.
In `@src/components/account/LoginFormSkeleton.tsx`:
- Around line 16-42: The LoginFormSkeleton wrapper div lacks accessibility
attributes to indicate loading state; update the outermost div in the
LoginFormSkeleton component to include aria-busy="true" and a descriptive
aria-label (e.g., "Signing in, content loading") and consider adding
role="status" or aria-live="polite" on the same wrapper so screen readers are
informed the skeleton is a loading placeholder.
In `@src/components/account/ProfileSkeleton.tsx`:
- Around line 7-60: The ProfileSkeleton component is missing an explicit return
type; update the function signature for ProfileSkeleton to declare its return
type (e.g., : JSX.Element or React.ReactElement) so it conforms to the repo's
strict TS guidelines—locate the export function ProfileSkeleton and add the
explicit return type to the signature without changing the implementation body.
In `@src/components/order/OrderPlacedSkeleton.tsx`:
- Line 11: Add an explicit return type to the skeleton component function:
update the OrderPlacedSkeleton declaration to include ": JSX.Element" as its
return type (i.e., function OrderPlacedSkeleton(): JSX.Element) so it conforms
to the repository's strict TypeScript typing rules and explicit return-type
guideline.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a840eb3f-433e-4b08-9dbe-e244a71b3709
📒 Files selected for processing (36)
src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/app/[country]/[locale]/(checkout)/order-placed/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/account/addresses/loading.tsxsrc/app/[country]/[locale]/(storefront)/account/credit-cards/loading.tsxsrc/app/[country]/[locale]/(storefront)/account/forgot-password/page.tsxsrc/app/[country]/[locale]/(storefront)/account/gift-cards/loading.tsxsrc/app/[country]/[locale]/(storefront)/account/layout.tsxsrc/app/[country]/[locale]/(storefront)/account/login/page.tsxsrc/app/[country]/[locale]/(storefront)/account/orders/loading.tsxsrc/app/[country]/[locale]/(storefront)/account/page.tsxsrc/app/[country]/[locale]/(storefront)/account/register/page.tsxsrc/app/[country]/[locale]/(storefront)/account/reset-password/page.tsxsrc/app/[country]/[locale]/(storefront)/c/[...permalink]/loading.tsxsrc/app/[country]/[locale]/(storefront)/cart/page.tsxsrc/components/account/AccountDashboardSkeleton.tsxsrc/components/account/AddressesSkeleton.tsxsrc/components/account/AuthFallbackSkeleton.tsxsrc/components/account/ContentSkeleton.tsxsrc/components/account/CreditCardsSkeleton.tsxsrc/components/account/GiftCardsSkeleton.tsxsrc/components/account/LoginFormSkeleton.tsxsrc/components/account/OrdersListSkeleton.tsxsrc/components/account/ProfileSkeleton.tsxsrc/components/account/RegisterFormSkeleton.tsxsrc/components/account/SidebarUserInfoSkeleton.tsxsrc/components/cart/CartSkeleton.tsxsrc/components/category/CategoryPageSkeleton.tsxsrc/components/checkout/CheckoutPageSkeleton.tsxsrc/components/layout/AccountLink.tsxsrc/components/layout/Footer.tsxsrc/components/layout/Header.tsxsrc/components/layout/MobileMenu.tsxsrc/components/order/OrderPlacedSkeleton.tsxsrc/components/products/ProductCardSkeleton.tsxsrc/components/products/filters/FilterBarSkeleton.tsxsrc/components/products/filters/ProductFilters.tsx
- ForgotPasswordFormSkeleton and ResetPasswordFormSkeleton replace the generic AuthFallbackSkeleton for `/account/forgot-password` and `/account/reset-password` — each mirrors its real Card layout (header, field count, submit h-13, back-to-sign-in link). - CartDrawerSkeleton extracts the inline pulse placeholder from `CartDrawer.tsx` into its own reusable file, matching the project convention of keeping all skeletons in separate components. AuthFallbackSkeleton stays as a generic fallback for any future auth page. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ot props
- `account/layout.tsx` — anonymous-user redirect now preserves the originally
requested path via `?redirect=<encoded-pathname>` so LoginPage can send the
user back after they sign in.
- `account/login/page.tsx` — `handleSubmit` wrapped in try/catch/finally so the
submit state is always cleared even on exceptions; the `?redirect=` target is
now validated through `safeRedirect()` to prevent open-redirect attacks
(rejects absolute URLs, protocol-relative paths, `javascript:`, etc.).
- `components/layout/AccountLink.tsx` — props changed to
`Omit<ComponentProps<typeof Link>, "href"> & { basePath }` and spread onto
`<Link>`, so the component composes correctly with Radix Slot consumers like
`<Button asChild>` and `<SheetClose asChild>` that inject
className / onClick / data-state / ref onto the child.
- `AuthFallbackSkeleton.tsx` — added `motion-reduce:animate-none` on the
pulsing wrapper so users with reduced-motion preferences aren't forced to
see the animation.
- Added explicit `React.JSX.Element` return types to `ContentSkeleton`,
`LoginFormSkeleton`, `ProfileSkeleton`, `OrderPlacedSkeleton`,
`AuthFallbackSkeleton`, and the `GiftCardsLoading` route, matching the
project's existing `ProductCardSkeleton` convention.
- New `src/lib/utils/redirect.ts` exports `safeRedirect()`.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-all-skeleton-loaders-on-mobile-and-desktop # Conflicts: # src/app/[country]/[locale]/(storefront)/c/[...permalink]/loading.tsx # src/components/products/filters/FilterBarSkeleton.tsx # src/components/products/filters/ProductFilters.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/account/*pages — each skeleton mirrors the real component's heights, spacing and markup so the skeleton → loaded transition doesn't cause layout shift.src/components/**/, used by inline loading states, Next.jsloading.tsxSuspense fallbacks, and layout-level dispatch inaccount/layout.tsx./accountinto a dedicated/account/loginroute:/accountis now dashboard-only with an auth redirect, and the account button in Header / Footer / MobileMenu routes anonymous users straight to/account/loginvia a newAccountLinkclient component — no more flash of the login form before the dashboard renders for authed users.Test plan
/— featured products/c/<any-category>— banner, subcategories, filter bar, grid/cart(2+ items) — items list + summary sidebar/checkout/<id>— contact, address, shipping, payment, pay-now/order-placed/<id>— success header, items, shipping+payment, addresses/account/login,/account/register,/account/forgot-password,/account/reset-password/account(dashboard),/account/profile,/account/orders,/account/gift-cards,/account/credit-cards,/account/addresses/account/logindirectly/account/*→ redirect to/account/login/account/login?redirect=/checkout/...→ after login returns to checkout/account/loginand/account/registerredirect to/account🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements