Skip to content
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.

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

Fixes for paywall tester #1

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions RevenueCat.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
1E99F81F2AC5917F0023E26E /* StoreMessagesHelperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E99F81D2AC5917F0023E26E /* StoreMessagesHelperTests.swift */; };
1ED4CA9F2CC25A5F0021AB8F /* SafariView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1ED4CA9E2CC25A5F0021AB8F /* SafariView.swift */; };
2C08B2E92CD40DBF0024857B /* ButtonComponentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C08B2E82CD40DBF0024857B /* ButtonComponentTests.swift */; };
2C08B3072CD55D660024857B /* FlexHStack.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C08B3062CD55D590024857B /* FlexHStack.swift */; };
2C0B98CD2797070B00C5874F /* PromotionalOffer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C0B98CC2797070B00C5874F /* PromotionalOffer.swift */; };
2C2AEB0F2CA64E0E00A50F38 /* Template1Preview.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C2AEB0E2CA64E0E00A50F38 /* Template1Preview.swift */; };
2C2AEB3B2CA7209F00A50F38 /* PaywallPackageComponent.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C2AEB3A2CA7209F00A50F38 /* PaywallPackageComponent.swift */; };
Expand Down Expand Up @@ -1178,6 +1179,7 @@
1E99F81D2AC5917F0023E26E /* StoreMessagesHelperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoreMessagesHelperTests.swift; sourceTree = "<group>"; };
1ED4CA9E2CC25A5F0021AB8F /* SafariView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SafariView.swift; sourceTree = "<group>"; };
2C08B2E82CD40DBF0024857B /* ButtonComponentTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ButtonComponentTests.swift; sourceTree = "<group>"; };
2C08B3062CD55D590024857B /* FlexHStack.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FlexHStack.swift; sourceTree = "<group>"; };
2C0B98CC2797070B00C5874F /* PromotionalOffer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PromotionalOffer.swift; sourceTree = "<group>"; };
2C2AEB0E2CA64E0E00A50F38 /* Template1Preview.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Template1Preview.swift; sourceTree = "<group>"; };
2C2AEB3A2CA7209F00A50F38 /* PaywallPackageComponent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaywallPackageComponent.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4495,6 +4497,7 @@
88B1BAEA2C813A3C001B7EE5 /* Stack */ = {
isa = PBXGroup;
children = (
2C08B3062CD55D590024857B /* FlexHStack.swift */,
88B1BAE82C813A3C001B7EE5 /* StackComponentView.swift */,
88B1BAE92C813A3C001B7EE5 /* StackComponentViewModel.swift */,
);
Expand Down Expand Up @@ -6213,6 +6216,7 @@
887A60C12C1D037000E1A461 /* DebugErrorView.swift in Sources */,
887A607C2C1D037000E1A461 /* ColorInformation+MultiScheme.swift in Sources */,
88B1BAFE2C813A3C001B7EE5 /* ImageComponentViewModel.swift in Sources */,
2C08B3072CD55D660024857B /* FlexHStack.swift in Sources */,
35C200B12C39254100B9778B /* FeedbackSurveyView.swift in Sources */,
35F249CC2C493DCC0058993A /* CustomerCenterPurchasesType.swift in Sources */,
887A60672C1D037000E1A461 /* PaywallError.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ extension PaywallComponent.FontWeight {

var fontWeight: Font.Weight {
switch self {
case .ultraLight:
case .extraLight:
return .ultraLight
case .thin:
return .thin
Expand All @@ -74,7 +74,7 @@ extension PaywallComponent.FontWeight {
return .semibold
case .bold:
return .bold
case .heavy:
case .extraBold:
return .heavy
case .black:
return .black
Expand Down Expand Up @@ -127,6 +127,17 @@ extension PaywallComponent.TwoDimensionAlignment {

extension PaywallComponent.HorizontalAlignment {

var stackAlignment: SwiftUI.HorizontalAlignment {
switch self {
case .leading:
return .leading
case .center:
return .center
case .trailing:
return .trailing
}
}

var textAlignment: TextAlignment {
switch self {
case .leading:
Expand All @@ -138,7 +149,7 @@ extension PaywallComponent.HorizontalAlignment {
}
}

var stackAlignment: SwiftUI.Alignment {
var frameAlignment: SwiftUI.Alignment {
switch self {
case .leading:
return .leading
Expand Down
96 changes: 96 additions & 0 deletions RevenueCatUI/Templates/Components/Stack/FlexHStack.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
//
// Copyright RevenueCat Inc. All Rights Reserved.
//
// Licensed under the MIT License (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://opensource.org/licenses/MIT
//
// FlexHStack.swift
//
// Created by Josh Holtz on 11/1/24.

import SwiftUI

#if PAYWALL_COMPONENTS

enum JustifyContent {
case start, center, end, spaceBetween, spaceAround, spaceEvenly
}

@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
struct FlexHStack: View {
let alignment: VerticalAlignment
let justifyContent: JustifyContent
let spacing: CGFloat?
let componentViewModels: [PaywallComponentViewModel]

let onDismiss: () -> Void

init(
alignment: VerticalAlignment,
spacing: CGFloat?,
justifyContent: JustifyContent,
componentViewModels: [PaywallComponentViewModel],
onDismiss: @escaping () -> Void
) {
self.alignment = alignment
self.spacing = spacing
self.justifyContent = justifyContent
self.componentViewModels = componentViewModels
self.onDismiss = onDismiss
}

var body: some View {
HStack(alignment: self.alignment, spacing: self.spacing) {
switch justifyContent {
case .start:
ForEach(0..<componentViewModels.count, id: \.self) { index in
ComponentsView(componentViewModels: [self.componentViewModels[index]], onDismiss: self.onDismiss)
}
Comment on lines +49 to +51
Copy link

Choose a reason for hiding this comment

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

logic: Using array index directly as id could cause issues if items change. Consider using a more stable identifier from PaywallComponentViewModel

Spacer()

case .center:
Spacer()
ForEach(0..<componentViewModels.count, id: \.self) { index in
ComponentsView(componentViewModels: [self.componentViewModels[index]], onDismiss: self.onDismiss)
}
Spacer()

case .end:
Spacer()
ForEach(0..<componentViewModels.count, id: \.self) { index in
ComponentsView(componentViewModels: [self.componentViewModels[index]], onDismiss: self.onDismiss)
}

case .spaceBetween:
ForEach(0..<componentViewModels.count, id: \.self) { index in
ComponentsView(componentViewModels: [self.componentViewModels[index]], onDismiss: self.onDismiss)
if index < self.componentViewModels.count - 1 {
Spacer()
}
}

case .spaceAround:
Spacer()
ForEach(0..<componentViewModels.count, id: \.self) { index in
ComponentsView(componentViewModels: [self.componentViewModels[index]], onDismiss: self.onDismiss)
if index < self.componentViewModels.count - 1 {
Spacer()
}
}
Spacer()

case .spaceEvenly:
ForEach(0..<componentViewModels.count, id: \.self) { index in
Spacer()
ComponentsView(componentViewModels: [self.componentViewModels[index]], onDismiss: self.onDismiss)
}
Spacer()
Comment on lines +86 to +90
Copy link

Choose a reason for hiding this comment

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

logic: spaceEvenly implementation places Spacer before first item, which differs from CSS flex justify-content: space-evenly. Should have equal space between all items

}
}
}
}

#endif
74 changes: 67 additions & 7 deletions RevenueCatUI/Templates/Components/Stack/StackComponentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,53 @@ struct StackComponentView: View {
Group {
switch viewModel.dimension {
case .vertical(let horizontalAlignment):
// LazyVStack needed for performance when loading
LazyVStack(spacing: viewModel.spacing) {
ComponentsView(componentViewModels: self.viewModel.viewModels, onDismiss: self.onDismiss)
.frame(maxWidth: .infinity, alignment: horizontalAlignment.stackAlignment)
Group {
// This is NOT a final implementation of this
// There are some horizontal sizing issues with using LazyVStack
// There are so performance issues with VStack with lots of children
Comment on lines +40 to +41
Copy link

Choose a reason for hiding this comment

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

syntax: typo in comment: 'There are so performance issues' should be 'There are some performance issues'

if viewModel.shouldUseVStack {
Copy link

Choose a reason for hiding this comment

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

logic: using a hardcoded threshold of 3 items (from ViewModel) to switch between VStack and LazyVStack could cause performance issues with complex child views even with few items

// VStack when not many things
VStack(
alignment: horizontalAlignment.stackAlignment,
spacing: viewModel.spacing
) {
ComponentsView(
componentViewModels: self.viewModel.viewModels,
onDismiss: self.onDismiss
)
}
} else {
// LazyVStack needed for performance when loading
LazyVStack(
alignment: horizontalAlignment.stackAlignment,
spacing: viewModel.spacing
) {
ComponentsView(
componentViewModels: self.viewModel.viewModels,
onDismiss: self.onDismiss
)
}
}
}
case .horizontal(let verticalAlignment):
HStack(alignment: verticalAlignment.stackAlignment, spacing: viewModel.spacing) {
ComponentsView(componentViewModels: self.viewModel.viewModels, onDismiss: self.onDismiss)
.applyIf(viewModel.shouldUseFlex) {
$0.frame(
maxWidth: .infinity,
alignment: horizontalAlignment.frameAlignment
)
}
case .horizontal(let verticalAlignment, let distribution):
if viewModel.shouldUseFlex {
FlexHStack(
alignment: verticalAlignment.stackAlignment,
spacing: viewModel.spacing,
justifyContent: distribution.justifyContent,
componentViewModels: self.viewModel.viewModels,
onDismiss: self.onDismiss
)
} else {
HStack(alignment: verticalAlignment.stackAlignment, spacing: viewModel.spacing) {
ComponentsView(componentViewModels: self.viewModel.viewModels, onDismiss: self.onDismiss)
}
}
case .zlayer(let alignment):
ZStack(alignment: alignment.stackAlignment) {
Expand All @@ -65,6 +104,27 @@ struct StackComponentView: View {

}

extension PaywallComponent.FlexDistribution {

var justifyContent: JustifyContent {
switch self {
case .start:
return .start
case .center:
return .center
case .end:
return .end
case .spaceBetween:
return .spaceBetween
case .spaceAround:
return .spaceAround
case .spaceEvenly:
return .spaceEvenly
}
}

}

struct WidthModifier: ViewModifier {
var width: PaywallComponent.WidthSize?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,33 @@ class StackComponentViewModel {
self.viewModels = viewModels
}

var shouldUseVStack: Bool {
switch self.dimension {
case .vertical:
if viewModels.count < 3 {
return true
}
return false
case .horizontal, .zlayer:
Comment on lines +53 to +57
Copy link

Choose a reason for hiding this comment

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

style: Simplify to return viewModels.count < 3 since both paths return a boolean

return false
}
}

var shouldUseFlex: Bool {
guard let widthType = self.component.width?.type else {
return false
}

switch widthType {
case .fit:
return false
case .fill:
return true
case .fixed:
return true
}
Comment on lines +67 to +74
Copy link

Choose a reason for hiding this comment

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

style: Consider combining .fill and .fixed cases since they have the same behavior

}

var dimension: PaywallComponent.Dimension {
component.dimension
}
Expand Down
11 changes: 6 additions & 5 deletions RevenueCatUI/Templates/Components/TemplateComponentsView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ struct TemplateComponentsView: View {
self.onDismiss = onDismiss

// Step 0: Decide which ComponentsConfig to use (base is default)
let componentsConfig = paywallComponentsData.componentsConfigs.base
let componentsConfig = paywallComponentsData.componentsConfig.base

// Step 1: Get localization
let localization = Self.chooseLocalization(for: paywallComponentsData)
Expand Down Expand Up @@ -113,10 +113,11 @@ struct TemplateComponentsView: View {
)
)

guard packageValidator.isValid else {
Logger.error(Strings.paywall_could_not_find_any_packages)
throw PackageGroupValidationError.noAvailablePackages("No available packages found")
}
// WIP: Maybe re-enable this later or add some warnings
// guard packageValidator.isValid else {
// Logger.error(Strings.paywall_could_not_find_any_packages)
// throw PackageGroupValidationError.noAvailablePackages("No available packages found")
// }
Comment on lines +116 to +120
Copy link

Choose a reason for hiding this comment

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

logic: Commenting out package validation could lead to runtime issues if no valid packages exist. Consider adding a warning or fallback behavior instead of completely removing the validation.


self.componentViewModel = componentViewModel
self._paywallState = .init(wrappedValue: PaywallState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ private enum Template1Preview {
static let title = PaywallComponent.TextComponent(
text: "title",
fontFamily: nil,
fontWeight: .heavy,
fontWeight: .black,
color: .init(light: "#000000"),
backgroundColor: nil,
padding: .zero,
Expand Down Expand Up @@ -137,7 +137,7 @@ private enum Template1Preview {
static let data: PaywallComponentsData = .init(
templateName: "components",
assetBaseURL: URL(string: "https://assets.pawwalls.com")!,
componentsConfigs: .init(
componentsConfig: .init(
base: .init(
stack: .init(
components: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private enum Template5Preview {
static let title = PaywallComponent.TextComponent(
text: "title",
fontFamily: nil,
fontWeight: .heavy,
fontWeight: .black,
color: .init(light: "#000000"),
backgroundColor: nil,
padding: .zero,
Expand Down Expand Up @@ -171,7 +171,7 @@ private enum Template5Preview {
components: [
.purchaseButton(purchaseButton)
],
dimension: .horizontal(.center),
dimension: .horizontal(.center, .start),
Copy link

Choose a reason for hiding this comment

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

style: horizontal dimension now includes .start parameter which may affect button alignment - verify this change is intentional and doesn't break existing layouts

width: .init(type: .fill, value: nil),
spacing: 0,
backgroundColor: nil
Expand Down Expand Up @@ -207,7 +207,7 @@ private enum Template5Preview {
static let data: PaywallComponentsData = .init(
templateName: "components",
assetBaseURL: URL(string: "https://assets.pawwalls.com")!,
componentsConfigs: .init(
componentsConfig: .init(
base: .init(
stack: .init(
components: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ struct TextComponentView_Previews: PreviewProvider {
component: .init(
text: "id_1",
fontFamily: nil,
fontWeight: .heavy,
fontWeight: .black,
color: .init(light: "#ff0000"),
backgroundColor: .init(light: "#dedede"),
padding: .init(top: 10,
Expand Down
Loading