feat(mobile-expo): CTARenderer section component#322
Conversation
Replace the CTA stub with a real renderer displaying heading, body text, and a tappable button with primary/secondary variant styling. Resolves #307 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ 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 (1)
WalkthroughImplements the CTARenderer component as a full React Native implementation to replace the temporary stub. The component renders call-to-action sections with optional heading and body text, a pressable button supporting primary and secondary variants, and URL navigation via Linking.openURL. Integration points are updated to use the new implementation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mobile/expo/src/components/sections/CTARenderer.tsx`:
- Around line 13-16: The CTA currently calls Linking.openURL inside handlePress
and discards Promise rejections and the UI still renders as an active link when
buttonLink is null; update the CTARenderer so the clickable control derives a
disabled state from buttonLink (e.g., treat null/empty/invalid URL as disabled)
and reflect that in the rendered button/Touchable (so it is not
accessible/pressable when disabled), and modify handlePress to await or call
Linking.openURL(buttonLink).catch(...) to handle and log/show failures instead
of discarding the rejection; reference the handlePress function and the
conditional around buttonLink in CTARenderer to implement these changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8047f923-c65b-42d1-9723-e406179442ae
📒 Files selected for processing (5)
mobile/expo/src/components/sections/CTARenderer.test.tsxmobile/expo/src/components/sections/CTARenderer.tsxmobile/expo/src/components/sections/CTAStub.tsxmobile/expo/src/components/sections/SectionDispatcher.tsxmobile/expo/src/components/sections/index.ts
💤 Files with no reviewable changes (1)
- mobile/expo/src/components/sections/CTAStub.tsx
| const handlePress = () => { | ||
| if (buttonLink) { | ||
| void Linking.openURL(buttonLink) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file
find . -type f -name "CTARenderer.tsx" 2>/dev/null | head -20Repository: JesusFilm/forge
Length of output: 112
🏁 Script executed:
# Also check for sectionModels.ts
find . -type f -name "sectionModels.ts" 2>/dev/null | head -20Repository: JesusFilm/forge
Length of output: 97
🏁 Script executed:
# List the mobile directory structure to understand the layout
ls -la mobile/ 2>/dev/null || echo "mobile directory not found"Repository: JesusFilm/forge
Length of output: 451
🏁 Script executed:
# Read the CTARenderer.tsx file to verify the code
cat -n ./mobile/expo/src/components/sections/CTARenderer.tsxRepository: JesusFilm/forge
Length of output: 3524
🏁 Script executed:
# Read the sectionModels.ts file to verify buttonLink is nullable
cat -n ./mobile/expo/src/lib/sectionModels.tsRepository: JesusFilm/forge
Length of output: 6199
Disable the CTA when there is no target, and handle failed URL opens.
Line 15 discards the Linking.openURL Promise rejection, and lines 33–45 still expose the control as an active link when buttonLink is null. With CMS-driven data, missing or malformed URLs become either a dead accessible control or an unhandled rejection on tap. Derive a disabled state from buttonLink and catch the open failure.
Suggested fix
export function CTARenderer({ section }: CTARendererProps) {
const { heading, body, buttonLabel, buttonLink, variant } = section
const isPrimary = variant !== "secondary"
+ const isDisabled = buttonLink == null
- const handlePress = () => {
- if (buttonLink) {
- void Linking.openURL(buttonLink)
- }
+ const handlePress = async () => {
+ if (buttonLink == null) return
+ try {
+ await Linking.openURL(buttonLink)
+ } catch {
+ console.warn(`CTARenderer: failed to open CTA link for section "${section.id}"`)
+ }
}
return (
@@
<Pressable
@@
onPress={handlePress}
+ disabled={isDisabled}
accessibilityRole="link"
accessibilityLabel={buttonLabel}
+ accessibilityState={{ disabled: isDisabled }}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/expo/src/components/sections/CTARenderer.tsx` around lines 13 - 16,
The CTA currently calls Linking.openURL inside handlePress and discards Promise
rejections and the UI still renders as an active link when buttonLink is null;
update the CTARenderer so the clickable control derives a disabled state from
buttonLink (e.g., treat null/empty/invalid URL as disabled) and reflect that in
the rendered button/Touchable (so it is not accessible/pressable when disabled),
and modify handlePress to await or call Linking.openURL(buttonLink).catch(...)
to handle and log/show failures instead of discarding the rejection; reference
the handlePress function and the conditional around buttonLink in CTARenderer to
implement these changes.
There was a problem hiding this comment.
Fixed in de2d9b4 — added disabled prop and reduced opacity styling when buttonLink is null. The button is now visually muted and non-interactive when there is no target URL.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cat -n ./mobile/expo/src/components/sections/CTARenderer.tsx
Length of output: 3667
@up-tandem, the disabled prop, isDisabled derivation, and the buttonDisabled opacity style are all correctly in place — the control is now properly non-interactive and visually muted when buttonLink is null. Thanks for addressing that.
One part of the original comment is still open though: Line 16 still uses void Linking.openURL(buttonLink), which silently swallows any rejection (e.g. a malformed URL from the CMS). The handlePress could be updated to catch and log the failure:
const handlePress = async () => {
if (buttonLink == null) return
try {
await Linking.openURL(buttonLink)
} catch {
console.warn(`CTARenderer: failed to open URL "${buttonLink}"`)
}
}Would you like to address that as well, or defer it to a follow-up?
🧠 Learnings used
Learnt from: up-tandem
Repo: JesusFilm/forge PR: 321
File: mobile/expo/src/components/sections/VideoHeroRenderer.tsx:30-84
Timestamp: 2026-03-10T00:46:57.860Z
Learning: Do not flag ts-expect-error on JSX elements in the section renderers under mobile/expo/src/components/sections/. This is a project-wide convention (introduced in scaffold PR `#317`) to address React 19 vs React Native type incompatibilities. Treat migrations to import { JSX } from 'react' as a separate cross-cutting chore applied uniformly across the codebase.
Add disabled state and reduced opacity when buttonLink is missing to prevent exposing a dead accessible control. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
primary(filled) andsecondary(outlined) variant stylingPressable+Linking.openURLfor button interactionContracts Changed
No
Regeneration Required
No
Validation
pnpm --filter @forge/expo lint— passes with 0 warningspnpm --filter @forge/expo test -- --testPathPattern=CTARenderer— 5/5 tests passpnpm --filter @forge/expo test -- --testPathPattern=SectionDispatcher— 11/11 tests pass (no regressions)Resolves #307
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests