feat(mobile-ios): Section renderer – CTA (#107)#332
Merged
Ur-imazing merged 5 commits intomainfrom Mar 11, 2026
Merged
Conversation
WalkthroughReplaces in-file section rendering with a dispatcher hierarchy: ForgeRootView now uses ExperienceSectionListView to render sections; new ExperienceSectionListView, ExperienceSectionView, SectionContentView, CTAView, and UnsupportedSectionView handle ordered section rendering and dispatching (DEBUG shows placeholders for unsupported types). Changes
Sequence DiagramsequenceDiagram
participant Root as ForgeRootView
participant List as ExperienceSectionListView
participant Dispatcher as ExperienceSectionView
participant Content as SectionContentView
participant CTA as CTAView
participant Unsupported as UnsupportedSectionView
Root->>List: render(sections)
List->>List: iterate sections
List->>Dispatcher: init(for: section)
Dispatcher->>Content: if .leaf -> pass content
Content->>Content: switch on content.type
alt content == .cta
Content->>CTA: render CTAView(content)
CTA->>CTA: compose heading, body, button (Link or Button)
else
Content->>Unsupported: render placeholder (DEBUG only)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
mobile/ios/Sources/ForgeMobile/Views/ForgeRootView.swift (1)
48-52: Hardcoded experience slug and locale.The slug
"easter"and locale"en"are hardcoded. This is acceptable for initial development, but consider adding a TODO comment to track making these configurable for production use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/ios/Sources/ForgeMobile/Views/ForgeRootView.swift` around lines 48 - 52, The call to viewModel.loadExperience(slug: "easter", locale: "en") uses hardcoded values; update the call site in ForgeRootView (the .task block where viewModel.loadExperience is invoked) by adding a TODO comment above it indicating these slug/locale values must be made configurable for production (e.g., TODO: make experience slug and locale configurable / remove hardcoded "easter" and "en" for production) and consider replacing the literals later with passed-in properties or configuration; leave the current call intact for now but ensure the TODO references viewModel.loadExperience to make the needed change discoverable.mobile/ios/Sources/ForgeMobile/Views/Sections/ExperienceSectionListView.swift (1)
9-13: Index-based ID may cause view state issues on section reordering.Using
id: \.offsetidentifies items by array position. If sections are reordered or inserted dynamically, SwiftUI will associate view state with the wrong items, potentially causing incorrect animations or state retention.Since
ExperienceSectiondoesn't conform toIdentifiable, consider extracting a stable ID from the underlying content:♻️ Proposed approach using stable section IDs
Add an
idcomputed property toExperienceSection:extension ExperienceSection { var id: String { switch self { case .leaf(let content): return content.id case .container(let container): return container.id case .section(let wrapper): return wrapper.id } } }Then update the ForEach:
- ForEach(Array(sections.enumerated()), id: \.offset) { _, section in + ForEach(sections, id: \.id) { section in ExperienceSectionView(section: section) }If sections are only loaded once and never mutate during the view's lifetime, the current approach is acceptable for initial implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/ios/Sources/ForgeMobile/Views/Sections/ExperienceSectionListView.swift` around lines 9 - 13, The ForEach currently uses the array index (id: \.offset) which can break view identity when sections are reordered; change to a stable identifier by adding a computed id on ExperienceSection (or conforming ExperienceSection to Identifiable) that extracts the underlying content id (reference the ExperienceSection type and its cases like .leaf/.container/.section) and update the ForEach in LazyVStack to iterate over sections using that stable id (e.g., ForEach(sections, id: \.id) { section in ExperienceSectionView(section: section) }) so view state tracks the correct section when items move.mobile/ios/Sources/ForgeMobile/Views/Sections/CTAView.swift (1)
43-69: Consider reducing duplication in button rendering.Both
linkButtonandactionButtonhave similar if-else structures differing only by.buttonStyle. This could be simplified by extracting the style selection.♻️ Optional refactor to reduce duplication
+ private var buttonStyleVariant: some PrimitiveButtonStyle { + section.variant == .secondary ? .bordered : .borderedProminent + } + `@ViewBuilder` private func linkButton(url: URL) -> some View { - if section.variant == .secondary { - Link(destination: url) { buttonContent } - .buttonStyle(.bordered) - .accessibilityLabel(section.buttonLabel) - .accessibilityAddTraits(.isLink) - } else { - Link(destination: url) { buttonContent } - .buttonStyle(.borderedProminent) - .accessibilityLabel(section.buttonLabel) - .accessibilityAddTraits(.isLink) - } + Link(destination: url) { buttonContent } + .buttonStyle(section.variant == .secondary ? .bordered : .borderedProminent) + .accessibilityLabel(section.buttonLabel) + .accessibilityAddTraits(.isLink) } `@ViewBuilder` private var actionButton: some View { - if section.variant == .secondary { - Button(action: {}, label: { buttonContent }) - .buttonStyle(.bordered) - .accessibilityLabel(section.buttonLabel) - } else { - Button(action: {}, label: { buttonContent }) - .buttonStyle(.borderedProminent) - .accessibilityLabel(section.buttonLabel) - } + Button(action: {}, label: { buttonContent }) + .buttonStyle(section.variant == .secondary ? .bordered : .borderedProminent) + .accessibilityLabel(section.buttonLabel) }Note: The ternary approach may require type erasure or a different approach since
.borderedand.borderedProminentare different types. The current explicit if-else works correctly and may be preferable for type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/ios/Sources/ForgeMobile/Views/Sections/CTAView.swift` around lines 43 - 69, Both linkButton and actionButton duplicate the same if/else differing only by which ButtonStyle is applied; extract the style selection into a helper to remove duplication: add a function like selectedButtonStyle() -> AnyButtonStyle (or a small helper buttonStyled<V: View>(_ content: V) -> some View) that returns .bordered or .borderedProminent based on section.variant, then call .buttonStyle(selectedButtonStyle()) from both linkButton and actionButton and keep the per-control accessibility modifiers (accessibilityAddTraits for Link) as-is; reference the existing functions linkButton and actionButton and the section.variant property when implementing the helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mobile/ios/Sources/ForgeMobile/Views/ForgeRootView.swift`:
- Around line 48-52: The call to viewModel.loadExperience(slug: "easter",
locale: "en") uses hardcoded values; update the call site in ForgeRootView (the
.task block where viewModel.loadExperience is invoked) by adding a TODO comment
above it indicating these slug/locale values must be made configurable for
production (e.g., TODO: make experience slug and locale configurable / remove
hardcoded "easter" and "en" for production) and consider replacing the literals
later with passed-in properties or configuration; leave the current call intact
for now but ensure the TODO references viewModel.loadExperience to make the
needed change discoverable.
In `@mobile/ios/Sources/ForgeMobile/Views/Sections/CTAView.swift`:
- Around line 43-69: Both linkButton and actionButton duplicate the same if/else
differing only by which ButtonStyle is applied; extract the style selection into
a helper to remove duplication: add a function like selectedButtonStyle() ->
AnyButtonStyle (or a small helper buttonStyled<V: View>(_ content: V) -> some
View) that returns .bordered or .borderedProminent based on section.variant,
then call .buttonStyle(selectedButtonStyle()) from both linkButton and
actionButton and keep the per-control accessibility modifiers
(accessibilityAddTraits for Link) as-is; reference the existing functions
linkButton and actionButton and the section.variant property when implementing
the helper.
In
`@mobile/ios/Sources/ForgeMobile/Views/Sections/ExperienceSectionListView.swift`:
- Around line 9-13: The ForEach currently uses the array index (id: \.offset)
which can break view identity when sections are reordered; change to a stable
identifier by adding a computed id on ExperienceSection (or conforming
ExperienceSection to Identifiable) that extracts the underlying content id
(reference the ExperienceSection type and its cases like
.leaf/.container/.section) and update the ForEach in LazyVStack to iterate over
sections using that stable id (e.g., ForEach(sections, id: \.id) { section in
ExperienceSectionView(section: section) }) so view state tracks the correct
section when items move.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6db3c7d-0757-41c1-a1b4-e8af6358d152
📒 Files selected for processing (5)
mobile/ios/Sources/ForgeMobile/Views/ForgeRootView.swiftmobile/ios/Sources/ForgeMobile/Views/Sections/CTAView.swiftmobile/ios/Sources/ForgeMobile/Views/Sections/ExperienceSectionListView.swiftmobile/ios/Sources/ForgeMobile/Views/Sections/ExperienceSectionView.swiftmobile/ios/Sources/ForgeMobile/Views/Sections/SectionContentView.swift
Introduce the section dispatch infrastructure for server-driven UI: - SectionContentView: switches on SectionContent enum to route leaf types - ExperienceSectionView: switches on ExperienceSection enum (leaf/container/section) - ExperienceSectionListView: renders ordered sections in a LazyVStack Unimplemented types show a debug placeholder in DEBUG, render nothing in release. This establishes the pattern subsequent Tier 1 renderers will plug into. Made-with: Cursor
Implement CTAView as the first Tier 1 leaf renderer for issue #107: - Renders heading, body text, and action button from CTASection model - Supports primary (borderedProminent) and secondary (bordered) variants - Conditionally renders Link when buttonLink is present, otherwise Button - Full accessibility labels and traits (isHeader, isLink) - SwiftUI previews for primary, secondary, and no-link states Update ForgeRootView to render sections via ExperienceSectionListView instead of dumping raw JSON, keeping a debug header in DEBUG builds. Made-with: Cursor
Ur-imazing
force-pushed
the
feat/107-cta-renderer
branch
from
b5ffa96 to
5d458af
Compare
Keep ExperienceSectionListView component over inline sectionsList, as the extracted component is the cleaner approach from the feature branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mobile/ios/Sources/ForgeMobile/Views/Sections/CTAView.swift (1)
43-56: Consider extracting variant-based styling to reduce duplication.Both
linkButtonandactionButtonduplicate the if/else structure for variant handling. You could extract the button style selection:♻️ Optional: Extract button style logic
private var variantButtonStyle: some PrimitiveButtonStyle { section.variant == .secondary ? .bordered : .borderedProminent }Then apply it once:
- `@ViewBuilder` - private func linkButton(url: URL) -> some View { - if section.variant == .secondary { - Link(destination: url) { buttonContent } - .buttonStyle(.bordered) - .accessibilityLabel(section.buttonLabel) - .accessibilityAddTraits(.isLink) - } else { - Link(destination: url) { buttonContent } - .buttonStyle(.borderedProminent) - .accessibilityLabel(section.buttonLabel) - .accessibilityAddTraits(.isLink) - } - } + private func linkButton(url: URL) -> some View { + Link(destination: url) { buttonContent } + .buttonStyle(section.variant == .secondary ? .bordered : .borderedProminent) + .accessibilityLabel(section.buttonLabel) + .accessibilityAddTraits(.isLink) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/ios/Sources/ForgeMobile/Views/Sections/CTAView.swift` around lines 43 - 56, Extract the duplicated variant-based button style logic into a single computed property (e.g., private var variantButtonStyle: some PrimitiveButtonStyle) that returns .bordered when section.variant == .secondary and .borderedProminent otherwise, then replace the if/else in linkButton (and the matching logic in actionButton) to always call Link or Button with .buttonStyle(variantButtonStyle) and keep the existing accessibilityLabel and accessibilityAddTraits calls unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mobile/ios/Sources/ForgeMobile/Views/Sections/CTAView.swift`:
- Around line 43-56: Extract the duplicated variant-based button style logic
into a single computed property (e.g., private var variantButtonStyle: some
PrimitiveButtonStyle) that returns .bordered when section.variant == .secondary
and .borderedProminent otherwise, then replace the if/else in linkButton (and
the matching logic in actionButton) to always call Link or Button with
.buttonStyle(variantButtonStyle) and keep the existing accessibilityLabel and
accessibilityAddTraits calls unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 781f479e-9058-42dd-894a-7bc0e0af882f
📒 Files selected for processing (5)
mobile/ios/Sources/ForgeMobile/Views/ForgeRootView.swiftmobile/ios/Sources/ForgeMobile/Views/Sections/CTAView.swiftmobile/ios/Sources/ForgeMobile/Views/Sections/ExperienceSectionListView.swiftmobile/ios/Sources/ForgeMobile/Views/Sections/ExperienceSectionView.swiftmobile/ios/Sources/ForgeMobile/Views/Sections/SectionContentView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- mobile/ios/Sources/ForgeMobile/Views/Sections/ExperienceSectionListView.swift
This was referenced Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CTAView) as the first Tier 1 leaf renderer, with primary/secondary variant styling, Link/Button conditional rendering, and full accessibility supportSectionContentView,ExperienceSectionView,ExperienceSectionListView) that subsequent renderers will plug intoForgeRootViewwith actual section rendering viaExperienceSectionListViewResolves #107
Contracts Changed
No
Regeneration Required
No
Validation
--strict: 0 violationsxcodebuild -project ForgeApp.xcodeproj -scheme ForgeApp -destination 'platform=iOS Simulator,name=iPhone 16' build: BUILD SUCCEEDEDMade with Cursor
Summary by CodeRabbit
New Features
Refactor