Fixes for paywalls v2 renderer after testing some real life paywalls #4436
Conversation
joshdholtz
changed the title
| @@ -18,10 +18,10 @@ import Foundation | |||
|
|
|||
| public extension PaywallComponent { | |||
|
|
|||
| enum Dimension: Codable, Sendable, Hashable { | |||
| @frozen enum Dimension: Codable, Sendable, Hashable { | |||
There was a problem hiding this comment.
I must say I'm not a big fan of this @frozen annotation... While it does make sense, it seems it might cause us some issues down the line. For this enum I don't foresee new values coming anytime soon but well... In any case, if this is the recommended approach, seems we just will need to be thinking forward a lot for these APIs.
There was a problem hiding this comment.
I will just remove them for now 😇
| @@ -109,7 +109,7 @@ public extension PaywallComponent { | |||
|
|
|||
| } | |||
|
|
|||
| enum WidthSizeType: String, Codable, Sendable, Hashable, Equatable { | |||
| @frozen enum WidthSizeType: String, Codable, Sendable, Hashable, Equatable { | |||
| case fit, fill, fixed | |||
There was a problem hiding this comment.
Should we allow something like fixedPercentage?
There was a problem hiding this comment.
There has been A LOT of discussion about this 😅 We decided to leave this out (for now) but will probably introduce some type of "grid" component that has percentage like this in a more structured way
| enum CodingKeys: String, CodingKey { | ||
| case type | ||
| case packageID = "packageId" | ||
| case isSelectedByDefault = "isDefaultSelected" |
There was a problem hiding this comment.
Hmm strange these are using camelCase instead of snake_case? Same below
There was a problem hiding this comment.
Codables are a bit weird here 😅
Keys will get deserialized from snake to camel automatically... however, packageID would get deserialized as package_i_d because of the ID both being capitals 🫠
Some of the other codables are values that we actually are expecting to be snake case as well but Codable only auto converts for keys (not values)
There was a problem hiding this comment.
That's.... confusing 🥴. But yeah makes sense. Thanks for explaining!
There was a problem hiding this comment.
It honestly is 😬 Maybe I should document this is all the place? Because it will be more than just me supporting this and I don't want to make anybody else think about Swift quirks 😛
There was a problem hiding this comment.
Maybe a quick comment could help yeah. Having said that, I think as long as the decoding behavior is well tested, this should be caught with tests
joshdholtz
force-pushed
the
fixes-for-paywall-tester
branch
from
58d2144
to
1864c91
Compare
Motivation
Fixes for paywall tester for Paywalls V2
Description
@frozento all the enums to get rid of Swift 6 warningsFlexHStackto support horizontaldistributionLazyVStackLazyVStackis used to make rendering a bit snappier feeling but it has issues with width sometimesmaxWidth: .infinityso added some naive logic to only useLazyVStackif more than 3 children. Ideally this should look recursively downCodablethings that were failing when testing real life paywallsDemo