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 V2] Updating UIConfig aliased colors to contain both light and dark #4650

Merged
merged 6 commits into from
Jan 13, 2025

Conversation

joshdholtz
Copy link
Member

@joshdholtz joshdholtz commented Jan 11, 2025

Motivation

Aliased color will be both light and dark

Description

This ended up being a whole thing...

TL;DR - Added a new DisplayableColorScheme object that has types of hex, linear, and radial...

PaywallComponent.ColorScheme -> DisplayableColorScheme->Color/Gradient`

Issue

We have .alias as a value on ColorInfo but we need to alias an entire set of light/dark colors (aka ColorScheme).

Our logic right now directly converts ColorInfo into a SwiftUI/UI color (for foreground, background, etc). This works fine for hex, linear, and radial but will not work well for alias. We need to perform the alias lookup at the ColorScheme level (near the view model layer instead of at the render layer)

Solution

Create a new DisplayableColorScheme and DisplayableColorInfo struct. These new data structures only have values for hex, linear, and radial.

ColorScheme (and ColorInfo) get mapped to DisplayableColorScheme (and DisplayableColorInfo) and in this mapping is where the alias look up happens. This makes it safer/cleaner at the render level because everything has a specific color now.

The converting to DisplayableColorScheme currently will fallback to a clear color if there is an error and log an error. We will EVENTUALLY AND SOON (in a follow up PR) add this validation that the alias exists in the creation of the view models so that we can error out early if needed.

NOTE: We will also add big backend validation that the alias colors exist so the SDKs will never (🤞) have to encounter an alias color not existing.

@joshdholtz joshdholtz marked this pull request as ready for review January 12, 2025 00:10
@joshdholtz joshdholtz requested review from a team January 12, 2025 00:10
Copy link
Member

@JayShortway JayShortway left a comment

Choose a reason for hiding this comment

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

This makes a lot of sense! :shipit:

Comment on lines +121 to +126
// Throwing error if alias has an alias
// This should NEVER happen though
case .alias(let name):
Logger.error("Aliased color '\(name)' has an aliased value which is not allowed.")
throw PaywallColorError.aliasedColorIsAliased(name)
}
Copy link
Member

Choose a reason for hiding this comment

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

Famous last words 😅
This will move to the ViewModel validation/initialization too, right?

@joshdholtz joshdholtz force-pushed the paywalls-v2/ui-config-colors-light-and-dark branch from cb94a09 to 5c70b03 Compare January 13, 2025 11:43
@joshdholtz joshdholtz merged commit 652aa67 into main Jan 13, 2025
10 checks passed
@joshdholtz joshdholtz deleted the paywalls-v2/ui-config-colors-light-and-dark branch January 13, 2025 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants