Skip to content

fix: broken layout during loading on voucher and pool pages#394

Merged
williamluke4 merged 1 commit intomainfrom
williamluke4/voucher-skeleton-fix
Apr 10, 2026
Merged

fix: broken layout during loading on voucher and pool pages#394
williamluke4 merged 1 commit intomainfrom
williamluke4/voucher-skeleton-fix

Conversation

@williamluke4
Copy link
Copy Markdown
Member

Summary

  • Root cause: EditableImageOverlay returned a bare fragment (<>{children}</>) when canEdit was false, stripping the relative positioning context and className props (like overflow-hidden rounded-2xl shadow-2xl) that hero section children depend on for absolute positioning. Since canEdit is false during loading before auth resolves, this broke the hero layout entirely until auth loaded.
  • Now always renders the structural wrapper div regardless of canEdit, only omitting the edit UI when not editable.
  • Adds skeleton placeholders for async content (type badge, exchange rate, description card, tags, tabs) on both voucher and pool pages so the loading state matches the final layout.

Test plan

  • Load a voucher detail page — hero section should render correctly during loading with skeleton placeholders
  • Load a pool detail page — hero section and tabs should render correctly during loading
  • Verify edit overlays still work when logged in as an owner
  • Check both mobile and desktop viewports

🤖 Generated with Claude Code

EditableImageOverlay returned a bare fragment when canEdit was false,
stripping the relative positioning context and className props that
children depend on for absolute positioning. This caused hero sections
on voucher and pool pages to break during loading before auth resolved.
Also adds skeleton placeholders for async content in both hero sections.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Apr 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sarafu-network Building Building Preview, Comment Apr 10, 2026 8:00am

Request Review

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 10, 2026

Claude finished @williamluke4's task in 1m 26s —— View job


Code Review

  • Gather context and read changed files
  • Review editable-image-overlay.tsx changes
  • Review voucher-hero-section.tsx changes
  • Review voucher-home-tab.tsx changes
  • Review pool-client-page.tsx changes
  • Provide comprehensive feedback

Summary

The root cause diagnosis is accurate: EditableImageOverlay was returning a bare <>{children}</> fragment when canEdit was false, which discarded the className prop (carrying overflow-hidden rounded-2xl shadow-2xl) and the relative positioning context that absolutely-positioned children inside the hero depend on. Since canEdit evaluates to false while auth resolves, the hero layout broke on every page load. The fix is minimal and correct.

The skeleton additions are a clean UX improvement that prevent layout shift and show the correct structural shape during loading.


editable-image-overlay.tsx — Core Fix

The fix (line 47):

- return <>{children}</>;
+ return <div className={cn("relative", className)}>{children}</div>;

This is exactly right. The editable path renders group/editable relative while the non-editable path now renders relative — the difference is intentional since the hover group class is only needed when editing is possible.

One subtle note: the relative class on the non-editable wrapper is now always applied even if the parent already has relative. This is harmless but slightly redundant in some call sites. Not worth changing.


voucher-hero-section.tsx — Skeleton Loading

The skeletons for the type badge and exchange rate are well-sized and styled (bg-white/20 correctly matches the hero's dark overlay). The three-state pattern is clean:

{isLoading ? <Skeleton ... /> : voucher?.voucher_type ? <VoucherTypeTag ... /> : null}

Both VoucherHeroSection and VoucherHomeTab call trpc.voucher.byAddress.useQuery with the same input — React Query deduplicates this, so no double fetching.


voucher-home-tab.tsx — Description Card Skeleton

The skeleton card layout (3 text lines + 3 tag-shaped skeletons) is a reasonable approximation of the description card's actual content.

Minor observation: The skeleton shows during loading even if the voucher has no description — meaning it will appear and then disappear if voucher_description is null. This is a small layout shift after loading completes, but it's unavoidable without knowing the shape of the data ahead of time. Acceptable.


pool-client-page.tsx — Tags, Description, and Tabs Skeletons

Tags + description skeletons: Correctly use isMetadataLoading for inline hero content skeletons.

Tabs skeleton condition: Uses metadata && pool which means the skeleton persists until both resolve. This is correct — PoolTabs requires both props so we can't render it partially. The fallback skeleton shows 6 tab pills + 8 card tiles, which approximates the actual pool tabs UI well.

Minor issue — pool loading state not tracked: The useSwapPool hook result is destructured as const { data: pool } without capturing isLoading. If pool resolves slowly (network latency vs cache), the hero "Pool Address" section renders with a live pool_address (good) but the PoolButtons won't appear until pool resolves:

<div className="pt-2 ">{pool && <PoolButtons pool={pool} />}</div>

This is existing behavior and not a regression, but worth noting as a follow-up improvement opportunity.

Trailing space in className="pt-2 " at line 183 — pre-existing nit, not introduced by this PR.


Testing Coverage

No tests are included, but these are UI loading state changes — visual regression tests are the appropriate vehicle and are typically handled via preview deploys (Vercel preview is building). The manual test plan in the PR description covers the right scenarios.


Verdict

Approve. The fix is correct and minimal, skeletons are appropriately styled for the dark hero overlay context, and the three-state isLoading ? skeleton : data ? content : null pattern is consistently applied across all four files. No security, performance, or correctness issues found.

@williamluke4 williamluke4 merged commit ad359a3 into main Apr 10, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant