Feature/subscriptions - #153
Conversation
📝 WalkthroughWalkthroughThis PR introduces a subscription feature to the app by adding a new subscriptions screen with bilingual monthly/yearly plan options, updating the settings menu with a subscription link, and removing an unused manual data fetch from the home screen. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 5
🧹 Nitpick comments (1)
app/(protected)/subscriptions/index.tsx (1)
15-20:SCREEN_WIDTHcaptured at module load — not responsive to rotation/resize.
Dimensions.get("window")is evaluated once at import time, soCARD_WIDTHandSIDE_PADDINGwon't adapt to orientation changes or foldables/tablets resizing. For a subscription flow this is usually acceptable, but consideruseWindowDimensions()if the app supports rotation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(protected)/subscriptions/index.tsx around lines 15 - 20, SCREEN_WIDTH is captured at module load via Dimensions.get("window"), so CARD_WIDTH and SIDE_PADDING won't update on rotation; replace the static import-time values by using useWindowDimensions() inside the component that renders the cards (or a custom hook) and compute CARD_WIDTH, CARD_GAP and SIDE_PADDING there (reference: variables SCREEN_WIDTH, CARD_WIDTH, SIDE_PADDING and the call to Dimensions.get("window")); ensure any consumers of those constants read the computed values from props/state instead of the module-level constants so layout updates on resize/orientation change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(protected)/subscriptions/index.tsx:
- Around line 137-141: The save-pillar text is hardcoded to "–17%" under the
yearly branch; update the render logic in the component where value === "yearly"
(the block using styles.savePill, styles.savePillText, and theme.primary) to
compute the actual percent saved from the active plan's monthlyPrice and
yearlyPrice (e.g., percent = 100 * (1 - yearlyPrice / (monthlyPrice * 12))) and
display that formatted percent per plan, or replace the text with a generic
label like "Save" if you prefer not to compute it; ensure you reference the plan
object used to render the card (the same prop/state that contains
monthlyPrice/yearlyPrice) so the displayed value matches the selected plan.
- Around line 159-170: The price display currently always appends "/mo" because
the template uses a static suffix; update the render logic that uses billing,
price, whole and cents (the computed price, whole and cents variables in this
file) to choose the suffix based on the billing mode: when billing === "monthly"
show "/mo", when billing === "yearly" show either "/yr" or show a two-line label
like "/mo billed annually" or the annual total (price * 12) depending on the
desired UX; adjust the code that constructs the displayed string (where whole
and cents are used) and the secondary label (lines around where dividerColor,
btnBg, btnTextColor are set) so the suffix is conditional and add an explicit
"billed annually" note when billing === "yearly".
- Around line 336-338: The PlanCard render passes a no-op onSubscribe
(onSubscribe={() => {}}) so the Subscribe CTA does nothing; replace this with a
real handler that either navigates to the purchase/subscription flow or shows a
visible placeholder (e.g., a toast/alert or console.warn) so the button has
feedback. Update the renderItem to pass a function that calls your subscription
helper (e.g., startSubscriptionFlow or navigateToCheckout) or at minimum calls
showToast("Subscription coming soon") / console.warn with plan id; reference the
PlanCard component and the renderItem callback to locate where to wire the
handler and add a TODO comment if the real flow will be implemented later.
- Around line 37-90: PLANS currently contains hardcoded user-facing strings
(tier, name, nameItalic, tagline, badge, features, CTA text, toggle labels and
percent labels) and must be localized: extract all visible strings into
locales/*.json under a subscriptions namespace (e.g.,
subscriptions.plans.basic.name, subscriptions.plans.basic.tagline,
subscriptions.features.*, subscriptions.cta.*) and replace inline literals in
the PLANS array with translation keys consumed via createT("subscriptions") and
t(...) at render time; ensure plan identifiers (id: "basic", "premium", "gold")
remain stable and only the displayed fields use t(...) so components like the
plan renderer, the toggle labels and badge display use t("plans.{id}.name"),
t("plans.{id}.tagline"), t("plans.{id}.features.[i]"), t("cta.subscribe") etc.,
and add missing keys to locales/en.json (and other locale files) to avoid
fallback gaps.
- Around line 314-339: The FlatList with initialScrollIndex can still fail to
scroll on mount; add an onScrollToIndexFailed handler to the FlatList (alongside
getItemLayout and initialScrollIndex) that waits briefly and then calls
flatListRef.current?.scrollToIndex({ index: info.index, animated: true }) to
retry (use the info.index from the handler), e.g. create a small timeout Promise
or setTimeout retry inside onScrollToIndexFailed to re-attempt scrolling for the
FlatList referenced by flatListRef.
---
Nitpick comments:
In `@app/`(protected)/subscriptions/index.tsx:
- Around line 15-20: SCREEN_WIDTH is captured at module load via
Dimensions.get("window"), so CARD_WIDTH and SIDE_PADDING won't update on
rotation; replace the static import-time values by using useWindowDimensions()
inside the component that renders the cards (or a custom hook) and compute
CARD_WIDTH, CARD_GAP and SIDE_PADDING there (reference: variables SCREEN_WIDTH,
CARD_WIDTH, SIDE_PADDING and the call to Dimensions.get("window")); ensure any
consumers of those constants read the computed values from props/state instead
of the module-level constants so layout updates on resize/orientation change.
🪄 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: e5d76fde-22da-4ae6-94a2-118f15f64a87
📒 Files selected for processing (4)
app/(protected)/(tabs)/index.tsxapp/(protected)/(tabs)/settings.tsxapp/(protected)/subscriptions/index.tsxlocales/en.json
💤 Files with no reviewable changes (1)
- app/(protected)/(tabs)/index.tsx
| const PLANS: Plan[] = [ | ||
| { | ||
| id: "basic", | ||
| tier: "01 Basic", | ||
| name: "Basic", | ||
| nameItalic: "plan", | ||
| tagline: "One request at a time", | ||
| monthlyPrice: 5.99, | ||
| yearlyPrice: 4.99, | ||
| features: [ | ||
| "Access to all features", | ||
| "Bi-weekly sync calls", | ||
| "Available 3 days each week", | ||
| "Easy-to-manage dashboard", | ||
| "Immediate start", | ||
| ], | ||
| }, | ||
| { | ||
| id: "premium", | ||
| tier: "02 Premium", | ||
| name: "Premium", | ||
| nameItalic: "plan", | ||
| tagline: "Double your output 2×", | ||
| monthlyPrice: 12.99, | ||
| yearlyPrice: 10.49, | ||
| badge: "BEST VALUE", | ||
| highlight: true, | ||
| features: [ | ||
| "All from Basic plan", | ||
| "Direct comms + weekly sync", | ||
| "Available 5 days each week", | ||
| "Delivery in avg. 48 hours", | ||
| "No-code add-ons for free", | ||
| "Easy-to-manage dashboard", | ||
| "Immediate start", | ||
| ], | ||
| }, | ||
| { | ||
| id: "gold", | ||
| tier: "03 Gold", | ||
| name: "Gold", | ||
| nameItalic: "plan", | ||
| tagline: "Fitting your individual needs", | ||
| monthlyPrice: 29.99, | ||
| yearlyPrice: 24.99, | ||
| features: [ | ||
| "Custom scope", | ||
| "Dedicated team", | ||
| "Fixed deadlines", | ||
| "Strategic consulting bi-weekly", | ||
| "Payment plan on milestones", | ||
| ], | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
Hardcoded English strings bypass i18n.
All user-facing text in this screen (headers, toggle labels, plan names/taglines/features, CTA "Subscribe", "Cancel anytime...", "BEST VALUE", "–17%") is hardcoded. The rest of the app uses createT(...) + t(...) (e.g., settings.tsx), and a translation key was added in locales/en.json only for the menu entry. This screen will not translate for other locales.
Move these strings to locales/*.json (e.g., under subscriptions.*) and consume via createT("subscriptions"). Plan data (features, taglines) should either live in locale files or be keyed for translation.
Also applies to: 118-139, 196-219, 300-305, 344-346
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(protected)/subscriptions/index.tsx around lines 37 - 90, PLANS
currently contains hardcoded user-facing strings (tier, name, nameItalic,
tagline, badge, features, CTA text, toggle labels and percent labels) and must
be localized: extract all visible strings into locales/*.json under a
subscriptions namespace (e.g., subscriptions.plans.basic.name,
subscriptions.plans.basic.tagline, subscriptions.features.*,
subscriptions.cta.*) and replace inline literals in the PLANS array with
translation keys consumed via createT("subscriptions") and t(...) at render
time; ensure plan identifiers (id: "basic", "premium", "gold") remain stable and
only the displayed fields use t(...) so components like the plan renderer, the
toggle labels and badge display use t("plans.{id}.name"),
t("plans.{id}.tagline"), t("plans.{id}.features.[i]"), t("cta.subscribe") etc.,
and add missing keys to locales/en.json (and other locale files) to avoid
fallback gaps.
| {value === "yearly" && ( | ||
| <View style={[styles.savePill, { backgroundColor: theme.primary }]}> | ||
| <Text style={styles.savePillText}>–17%</Text> | ||
| </View> | ||
| )} |
There was a problem hiding this comment.
Hardcoded "–17%" save pill doesn't match actual per-plan savings.
Savings vary per plan (Basic: ~16.7%, Premium: ~19.2%, Gold: ~16.7%). A single hardcoded value is misleading. Either compute it from the active plan's monthlyPrice/yearlyPrice, or remove it in favor of a generic "Save" label.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(protected)/subscriptions/index.tsx around lines 137 - 141, The
save-pillar text is hardcoded to "–17%" under the yearly branch; update the
render logic in the component where value === "yearly" (the block using
styles.savePill, styles.savePillText, and theme.primary) to compute the actual
percent saved from the active plan's monthlyPrice and yearlyPrice (e.g., percent
= 100 * (1 - yearlyPrice / (monthlyPrice * 12))) and display that formatted
percent per plan, or replace the text with a generic label like "Save" if you
prefer not to compute it; ensure you reference the plan object used to render
the card (the same prop/state that contains monthlyPrice/yearlyPrice) so the
displayed value matches the selected plan.
| const price = billing === "monthly" ? plan.monthlyPrice : plan.yearlyPrice; | ||
| const isHighlight = plan.highlight; | ||
|
|
||
| const cardBg = isHighlight ? theme.primary : theme.background; | ||
| const textColor = isHighlight ? "#fff" : theme.text; | ||
| const mutedColor = isHighlight ? "rgba(255,255,255,0.6)" : theme.accent; | ||
| const dividerColor = isHighlight ? "rgba(255,255,255,0.15)" : theme.accent + "22"; | ||
| const btnBg = isHighlight ? "#fff" : theme.primary; | ||
| const btnTextColor = isHighlight ? theme.primary : "#fff"; | ||
|
|
||
| const whole = Math.floor(price); | ||
| const cents = (price % 1).toFixed(2).slice(1); |
There was a problem hiding this comment.
Price suffix always shows /mo even when yearly billing is selected.
When billing === "yearly", plan.yearlyPrice is rendered with a static /mo label. If yearlyPrice represents the monthly-equivalent of an annual plan, the UX is still confusing — users selecting "Yearly" expect to see either the annual total or an explicit "/mo, billed yearly" clarification. Consider making the suffix conditional on billing, or append a secondary line like "billed annually".
Proposed adjustment
- <Text style={[styles.pricePer, { color: mutedColor }]}>/mo</Text>
+ <Text style={[styles.pricePer, { color: mutedColor }]}>
+ {billing === "yearly" ? "/mo, billed yearly" : "/mo"}
+ </Text>Also applies to: 222-233
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(protected)/subscriptions/index.tsx around lines 159 - 170, The price
display currently always appends "/mo" because the template uses a static
suffix; update the render logic that uses billing, price, whole and cents (the
computed price, whole and cents variables in this file) to choose the suffix
based on the billing mode: when billing === "monthly" show "/mo", when billing
=== "yearly" show either "/yr" or show a two-line label like "/mo billed
annually" or the annual total (price * 12) depending on the desired UX; adjust
the code that constructs the displayed string (where whole and cents are used)
and the secondary label (lines around where dividerColor, btnBg, btnTextColor
are set) so the suffix is conditional and add an explicit "billed annually" note
when billing === "yearly".
| <FlatList | ||
| ref={flatListRef} | ||
| data={PLANS} | ||
| keyExtractor={(p) => p.id} | ||
| horizontal | ||
| showsHorizontalScrollIndicator={false} | ||
| snapToInterval={CARD_WIDTH + CARD_GAP} | ||
| snapToAlignment="start" | ||
| decelerationRate="fast" | ||
| contentContainerStyle={{ | ||
| paddingHorizontal: SIDE_PADDING, | ||
| paddingVertical: 20, | ||
| gap: CARD_GAP, | ||
| }} | ||
| initialScrollIndex={1} | ||
| getItemLayout={(_, index) => ({ | ||
| length: CARD_WIDTH + CARD_GAP, | ||
| offset: (CARD_WIDTH + CARD_GAP) * index, | ||
| index, | ||
| })} | ||
| onScroll={handleScroll} | ||
| scrollEventThrottle={16} | ||
| renderItem={({ item }) => ( | ||
| <PlanCard plan={item} billing={billing} onSubscribe={() => {}} /> | ||
| )} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
react-native FlatList initialScrollIndex onScrollToIndexFailed best practice
💡 Result:
Best practice for using initialScrollIndex and handling onScrollToIndexFailed in React Native FlatList: 1. Implement getItemLayout (highest priority for reliability and performance): Provide exact dimensions for each item. Required for initialScrollIndex per official docs. Works only if all items have predictable, fixed heights/widths. Example for vertical list with fixed item height (e.g., 80px): const ITEM_HEIGHT = 80; const getItemLayout = (data, index) => ({ length: ITEM_HEIGHT, offset: ITEM_HEIGHT * index, index, }); For horizontal lists, use width instead of height. Include margins/padding in calculations. 2. Always add onScrollToIndexFailed handler: Catches failures when target index isn't rendered yet (common without getItemLayout or for distant indices). Official docs note scrollToIndex cannot reach outside render window without getItemLayout. Recommended implementation (most common, from multiple high-score SO answers): const flatListRef = useRef(null); <FlatList ref={flatListRef} initialScrollIndex={targetIndex} getItemLayout={getItemLayout} // if possible onScrollToIndexFailed={(info) => { const wait = new Promise(resolve => setTimeout(resolve, 500)); wait.then( => { flatListRef.current?.scrollToIndex({index: info.index, animated: true}); }); }} // other props... /> Alternative using averageItemLength (for uniform items): onScrollToIndexFailed={(info) => { const offset = info.averageItemLength * info.index; flatListRef.current?.scrollToOffset({offset}); // Then retry scrollToIndex after delay }} 3. Increase initialNumToRender: Default is 10. Set higher (e.g., 60) if target index is far and getItemLayout unavailable. 4. Timing considerations: - Use onContentSizeChange or onLayout for programmatic scrollToIndex after render. - Wrap calls in requestAnimationFrame or short setTimeout. - For initial positioning, prefer initialScrollIndex over post-mount scrollToIndex. 5. Known issues/caveats (as of 2026): - initialScrollIndex may skip rendering earlier items if list fits viewport (ongoing GitHub issues #56237, #44544). - Workaround: disableVirtualization={true} or scrollToIndex after mount. Official docs confirm: initialScrollIndex requires getItemLayout; scrollToIndex warns about needing it or onScrollToIndexFailed. This combination (getItemLayout + onScrollToIndexFailed timeout retry) is the consensus best practice across Stack Overflow (82+ score answers), GitHub issues, and tutorials.
Citations:
- 1: https://stackoverflow.com/questions/53059609/flat-list-scrolltoindex-should-be-used-in-conjunction-with-getitemlayout-or-on
- 2: https://stackoverflow.com/questions/53059609/flat-list-scrolltoindex-should-be-used-in-conjunction-with-getitemlayout-or-on/60320726
- 3: https://stackoverflow.com/questions/44540621/initialscrollindex-not-working-for-flatlist-react-native
- 4: https://reactnative.dev/docs/flatlist
- 5: https://facebook.github.io/react-native/docs/flatlist
- 6: https://stackoverflow.com/questions/64677087/react-native-getting-an-error-when-scrolltoindex-using-flatlist-ref
- 7: https://stackoverflow.com/questions/67338792/how-do-i-use-the-getitemlayout-prop-for-flatlist-in-react-native
- 8: https://runebook.dev/en/articles/react_native/flatlist/getitemlayout
- 9: https://stackoverflow.com/questions/48679272/flatlist-getitemlayout-usecase
- 10: https://stackoverflow.com/questions/56416228/getitemlayout-in-react-native-flatlist
- 11: https://reactnative.dev/docs/flatlist.md
- 12: http://facebook.github.io/react-native/docs/flatlist.html
- 13: Using initialScrollIndex with Flatlist will cause missing items with react native react/react-native#44544
- 14: Fix FlatList initialScrollIndex behavior when short list fits viewport react/react-native#56262
- 15: FlatList with initialScrollIndex skips earlier items when list fits viewport react/react-native#56237
🏁 Script executed:
fd -t f 'index.tsx' | grep subscriptionsRepository: MindsofD23b/Frontend-Elysio
Length of output: 110
🏁 Script executed:
if [ -f "app/(protected)/subscriptions/index.tsx" ]; then
wc -l "app/(protected)/subscriptions/index.tsx"
else
find . -path "*subscriptions*index.tsx" -type f
fiRepository: MindsofD23b/Frontend-Elysio
Length of output: 114
🏁 Script executed:
sed -n '314,339p' "app/(protected)/subscriptions/index.tsx"Repository: MindsofD23b/Frontend-Elysio
Length of output: 1095
Add onScrollToIndexFailed handler to prevent scroll failures with initialScrollIndex.
While getItemLayout significantly reduces the risk of scroll failures, the official React Native documentation recommends pairing it with onScrollToIndexFailed as a defensive handler. This is especially important when using initialScrollIndex with a horizontal FlatList, as layout measurements can occasionally be unavailable at mount time.
<details>
<summary>Suggested implementation:</summary>
```jsx
<FlatList
ref={flatListRef}
// ... existing props
initialScrollIndex={1}
getItemLayout={(_, index) => ({
length: CARD_WIDTH + CARD_GAP,
offset: (CARD_WIDTH + CARD_GAP) * index,
index,
})}
onScrollToIndexFailed={(info) => {
const wait = new Promise(resolve => setTimeout(resolve, 500));
wait.then(() => {
flatListRef.current?.scrollToIndex({index: info.index, animated: true});
});
}}
// ... rest of props
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(protected)/subscriptions/index.tsx around lines 314 - 339, The FlatList
with initialScrollIndex can still fail to scroll on mount; add an
onScrollToIndexFailed handler to the FlatList (alongside getItemLayout and
initialScrollIndex) that waits briefly and then calls
flatListRef.current?.scrollToIndex({ index: info.index, animated: true }) to
retry (use the info.index from the handler), e.g. create a small timeout Promise
or setTimeout retry inside onScrollToIndexFailed to re-attempt scrolling for the
FlatList referenced by flatListRef.
| renderItem={({ item }) => ( | ||
| <PlanCard plan={item} billing={billing} onSubscribe={() => {}} /> | ||
| )} |
There was a problem hiding this comment.
Subscribe CTA is a no-op.
onSubscribe={() => {}} — tapping "Subscribe" does nothing. At minimum add a TODO or a placeholder navigation/toast so this isn't silently shipped as dead UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(protected)/subscriptions/index.tsx around lines 336 - 338, The PlanCard
render passes a no-op onSubscribe (onSubscribe={() => {}}) so the Subscribe CTA
does nothing; replace this with a real handler that either navigates to the
purchase/subscription flow or shows a visible placeholder (e.g., a toast/alert
or console.warn) so the button has feedback. Update the renderItem to pass a
function that calls your subscription helper (e.g., startSubscriptionFlow or
navigateToCheckout) or at minimum calls showToast("Subscription coming soon") /
console.warn with plan id; reference the PlanCard component and the renderItem
callback to locate where to wire the handler and add a TODO comment if the real
flow will be implemented later.
2 participants
Summary by CodeRabbit