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

Paywalls: add support for multiple images in template configuration #2832

Merged
merged 1 commit into from
Jul 18, 2023
Merged
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
2 changes: 1 addition & 1 deletion RevenueCatUI/Data/TemplateViewConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ struct TemplateViewConfiguration {
let packages: PackageConfiguration
let configuration: PaywallData.Configuration
let colors: PaywallData.Configuration.Colors
let headerImageURL: URL
let imageURLs: [URL]

}

Expand Down
4 changes: 2 additions & 2 deletions RevenueCatUI/Data/TestData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ internal enum TestData {
template: .example1,
config: .init(
packages: [.monthly],
headerImageName: Self.paywallHeaderImageName,
imageNames: [Self.paywallHeaderImageName],
colors: .init(light: Self.lightColors, dark: Self.darkColors)
),
localization: Self.localization,
Expand All @@ -76,7 +76,7 @@ internal enum TestData {
template: .example1,
config: .init(
packages: [.annual],
headerImageName: Self.paywallHeaderImageName,
imageNames: [Self.paywallHeaderImageName],
colors: .init(light: Self.lightColors, dark: Self.darkColors)
),
localization: Self.localization,
Expand Down
57 changes: 32 additions & 25 deletions RevenueCatUI/Templates/Example1Template.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,31 +52,8 @@ private struct Example1TemplateContent: View {
VStack {
ScrollView(.vertical) {
VStack {
AsyncImage(
url: self.configuration.headerImageURL,
transaction: .init(animation: Constants.defaultAnimation)
) { phase in
if let image = phase.image {
image
.fitToAspect(Self.imageAspectRatio, contentMode: .fill)
} else if let error = phase.error {
DebugErrorView("Error loading image from '\(self.configuration.headerImageURL)': \(error)",
releaseBehavior: .emptyView)
} else {
Rectangle()
.hidden()
}
}
.frame(maxWidth: .infinity)
.aspectRatio(Self.imageAspectRatio, contentMode: .fit)
.clipShape(
Circle()
.offset(y: -140)
.scale(3.0)
)
.padding(.bottom)

Spacer()
self.headerImage
.padding(.bottom)

Group {
Text(verbatim: self.localization.title)
Expand Down Expand Up @@ -106,6 +83,36 @@ private struct Example1TemplateContent: View {
.background(self.configuration.colors.backgroundColor)
}

@ViewBuilder
private var headerImage: some View {
if let headerImage = self.configuration.imageURLs.first {
AsyncImage(
url: headerImage,
transaction: .init(animation: Constants.defaultAnimation)
) { phase in
if let image = phase.image {
image
.fitToAspect(Self.imageAspectRatio, contentMode: .fill)
} else if let error = phase.error {
DebugErrorView("Error loading image from '\(headerImage)': \(error)",
releaseBehavior: .emptyView)
} else {
Rectangle()
.hidden()
}
}
.frame(maxWidth: .infinity)
.aspectRatio(Self.imageAspectRatio, contentMode: .fit)
.clipShape(
Circle()
.offset(y: -140)
.scale(3.0)
)

Spacer()
}
}

private var offerDetails: some View {
let detailsWithIntroOffer = self.localization.offerDetailsWithIntroOffer

Expand Down
2 changes: 1 addition & 1 deletion RevenueCatUI/Templates/TemplateViewType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ extension PaywallData {
setting: self.template.packageSetting),
configuration: self.config,
colors: self.config.colors.multiScheme,
headerImageURL: self.headerImageURL
imageURLs: self.imageURLs
)
}
}
Expand Down
22 changes: 15 additions & 7 deletions Sources/Paywalls/PaywallData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -137,19 +137,27 @@ extension PaywallData {
/// The list of package types this paywall will display
public var packages: [PackageType]

/// The name for the header image asset.
public var headerImageName: String
/// The names for image assets.
public var imageNames: [String] {
get { self._imageNames }
set { self._imageNames = newValue }
}

/// The set of colors used
public var colors: ColorInformation

// swiftlint:disable:next missing_docs
public init(packages: [PackageType], headerImageName: String, colors: ColorInformation) {
public init(packages: [PackageType], imageNames: [String], colors: ColorInformation) {
assert(!imageNames.isEmpty)

self.packages = packages
self.headerImageName = headerImageName
self._imageNames = imageNames
self.colors = colors
}

@EnsureNonEmptyArrayDecodable
var _imageNames: [String]

}

}
Expand Down Expand Up @@ -208,8 +216,8 @@ extension PaywallData.Configuration {
public extension PaywallData {

/// The remote URL to load the header image asset.
var headerImageURL: URL {
return self.assetBaseURL.appendingPathComponent(self.config.headerImageName)
var imageURLs: [URL] {
self.config.imageNames.map { self.assetBaseURL.appendingPathComponent($0) }
}

}
Expand Down Expand Up @@ -274,7 +282,7 @@ extension PaywallData.Configuration: Codable {

private enum CodingKeys: String, CodingKey {
case packages
case headerImageName = "headerImage"
case _imageNames = "images"
case colors
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ func checkPaywallData(_ data: PaywallData) {

func checkPaywallConfiguration(_ config: PaywallData.Configuration,
_ colors: PaywallData.Configuration.ColorInformation) {
let _: PaywallData.Configuration = .init(packages: [.monthly, .annual], headerImageName: "", colors: colors)
let _: PaywallData.Configuration = .init(packages: [.monthly, .annual], imageNames: [""], colors: colors)
let _: [PackageType] = config.packages
let _: String = config.headerImageName
let _: [String] = config.imageNames
}

func checkPaywallLocalizedConfig(_ config: PaywallData.LocalizedConfiguration) {
Expand Down
2 changes: 1 addition & 1 deletion Tests/RevenueCatUITests/Helpers/DataExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ extension PaywallData {
var withLocalImage: Self {
var copy = self
copy.assetBaseURL = URL(fileURLWithPath: Bundle.module.bundlePath)
copy.config.headerImageName = "image.png"
copy.config.imageNames = ["image.png"]

return copy
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"default_locale": "en_US",
"config": {
"packages": ["$rc_monthly", "$rc_annual"],
"header_image": "asset_name",
"images": ["asset_1", "asset_2"],
"colors": {
"light": {
"background": "#FF00AA",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"default_locale": "en_US",
"config": {
"packages": ["$rc_monthly", "$rc_annual"],
"header_image": "asset_name.png",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@guido732 @lburdock as discussed in the doc, this is now an array. The current template only supports 1 but allows us to display multiple images in future templates.
I've added validation when decoding it to ensure that this is not empty. It would be nice if the frontend did the same to make sure we never save a template without an image.

"images": ["asset_name.png"],
"colors": {
"light": {
"background": "#FF00AA",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"template_name": "sample_1",
"localized_strings": {
"en_US": {
"title": "Paywall",
"subtitle": "Description",
"call_to_action": "Purchase now",
"call_to_action_with_intro_offer": "Purchase now",
"offer_details": "{{ price_per_month }} per month",
"offer_details_with_intro_offer": "Start your {{ intro_duration }} trial, then {{ price_per_month }} per month"
}
},
"default_locale": "en_US",
"config": {
"packages": ["$rc_monthly"],
"images": [],
"colors": {
"light": {
"background": "#FF00AA",
"foreground": "#FF00AA22",
"call_to_action_background": "#FF00AACC",
"call_to_action_foreground": "#FF00AA"
},
"dark": null
}
},
"asset_base_url": "https://rc-paywalls.s3.amazonaws.com"
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"default_locale": "es_ES",
"config": {
"packages": ["$rc_monthly", "$rc_annual"],
"header_image": "asset_name",
"images": ["asset_name"],
"colors": {
"light": {
"background": "#FFFFFF",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"default_locale": "es_ES",
"config": {
"packages": ["$rc_monthly", "$rc_annual"],
"header_image": "asset_name",
"images": ["asset_name"],
"colors": {
"light": {
"background": "#FFFFFF",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class OfferingsDecodingTests: BaseHTTPResponseTest {
try expect(paywall.assetBaseURL) == XCTUnwrap(URL(string: "https://rc-paywalls.s3.amazonaws.com"))

expect(paywall.config.packages) == [.monthly, .annual]
expect(paywall.config.headerImageName) == "asset_name"
expect(paywall.config.imageNames) == ["asset_1", "asset_2"]

let enConfig = try XCTUnwrap(paywall.config(for: Locale(identifier: "en_US")))
expect(enConfig.title) == "Paywall"
Expand Down
12 changes: 10 additions & 2 deletions Tests/UnitTests/Paywalls/PaywallDataTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class PaywallDataTests: BaseHTTPResponseTest {
expect(paywall.defaultLocale) == Locale(identifier: Self.defaultLocale)
expect(paywall.assetBaseURL) == URL(string: "https://rc-paywalls.s3.amazonaws.com")!
expect(paywall.config.packages) == [.monthly, .annual]
expect(paywall.config.headerImageName) == "asset_name.png"
expect(paywall.config.imageNames) == ["asset_name.png"]

expect(paywall.config.colors.light.background.stringRepresentation) == "#FF00AA"
expect(paywall.config.colors.light.foreground.stringRepresentation) == "#FF00AA22"
Expand All @@ -45,7 +45,9 @@ class PaywallDataTests: BaseHTTPResponseTest {
expect(paywall.config.colors.dark?.callToActionBackground.stringRepresentation) == "#112233AA"
expect(paywall.config.colors.dark?.callToActionForeground.stringRepresentation) == "#AABBCC"

expect(paywall.headerImageURL) == URL(string: "https://rc-paywalls.s3.amazonaws.com/asset_name.png")!
expect(paywall.imageURLs) == [
URL(string: "https://rc-paywalls.s3.amazonaws.com/asset_name.png")!
]

let enConfig = try XCTUnwrap(paywall.config(for: Locale(identifier: "en_US")))
expect(enConfig.title) == "Paywall"
Expand Down Expand Up @@ -79,6 +81,12 @@ class PaywallDataTests: BaseHTTPResponseTest {
expect(localization.title) == "Tienda"
}

func testFailsToDecodeWithNoImages() throws {
expect {
let _: PaywallData = try self.decodeFixture("PaywallData-empty_images")
}.to(throwError(EnsureNonEmptyArrayDecodable<String>.Error()))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm this should probably be a more precise error instead.

}

#if !os(watchOS)
func testMissingCurrentAndDefaultFails() throws {
let paywall: PaywallData = try self.decodeFixture("PaywallData-missing_current_and_default_locale")
Expand Down