airbnb / Showkase

šŸ”¦ Showkase is an annotation-processor based Android library that helps you organize, discover, search and visualize Jetpack Compose UI elements
https://medium.com/airbnb-engineering/introducing-showkase-a-library-to-organize-discover-and-visualize-your-jetpack-compose-elements-d5c34ef01095
Apache License 2.0
2.08k stars 106 forks source link

Qualified night/dark assets not being picked up in @Preview snapshot tests #377

Open drinkthestars opened 3 months ago

drinkthestars commented 3 months ago

Given the following:

@Preview(name = "TestBox - Light")
@Composable
fun TestPreviewLight() {
    TestBox()
}

@Preview(name = "TestBox - Dark", uiMode = Configuration.UI_MODE_NIGHT_YES)
@Composable
fun TestPreviewDark() {
    TestBox()
}

@Composable
fun TestBox(
    modifier: Modifier = Modifier
) {
    Box(
        modifier = modifier.size(100.dp)
    ) {
        Image(
            imageVector = ImageVector.vectorResource(id = R.drawable.test_drawable),
            contentDescription = "Testing background",
        )
    }
}

where the R.drawable.test_drawable is a vector that references night/day qualified colors or has a drawable-night equivalent, the recorded snapshots only show the light variant - somehow the dark/night qualified assets are not being picked up:

Preview:

Screenshot 2024-04-04 at 1 53 35 AM

Golden Snapshots:

Screenshot 2024-04-04 at 1 52 45 AM

Tried the following to try to force it, but didn't work either:

@Preview...
fun TestPreview() {
    val customConfiguration = Configuration(LocalConfiguration.current).apply {
        uiMode = Configuration.UI_MODE_NIGHT_YES
    }
    CompositionLocalProvider(LocalConfiguration provides customConfiguration) {
        TestBox()
    }
}

Is this a bug/limitation or is it a config issue? Thanks!

vinaygaba commented 1 month ago

@drinkthestars Showkase isn't currently parsing uiMode from the @Preview annotation but should be possible to add. The open question from my perspective is that each component anyway gets previewed in dark mode inside the Showkase browser and I wonder if that should be the behavior even with the Paparazzi screenshots? If that is indeed the case, I'm on the fence about how that's reconciled with previews that are specifically meant to be in dark mode through the use of the uiMode property. Any thoughts?

drinkthestars commented 2 weeks ago

Continuing an offline discussion. Gave it some more thought, and this is probably a very naive approach but it looks like with this issue and https://github.com/airbnb/Showkase/issues/292, we have the following requirements:

For the opt-in mechanism: Introduce a new annotation, e.g. ShowkaseCustomConfig, which can be used in conjunction with @Preview. This annotation supports hierarchical configuration options to specify how permutations should be handled.

The following cases would be supported:

Case 1: Merely inheriting from @Preview - this mode guarantees that the @Preview settings are used as is.

@Preview(uiMode = Configuration.UI_MODE_NIGHT_YES)
@ShowkaseCustomConfig(config = ShowkaseCustomConfig.Config.InheritPreview)
@Composable
fun NightPreview() {
    // This Composable will strictly follow the NIGHT mode setup from @Preview, no additional permutations.
}

Case 2: In this scenario, specific dimensions are included or excluded, building on the base settings from @Preview.

@Preview(uiMode = Configuration.UI_MODE_NIGHT_YES)
@ShowkaseCustomConfig(config = ShowkaseCustomConfig.Config.MergePreview(
    excludeDimensions = arrayOf(Dimension.RTL)
))
@Composable
fun CustomizedPreview() {
    // Applies NIGHT mode from @Preview; includes everything except RTL.
}

Case 3: This setup specifies broad customization, impacting how permutations are generated without specific @Preview settings to override.

@Preview
@ShowkaseCustomConfig(config = ShowkaseCustomConfig.Config.MergePreview(
    excludeDimensions = arrayOf(Dimension.RTL, Dimension.FontSize)
))
@Composable
fun BroadCustomizedPreview() {
    // Generates permutations for all UI modes but excludes RTL & FontSize.
}

Handling Complex Edge Cases One example: if no @Preview args are provided but MergePreview is used, the default UI settings should be assumed, and only the specified excludeDimensions should be applied.

This assumes a lot of flexibility is required - if we assume some flexibility loss perhaps this can be whittled down further. Overall the main idea is to have some type of an opt-in customization mechanism. Sorry for the long text, hoping this makes sense somewhat šŸ˜…

vinaygaba commented 2 weeks ago

@drinkthestars For the Showkase configuration, do you think it makes sense to handle it at the root level instead of doing it for each preview? Would that work better?

drinkthestars commented 2 weeks ago

@vinaygaba hmm, maybe šŸ¤” would you have an example of what you're thinking of? trying to visualize it