Adds ThemedColorProvider to resources module.

[posytive]

Draft for #336

Introduces the concept of a color that is aware of Dark/Light mode and it gives the proper value automatically.

Advantages:

• the actual colors are not constantly re-created from the hex values, but they are chaced
• on iOS side, colorWithDynamicProvider is used, which brings this functionality automatically
• on Android side, the color value will not be cached but rather provided every time depending on the theme
• if, for any reason, access to each theme value is still needed it remains possible by calling Provider.darkColor or Provider.lightColor

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.54%. Comparing base (54c37f7) to head (4539f68).
Report is 2890 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #277   +/-   ##
=========================================
  Coverage     49.54%   49.54%           
  Complexity      199      199           
=========================================
  Files            52       52           
  Lines          1314     1314           
  Branches        256      256           
=========================================
  Hits            651      651           
  Misses          530      530           
  Partials        133      133           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[abelthefirst]

I think this API might be improved a bit, if color property type would be Flow. That would explicitly indicate its dynamic nature.

[posytive]

I see your point, and this feature can be implemented with a Flow.
But, thinking about the use case, I am not sure that is the best way to go.
This provider is meant to unify and simplify the usage of cross-platform color resources, when it comes to Dark/Light mode. Which is quite different in the way that each color on iOS has 2 versions, but not on Android (where a set of colors has 2 versions, not the color itself)
iOS has no need for a Flow, cause the changing of colors on UI is something that happens by using colorWithDynamicProvider . I don't see it as something you'd need to collect multiple times.

Also on the android side, I don't see a use case where is important to observe this color change, if not to trigger recomposition.
But even then, is not the change of color that should be observed but rather the Theme change.
So simplicity is preferred in this case.
In some projects using Kaluga, the common color resource file is currently made of getters or lazy initializations.

FYI:
With this idea, on Compose, there is no need to implement Material Colors: is currently intended for colors that we can use directly from a definition in common code. (a Compose Color can be easily created from a Kaluga Color).
Maybe it matters more to indicate the dynamic nature of these assets when referenced on the ViewModels.

[abelthefirst]

But even then, is not the change of color that should be observed but rather the Theme change.

I absolutely agree, but I don't see a shared implementation of Flow<Theme> either. Moreover, in Compose UI it won't really work: the re-composition will indeed be triggered, but due to the skipping if the inputs haven't changed, it might not affect the places, where colors are being read (it really depends on the implementation).

[corrado4eyes]

I would rename these 2 properties just light and dark, is already known they refer to color. What do you think?

[posytive]

thought about it, but light and dark are also the name of the params which are strings.
I preferred not to rename those to lightHexValue or ligthValue, but to rename these instead.
Because I prefer to call it with val somename by themeColor(light = "#fff", dark = "#000"): very simple
Also, the third and more important variable is called color, so darkColor and lightColor seem consistent to me.

Good point, though

[Daeda88]

There is an isInDarkMode method on the kaluga level. So I dont see the point of making this platform specific. It should just be:

fun themeColor(light: Color, dark: Color): Color {
if (isInDarkMode) dark else light
}

[Daeda88]

Also, if you want the dynamic behaviour on Android on a named level, you can just make a values_night folder and put the dark color there.

[avdyushin]

The main reason that simple if statement doesn't reflect change in real time on iOS (including SwiftUI Previews) so UIColor should be created on other way using special constructor to have dynamic colors.

[thoutbeckers]

No, look at the implementation. iOS provides a color that is both light and dark, whereas this does not exist on Android. See https://splendo.slack.com/archives/CN1T4SM5Y/p1618294377023400

[avdyushin]

Actually good point regarding values-night worth to try on Android and just returning same color name to be handled by Android itself?

[thoutbeckers]

putting it in values-night defies the point of what this method tries to do, defining colors once in common code.

[posytive]

Actually good point regarding values-night worth to try on Android and just returning same color name to be handled by Android itself?

Yes, this is worth looking into and too bad I didn't think about it in the first place.
I was doubting that values-night is the folder used in case of dark mode (earlier versions on Android would use this folder for other modes), but indeed it is.

I will make some more experiments when I have time

@Tijl I agree that the preferred usage of this class is indeed by using the strings directly.
Although the problem is trying to solve is also to unify the usage of colors on platform without breaking the dark/light switch.
In that sense, I think is worth looking into the values-night solution while still using these ColorProviders, we would only need to add 1 extension to accepts colors directly. It will still be valuable the fact that we define the difference in the constructor.
I think this is useful for project where the best/only solution to branded colors is with scripts that generates the XML files in the correct places.
In general, we should consider how to support projects that already have their color definitions in xml and don't need to change that, while they do need to add support fro dark/light easily
(imho)

[Tijl]

Can you stop tagging me please

[Daeda88]

This doesnt look like it solves anything really. And if it does, it does so in an unclear way.

[posytive]

This doesnt look like it solves anything really. And if it does, it does so in an unclear way.

Fair point, this could be more complete to be used in an easier way, especially on Android.
But I did change the README, and tried to explain there, the best I could, what this is for. (Dark/Light switch).
So please let me know what is not clear about that.
Otherwise my answer is This comment is not helpful really. And if it does, it does so in an unclear way. :)

Truth be told, the way I am currently using it in a project, does include a piece of glue that makes it more usable on android:

@Composable
fun ThemedColorProvider.composeThemedColor(): Color
    = if (isSystemInDarkTheme()) Color(darkColor) else Color(lightColor)

So, when I need an automatically recomposing color on Android, I'd go Colors.backgroundProvider.composeThemedColor() INSTEAD of using MaterialTheme.

Even better if I can directly use Compose's isSystemInDarkTheme() instead of Kaluga's isInDarkMode in the actual class on this PR.
I think it will look more complete then, but I need to change the target branch and consider this an RC feature.
I'll do that as soon as I have time for it

[thoutbeckers]

@posytive this still has no linked issue in the description