Improved gift subscription Portal screens across selection, preview, and mobile#27770
Improved gift subscription Portal screens across selection, preview, and mobile#27770minimaluminium merged 10 commits intomainfrom
Conversation
- also balanced .gh-portal-gift-checkout-left bottom padding to match the top
Changelog for v2.68.32 -> 2.68.33:
|
ℹ️ 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 as they are similar to previous changes (1)
WalkthroughThis PR adds a new pricing helper export Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Microsoft Presidio Analyzer (2.2.362)apps/portal/src/components/pages/gift-page.jsMicrosoft Presidio Analyzer failed to scan this file 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/portal/src/components/pages/gift-page.js (1)
645-672:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
window.matchMediais not available in the JSDOM test env — breaks unit tests.Static analysis reports
TypeError: window.matchMedia is not a functionoriginating at line 654, surfaced viatest/portal-links.test.js("opens gift page when giftSubscriptions labs flag is enabled"). JSDOM does not implementmatchMediaby default, so any test that mountsGiftPagewill throw inside theuseLayoutEffect.Guard the call so the layout effect degrades gracefully when
matchMediais unavailable.🛡️ Proposed fix
- if (window.matchMedia('(max-width: 880px)').matches) { + if (typeof window.matchMedia === 'function' && window.matchMedia('(max-width: 880px)').matches) { inner.style.marginTop = ''; centeringDoneRef.current = true; return; }Alternatively, mock
window.matchMediain the test setup file. The runtime guard is preferable since it also protects any non-browser embedding.🤖 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 645 - 672, The useLayoutEffect that centers the inner element calls window.matchMedia and breaks in JSDOM; update the guard around that call in the effect (the block using useLayoutEffect, centeringDoneRef, innerRef, leftRef) to first check that window.matchMedia exists (e.g. typeof window.matchMedia === 'function' or window.matchMedia != null) before calling .matches, and if it's unavailable treat it as the mobile case (clear inner.style.marginTop / skip centering) so the effect degrades gracefully in tests and non-browser environments.
🧹 Nitpick comments (1)
apps/portal/src/components/pages/gift-page.js (1)
613-623: ⚡ Quick winNaming nit: helper name vs. responsibility.
formatGiftValueis now a generic price-object formatter ({amount, currency} → "$X") used wherever a price needs to render — not specifically tied to agift. Consider renaming to something likeformatPriceAmount(and re-exporting) so callers ingift-redemption-page.js/magic-link-page.js/gift-success-page.jsaren't misled into passing agiftobject (which appears to already have caused the shape mismatch flagged ingift-redemption-page.jsline 221).🤖 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 613 - 623, Rename the helper formatGiftValue to a more accurate name like formatPriceAmount and update its export so it reflects that it formats a generic price object ({amount, currency} → "$X"); change the function declaration from formatGiftValue to formatPriceAmount, update all internal callers (e.g., getTierPriceLabel uses formatPriceAmount), and export it under the new name while optionally re-exporting the old name (export { formatPriceAmount as formatGiftValue }) to maintain backwards compatibility for callers in gift-redemption-page.js, magic-link-page.js and gift-success-page.js; ensure imports in those modules are updated or keep the alias export so existing imports continue to work.
🤖 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.
Outside diff comments:
In `@apps/portal/src/components/pages/gift-page.js`:
- Around line 645-672: The useLayoutEffect that centers the inner element calls
window.matchMedia and breaks in JSDOM; update the guard around that call in the
effect (the block using useLayoutEffect, centeringDoneRef, innerRef, leftRef) to
first check that window.matchMedia exists (e.g. typeof window.matchMedia ===
'function' or window.matchMedia != null) before calling .matches, and if it's
unavailable treat it as the mobile case (clear inner.style.marginTop / skip
centering) so the effect degrades gracefully in tests and non-browser
environments.
---
Nitpick comments:
In `@apps/portal/src/components/pages/gift-page.js`:
- Around line 613-623: Rename the helper formatGiftValue to a more accurate name
like formatPriceAmount and update its export so it reflects that it formats a
generic price object ({amount, currency} → "$X"); change the function
declaration from formatGiftValue to formatPriceAmount, update all internal
callers (e.g., getTierPriceLabel uses formatPriceAmount), and export it under
the new name while optionally re-exporting the old name (export {
formatPriceAmount as formatGiftValue }) to maintain backwards compatibility for
callers in gift-redemption-page.js, magic-link-page.js and gift-success-page.js;
ensure imports in those modules are updated or keep the alias export so existing
imports continue to work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6fd5a581-614d-4328-af7b-61060a8b195b
📒 Files selected for processing (6)
apps/portal/package.jsonapps/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.jsapps/portal/src/components/popup-modal.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/portal/src/components/pages/gift-page.js`:
- Around line 613-618: The exported helper formatGiftValue currently
destructures its parameter directly, so calling formatGiftValue(null) throws;
change it to first coalesce the incoming argument to an empty object before
destructuring (e.g., accept a single param and do const { amount, currency } =
param ?? {}), then keep the existing null/undefined checks and return logic;
update the function signature referencing formatGiftValue and ensure all
internal references use the coalesced values so both undefined and null inputs
are safely handled.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 934fcc72-2706-459b-906b-bee00975a904
📒 Files selected for processing (1)
apps/portal/src/components/pages/gift-page.js
1 participant
ref https://linear.app/ghost/issue/BER-3600/design-iteration