Skip to content

Fix Tabs component package inheritance for tabs without packages #5929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
facumenzella wants to merge 10 commits into
base: main
Choose a base branch
from
+950 −12
Open

Fix Tabs component package inheritance for tabs without packages #5929

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ac193d8
Fix bug where package-less tab does no inherit parent package
facumenzella Dec 11, 2025
83fb53f
extract into function
facumenzella Dec 11, 2025
9dcfda8
add missing file to project
facumenzella Dec 11, 2025
a473d6f
Merge branch 'main' into fix/tabs-package-inheritance
facumenzella Dec 11, 2025
8836ed0
Merge branch 'main' into fix/tabs-package-inheritance
facumenzella Dec 12, 2025
02e70ba
fix issue with dup packages
facumenzella Dec 12, 2025
8a6e4d8
updated tests to new logic
facumenzella Dec 15, 2025
8343596
Merge branch 'main' into fix/tabs-package-inheritance
facumenzella Dec 15, 2025
0a8f5ae
propagate selection down the tabs
facumenzella Dec 15, 2025
441a44d
latest updates
facumenzella Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions Package.resolved
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions RevenueCat.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,7 @@
57BB08682DD3D9EC007493E1 /* SubscriptionDetailViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57BB08672DD3D9EA007493E1 /* SubscriptionDetailViewModelTests.swift */; };
57BD50AA27692B7500211D6D /* StoreKitError+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57BD50A927692B7500211D6D /* StoreKitError+Extensions.swift */; };
57BF87592967880C00424254 /* MockCachingTrialOrIntroPriceEligibilityChecker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57BF87582967880C00424254 /* MockCachingTrialOrIntroPriceEligibilityChecker.swift */; };
57BFCD0B2EEB0E56009E5844 /* TabsPackageInheritanceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57BFCD0A2EEB0E56009E5844 /* TabsPackageInheritanceTests.swift */; };
57C0BB6929F840BD00827807 /* FrameworkDisambiguation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57C0BB6829F840BD00827807 /* FrameworkDisambiguation.swift */; };
57C2931528BFEF4F0054EDFC /* PurchasesError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57C2931428BFEF4F0054EDFC /* PurchasesError.swift */; };
57C381B72791E593009E3940 /* StoreKit2TransactionListenerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57C381B62791E593009E3940 /* StoreKit2TransactionListenerTests.swift */; };
Expand Down Expand Up @@ -2356,6 +2357,7 @@
57BB08672DD3D9EA007493E1 /* SubscriptionDetailViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionDetailViewModelTests.swift; sourceTree = "<group>"; };
57BD50A927692B7500211D6D /* StoreKitError+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "StoreKitError+Extensions.swift"; sourceTree = "<group>"; };
57BF87582967880C00424254 /* MockCachingTrialOrIntroPriceEligibilityChecker.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockCachingTrialOrIntroPriceEligibilityChecker.swift; sourceTree = "<group>"; };
57BFCD0A2EEB0E56009E5844 /* TabsPackageInheritanceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabsPackageInheritanceTests.swift; sourceTree = "<group>"; };
57C0BB6829F840BD00827807 /* FrameworkDisambiguation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FrameworkDisambiguation.swift; sourceTree = "<group>"; };
57C2931428BFEF4F0054EDFC /* PurchasesError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PurchasesError.swift; sourceTree = "<group>"; };
57C381B62791E593009E3940 /* StoreKit2TransactionListenerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoreKit2TransactionListenerTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2958,6 +2960,7 @@
030890822D2B77DD0069677B /* PaywallsV2 */ = {
isa = PBXGroup;
children = (
57BFCD0A2EEB0E56009E5844 /* TabsPackageInheritanceTests.swift */,
03CCFCE12DE9638F0052B764 /* __PreviewResources__ */,
83EE33252E1E20360011CF1C /* PaywallPreviewResourcesLoader.swift */,
038EC8222DE7A8AC00786C42 /* TakeScreenshot.swift */,
Expand Down Expand Up @@ -7892,6 +7895,7 @@
887A632B2C1D177800E1A461 /* LocalizedAlertErrorTests.swift in Sources */,
887A632C2C1D177800E1A461 /* PackageVariablesTests.swift in Sources */,
887A632D2C1D177800E1A461 /* PaywallDataValidationTests.swift in Sources */,
57BFCD0B2EEB0E56009E5844 /* TabsPackageInheritanceTests.swift in Sources */,
887A632E2C1D177800E1A461 /* TemplateViewConfigurationTests.swift in Sources */,
887A632F2C1D177800E1A461 /* VariablesTests.swift in Sources */,
887A63302C1D177800E1A461 /* AsyncTestHelpers.swift in Sources */,
Expand Down
97 changes: 85 additions & 12 deletions RevenueCatUI/Templates/V2/Components/Tabs/TabsComponentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,61 @@ struct LoadedTabsComponentView: View {
defaultTabId: viewModel.defaultTabId
))

// MARK: - Package Context Inheritance for Tabs
//
// This handles a nuanced scenario where tabs may or may not have their own packages:
//
// Example structure:
// - Package A (parent scope, default)
// - Package B (parent scope)
// - Tabs Component
// - Tab 1: has Package C (its own package)
// - Tab 2: no packages (should inherit from parent)
//
// Requirements:
// 1. Tabs WITH packages: use their own package context, propagate to parent for purchase button
// 2. Tabs WITHOUT packages: inherit from parent's selected package
// 3. Tabs WITHOUT packages should NOT be affected when tabs WITH packages propagate their package
// 4. Tabs WITHOUT packages SHOULD update when user selects a different parent package
Copy link
Member

@ajpallares ajpallares Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading this I wonder what happens in this scenario, same example structure:

  • Initially, Package A is the default.
  • Select Tab 1, and then select Package C in that tab --> "propagate to parent for purchase button".
  • Select Tab 2.
    What package is shown in Tab 2?

According to the explanation below

This context will be kept in sync with parent changes via onChangeOf in the body, which filters out propagations from tabs with packages.

This means that Tab 1 would always show the latest Parent package selected (A or B) even when Package C could be the latest package selected (from tab 2).

Is that what we want? That's what I gather from

Tabs WITHOUT packages should NOT be affected when tabs WITH packages propagate their package

But in the scenario above, what package is shown in Tab 2?

Copy link
Contributor Author

@facumenzella facumenzella Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a video of the behavior:
Uploading Simulator Screen Recording - iPhone 16 Pro - 2025-12-12 at 10.38.25.mov…

For Tab2, the "active" package will be first the default in the parent scope, or the latest selected one.

//
// Solution:
// - Create a PackageContext for ALL tabs (not just those with packages)
// - Tabs with packages: initialized with their own packages
// - Tabs without packages: initialized with parent's current state, then kept in sync
// via `onChangeOf` observer that filters out tab propagations (see body)
//
self._tierPackageContexts = .init(initialValue: Dictionary(
uniqueKeysWithValues: viewModel.tabViewModels.map { key, tabViewModel in
let packageContext = PackageContext(
package: tabViewModel.defaultSelectedPackage,
variableContext: .init(
packages: tabViewModel.packages,
showZeroDecimalPlacePrices: parentPackageContext.variableContext.showZeroDecimalPlacePrices
uniqueKeysWithValues: viewModel.tabViewModels.map { key, tabViewModel -> (String, PackageContext) in
if !tabViewModel.packages.isEmpty {
// Tab has its own packages - create context with tab's packages
let packageContext = PackageContext(
package: tabViewModel.defaultSelectedPackage,
variableContext: .init(
packages: tabViewModel.packages,
showZeroDecimalPlacePrices: parentPackageContext.variableContext.showZeroDecimalPlacePrices
)
)
)
return (key, packageContext)
return (key, packageContext)
} else {
// Tab has no packages - inherit from parent's current state.
// This context will be kept in sync with parent changes via `onChangeOf`
// in the body, which filters out propagations from tabs with packages.
let packageContext = PackageContext(
package: parentPackageContext.package,
variableContext: parentPackageContext.variableContext
)
Copy link
Member

@ajpallares ajpallares Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we're creating a new instance of PackageContext, which means that the onChangeOf won't really work because the @Published package of the new PackageContext won't change when the parent's package changes. In other words, this requirement

Tabs WITHOUT packages SHOULD update when user selects a different parent package
would not work right now.

I'm not 100% sure about this, but, if the Tab component doesn't have any package itself at this point (tabViewModel.packages.isEmpty), I think you could just do

return (key, parentPackageContext)

But we'd need to test it thoroughly, as I'm not sure that this behavior would match the desired behavior (mostly about not propagating the selected package inside Tab 1 to Tab 2)

Copy link
Contributor Author

@facumenzella facumenzella Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this works because:

  1. self.packageContext (line 73) is the parent's @EnvironmentObject
  2. onChangeOf(self.packageContext.package) watches THIS parent context
  3. When parent changes, syncParentPackageToTabsWithoutPackages updates the separate tier contexts

return (key, packageContext)
}
}
))
}

/// Set of package identifiers that belong to tabs (not parent scope).
/// Used to distinguish between parent package selections and tab package propagations.
private var tabPackageIdentifiers: Set<String> {
Set(viewModel.tabViewModels.values.flatMap { $0.packages.map(\.identifier) })
}

var body: some View {
if let activeTabViewModel,
let tierPackageContext = self.tierPackageContexts[self.tabControlContext.selectedTabId] {
Expand All @@ -138,10 +179,8 @@ struct LoadedTabsComponentView: View {
.onAppear {
if !wasConfigured {
self.wasConfigured = true
// In the event that the tabs components contain unique selected packages, we need to ensure that
// the first selected tab's selected package is propagated up to the purchase button. This sends
// that signal only for the initially rendered tab, then the onChange passed into the loadedTabView
// handles subsequent changes
// Propagate the initial tab's package to parent context for the purchase button.
// Subsequent changes are handled by the onChange callback in LoadedTabComponentView.
if let package = tierPackageContext.package {
self.packageContext.update(
package: package,
Expand All @@ -150,6 +189,40 @@ struct LoadedTabsComponentView: View {
}
}
}
.onChangeOf(self.packageContext.package) { newPackage in
self.syncParentPackageToTabsWithoutPackages(newPackage)
}
}
}

// MARK: - Sync parent package changes to tabs without packages
//
// This keeps tabs without packages in sync with parent package selections,
// while filtering out propagations from tabs that have their own packages.
//
// Flow example:
// 1. User selects Package B (parent) → packageContext updates to B
// 2. This function checks: is B a tab package? No.
// 3. Updates Tab 2's context (which has no packages) with Package B ✓
//
// Filtered example:
// 1. Tab 1 propagates Package C → packageContext updates to C
// 2. This function checks: is C a tab package? Yes (it's in Tab 1).
// 3. Does NOT update Tab 2's context - Tab 2 keeps showing parent's package ✓
//
private func syncParentPackageToTabsWithoutPackages(_ newPackage: Package?) {
guard let newPackage = newPackage else { return }

let isTabPackage = self.tabPackageIdentifiers.contains(newPackage.identifier)

if !isTabPackage {
// Parent package selection - update all tabs without packages
for (tabId, tabViewModel) in self.viewModel.tabViewModels where tabViewModel.packages.isEmpty {
self.tierPackageContexts[tabId]?.update(
package: newPackage,
variableContext: self.packageContext.variableContext
)
}
}
}

Expand Down
Loading