[Paywalls V2] New overrides structure#4705
Merged
Merged
Conversation
1 build decreased size
Paywalls 1.0 (1)
|
| Item | Install Size Change |
|---|---|
| DYLD.String Table | ⬇️ -39.9 kB |
| RevenueCatUI.TextComponentViewModel.TextComponentViewModel | ⬇️ -14.4 kB |
| RevenueCatUI.ImageComponentViewModel.ImageComponentViewModel | ⬇️ -12.2 kB |
| Code Signature | ⬇️ -4.9 kB |
| DYLD.Exports | ⬇️ -1.4 kB |
View Treemap
🛸 Powered by Emerge Tools
Comment trigger: Size diff threshold of 100.00kB exceeded
joshdholtz
commented
Jan 27, 2025
Comment on lines
+63
to
+68
| for presentedOverride in presentedOverrides where self.shouldApply( | ||
| for: presentedOverride.conditions, | ||
| state: state, | ||
| activeCondition: condition, | ||
| isEligibleForIntroOffer: isEligibleForIntroOffer) { | ||
| presentedPartial = Self.combine(presentedPartial, with: presentedOverride.properties) |
Member
Author
There was a problem hiding this comment.
This is where the overrides are iterated from lowest to higher priority (in the order they are given) and will only combine them if all the conditions are true
joshdholtz
commented
Jan 27, 2025
Comment on lines
+74
to
+96
| private static func shouldApply( | ||
| for conditions: [PaywallComponent.Condition], | ||
| state: ComponentViewState, | ||
| activeCondition: ScreenCondition, | ||
| isEligibleForIntroOffer: Bool | ||
| ) -> Bool { | ||
| // Early return when any condition evaluates to false | ||
| for condition in conditions { | ||
| switch condition { | ||
| case .compact, .medium, .expanded: | ||
| if !activeCondition.applicableConditions.contains(condition) { | ||
| return false | ||
| } | ||
| case .introOffer: | ||
| if !isEligibleForIntroOffer { | ||
| return false | ||
| } | ||
| case .selected: | ||
| if state != .selected { | ||
| return false | ||
| } | ||
| case .unsupported: | ||
| return false |
Member
Author
There was a problem hiding this comment.
Logic for determining if all the conditions are true
joshdholtz
commented
Jan 27, 2025
Comment on lines
+46
to
+47
| // For unknown cases | ||
| case unsupported |
Member
Author
There was a problem hiding this comment.
I added a case for unsupported so that the decoding wouldn't fail for multiple_intro_offers or if any other new ones where added
JayShortway
approved these changes
Jan 27, 2025
Comment on lines
+63
to
+68
| for presentedOverride in presentedOverrides where self.shouldApply( | ||
| for: presentedOverride.conditions, | ||
| state: state, | ||
| activeCondition: condition, | ||
| isEligibleForIntroOffer: isEligibleForIntroOffer) { | ||
| presentedPartial = Self.combine(presentedPartial, with: presentedOverride.properties) |
|
|
||
| } | ||
|
|
||
| private extension ScreenCondition { |
Member
There was a problem hiding this comment.
I'm not suggesting or requesting any changes, especially not in this PR, but do we still need ScreenCondition? Can we not make do with just PaywallComponent.Condition?
Member
Author
There was a problem hiding this comment.
Ohhhhhh, this is a good point! I think we can totally merge the two in a follow up PR 👍
Comment on lines
+46
to
+47
| // For unknown cases | ||
| case unsupported |
This was referenced Jan 29, 2025
tonidero
added a commit
to RevenueCat/purchases-android
that referenced
this pull request
Feb 7, 2025
## Motivation Migrating `overrides` from a structured object to an array of override objects ## Description ### Previously Overrides were a very structure object that had all of the different types of states/conditions that allowed base properties of a component to be overridden. However, this was not flexible when it came to multiple being activated as it was missing for cases when multiples types of conditions were being evaluated. ### Now Overrides is an array that has an array of `conditions` and `properties`. It will get applied in order from first to last IF all of the `conditions` are true. This allows the FE/BE to determine the orders of overrides from lowest to higher priority so all of the SDKs/renderers don't need to duplicate that logic. Android equivalent of RevenueCat/purchases-ios#4705
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
2 participants
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.
Motivation
Migrating
overridesfrom a structured object to an array of override objectsDescription
Previously
Overrides were a very structure object that had all of the different types of states/conditions that allowed base properties of a component to be overridden. However, this was not flexible when it came to multiple being activated as it was missing for cases when multiples types of conditions were being evaluated.
Now
Overrides is an array that has an array of
conditionsandproperties. It will get applied in order from first to last IF all of theconditionsare true.This allows the FE/BE to determine the orders of overrides from lowest to higher priority so all of the SDKs/renderers don't need to duplicate that logic.