Refreshed Portal gift tier picker and card design#27705
Conversation
Tier rows on the gift selection screen now expand inline to reveal each tier's benefits, and the CTA sticks to the bottom with a fade gradient. Refreshes the gift card preview on the right with a brand-coloured panel, ID-badge style card, and lighter close icon, details toggle and benefit text against the brand background.
…k pages The card on the right side of the gift confirmation, redemption, and magic-link-in-gift-mode pages now uses the brand-coloured panel and ID-badge card from the selection page, drops the ribbon and bow ornament, and adds a "membership" suffix to the tier name.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThe portal's gift UX was reworked across four pages. GiftPage received a major redesign: new CSS, an accordion-based tier selector, left/right column layout, centering via useLayoutEffect, tiltable right-card visuals, and decorative orb/ribbon/bow assets. GiftRedemptionPage, GiftSuccessPage, and MagicLinkPage had their right-side panels reorganized into a card-stack wrapper, simplified inner card structure, updated tier labeling (appending "membership"), and an optional collapsible benefits/details section. Package version for the portal was bumped from 2.68.28 to 2.68.29. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Changelog for v2.68.28 -> 2.68.29:
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
apps/portal/src/components/pages/magic-link-page.js (2)
344-361: ⚡ Quick winCursor-driven card tilt is missing on this page.
gift-page.js,gift-redemption-page.js, andgift-success-page.jsall spreadcardTiltPropson.gh-portal-gift-checkout-rightand passcardRefto.gh-portal-gift-checkout-card. Here neither is wired (nouseCardTiltimport; class component can't use hooks directly), so the card never tilts to cursor movement — only thedata-revealingreveal rotation fires. To restore visual parity, extract a small functional wrapper (e.g.<TiltCard>) that usesuseCardTiltinternally and renders its children, then use it here in place of the bare.gh-portal-gift-checkout-right+ card-frame markup.🤖 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 `@apps/portal/src/components/pages/magic-link-page.js` around lines 344 - 361, The right-side gift card markup in magic-link-page.js is missing the cursor-driven tilt logic: create a small functional wrapper component (e.g. TiltCard) that imports and calls useCardTilt(), spreads the returned cardTiltProps onto the outer wrapper that currently has className 'gh-portal-gift-checkout-right', and forwards the cardRef to the inner element with className 'gh-portal-gift-checkout-card' (or accept children and attach ref there); replace the current plain markup with this <TiltCard> wrapper so the card element receives the hook's ref and the wrapper receives the tilt props to restore cursor tilt behavior.
358-358: 💤 Low valueGuard against a missing
gift.tier.
gift?.tier?.namewill render"undefined membership"ifgift.tieris absent, since the optional chain falls through into the template literal. TheisGiftModegate at line 414 only checkspageData?.gift, notgift.tier. Either tighten the gate to requirepageData?.gift?.tier, or coerce the fallback here (e.g.gift.tier?.name ? \${gift.tier.name} membership` : ''`).🤖 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 `@apps/portal/src/components/pages/magic-link-page.js` at line 358, The template currently renders `${gift.tier?.name} membership` which will show "undefined membership" if gift.tier is missing; either tighten the existing isGiftMode gate to require pageData?.gift?.tier (so rendering only occurs when a tier exists) or change the expression in the gh-portal-gift-checkout-card-tier div to coerce a safe fallback (e.g., render `${gift.tier?.name ? `${gift.tier.name} membership` : ''}`) so that missing gift.tier does not produce "undefined membership"; update either the isGiftMode check or the rendering expression around gift.tier accordingly.apps/portal/src/components/pages/gift-page.js (3)
572-594: 💤 Low valuePass an explicit dependency array to
useLayoutEffect.Without a dependency array, this effect runs after every render (state updates, tier switches, hover-driven re-renders, etc.). The
centeringDoneRef.currentshort-circuit keeps it cheap, but the intent is clearly "run once on mount." Pass[]so this matches the comment "On first paint" and avoids any chance of subtle re-execution if the early-out conditions ever change.♻️ Proposed fix
useLayoutEffect(() => { if (centeringDoneRef.current) { return; } ... centeringDoneRef.current = true; - }); + }, []);🤖 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 `@apps/portal/src/components/pages/gift-page.js` around lines 572 - 594, The useLayoutEffect that centers the inner element runs without a dependency array so it executes on every render; update the hook call in the GiftPage component (the useLayoutEffect using centeringDoneRef, innerRef, leftRef) to include an explicit empty dependency array ([]) so it only runs once on mount/"first paint"; ensure the existing early returns/refs (centeringDoneRef.current, innerRef, leftRef) remain unchanged and, if your linter complains about exhaustive-deps, add a concise eslint-disable-next-line comment scoped to this line rather than changing the effect logic.
668-712: ⚖️ Poor tradeoffRadiogroup is missing arrow-key navigation.
The
role='radiogroup'/role='radio'pattern requires arrow keys to move focus between radios per the WAI-ARIA APG; right now only Tab works, and each radio is tab-stoppable, which is the inverse of the expected single-tabstop-with-roving-focus behavior. Consider either dropping the radio roles (and styling the buttons as a regular list) or wiring uponKeyDownto handle ArrowUp/Down/Left/Right andtabIndexto set the roving tabstop on the selected option.🤖 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 `@apps/portal/src/components/pages/gift-page.js` around lines 668 - 712, The radiogroup currently uses role='radiogroup' and role='radio' but lacks roving keyboard behavior; update the buttons rendered in products.map (the radio buttons that call setSelectedProductId and use activeProduct) to implement roving focus: give each button a tabIndex of (isSelected ? 0 : -1), add an onKeyDown handler that listens for ArrowUp/ArrowLeft and ArrowDown/ArrowRight to compute the previous/next product index (using the products array and product.id), call setSelectedProductId with the new id and move focus to the corresponding button (use refs or querySelector by data-test-tier/product.id), and ensure Enter/Space also activate selection; alternatively, if you don't want ARIA radios, remove role='radiogroup' and role='radio' and keep them as regular buttons—choose one approach and apply it to the elements inside the products.map loop.
731-749: 💤 Low valueInconsistent card-stack structure across the four gift pages.
The other three pages (
gift-redemption-page.js,gift-success-page.js,magic-link-page.js) wrap the card in.gh-portal-gift-checkout-card-frameand setdata-revealingon.gh-portal-gift-checkout-card-stack, since they all expose a "Gift details" toggle. Here the card-frame wrapper is omitted, which is technically fine because this page has no details toggle — but the divergence is easy to miss when refactoring. If it stays divergent, consider adding a brief comment so the structure isn't accidentally "fixed" later; otherwise add the wrapper for structural parity.🤖 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 `@apps/portal/src/components/pages/gift-page.js` around lines 731 - 749, The card stack markup in gift-page.js diverges from the other pages by omitting the .gh-portal-gift-checkout-card-frame wrapper and the data-revealing attribute on .gh-portal-gift-checkout-card-stack; either restore structural parity by wrapping the existing .gh-portal-gift-checkout-card element in a div with class "gh-portal-gift-checkout-card-frame" and add data-revealing (e.g. data-revealing="true" or matching the other pages) to the .gh-portal-gift-checkout-card-stack element, or if you intentionally want this page to differ, add a brief comment next to the .gh-portal-gift-checkout-card-stack or card markup explaining why the frame is omitted to prevent accidental refactors.
🤖 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.
Nitpick comments:
In `@apps/portal/src/components/pages/gift-page.js`:
- Around line 572-594: The useLayoutEffect that centers the inner element runs
without a dependency array so it executes on every render; update the hook call
in the GiftPage component (the useLayoutEffect using centeringDoneRef, innerRef,
leftRef) to include an explicit empty dependency array ([]) so it only runs once
on mount/"first paint"; ensure the existing early returns/refs
(centeringDoneRef.current, innerRef, leftRef) remain unchanged and, if your
linter complains about exhaustive-deps, add a concise eslint-disable-next-line
comment scoped to this line rather than changing the effect logic.
- Around line 668-712: The radiogroup currently uses role='radiogroup' and
role='radio' but lacks roving keyboard behavior; update the buttons rendered in
products.map (the radio buttons that call setSelectedProductId and use
activeProduct) to implement roving focus: give each button a tabIndex of
(isSelected ? 0 : -1), add an onKeyDown handler that listens for
ArrowUp/ArrowLeft and ArrowDown/ArrowRight to compute the previous/next product
index (using the products array and product.id), call setSelectedProductId with
the new id and move focus to the corresponding button (use refs or querySelector
by data-test-tier/product.id), and ensure Enter/Space also activate selection;
alternatively, if you don't want ARIA radios, remove role='radiogroup' and
role='radio' and keep them as regular buttons—choose one approach and apply it
to the elements inside the products.map loop.
- Around line 731-749: The card stack markup in gift-page.js diverges from the
other pages by omitting the .gh-portal-gift-checkout-card-frame wrapper and the
data-revealing attribute on .gh-portal-gift-checkout-card-stack; either restore
structural parity by wrapping the existing .gh-portal-gift-checkout-card element
in a div with class "gh-portal-gift-checkout-card-frame" and add data-revealing
(e.g. data-revealing="true" or matching the other pages) to the
.gh-portal-gift-checkout-card-stack element, or if you intentionally want this
page to differ, add a brief comment next to the
.gh-portal-gift-checkout-card-stack or card markup explaining why the frame is
omitted to prevent accidental refactors.
In `@apps/portal/src/components/pages/magic-link-page.js`:
- Around line 344-361: The right-side gift card markup in magic-link-page.js is
missing the cursor-driven tilt logic: create a small functional wrapper
component (e.g. TiltCard) that imports and calls useCardTilt(), spreads the
returned cardTiltProps onto the outer wrapper that currently has className
'gh-portal-gift-checkout-right', and forwards the cardRef to the inner element
with className 'gh-portal-gift-checkout-card' (or accept children and attach ref
there); replace the current plain markup with this <TiltCard> wrapper so the
card element receives the hook's ref and the wrapper receives the tilt props to
restore cursor tilt behavior.
- Line 358: The template currently renders `${gift.tier?.name} membership` which
will show "undefined membership" if gift.tier is missing; either tighten the
existing isGiftMode gate to require pageData?.gift?.tier (so rendering only
occurs when a tier exists) or change the expression in the
gh-portal-gift-checkout-card-tier div to coerce a safe fallback (e.g., render
`${gift.tier?.name ? `${gift.tier.name} membership` : ''}`) so that missing
gift.tier does not produce "undefined membership"; update either the isGiftMode
check or the rendering expression around gift.tier accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 763d54e4-3632-4f75-89ae-6bf7b9cf9442
⛔ Files ignored due to path filters (1)
apps/portal/src/images/gift-card-orb.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
apps/portal/src/components/pages/gift-page.jsapps/portal/src/components/pages/gift-redemption-page.jsapps/portal/src/components/pages/gift-success-page.jsapps/portal/src/components/pages/magic-link-page.js
1 participant
ref https://linear.app/ghost/issue/BER-3600/design-iteration