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

Paparazzi Snapshot tests failing between 0.4.1 -> 0.4.2 #87

Closed
wezley98 opened this issue Jan 9, 2024 · 5 comments · Fixed by #91
Closed

Paparazzi Snapshot tests failing between 0.4.1 -> 0.4.2 #87

wezley98 opened this issue Jan 9, 2024 · 5 comments · Fixed by #91

Comments

@wezley98
Copy link

wezley98 commented Jan 9, 2024

As title, we integrated haze into our project at 0.4.1, but any version above 0.4.2/0.4.3/0.4.4 doesn't show the haze in our snapshots anymore.

0.4.4 Delta

delta-com sky sport group snapshots components_CarouselListComponentSnapshotTest_testCarouselListComponent NEXUS_5,1 0,10 Tile(layout=Hero, images=Images(sixt ,Light

Example Usage

Box(modifier = Modifier.clip(RoundedCornerShape(8.dp))) {
            val imageSizeModifier = Modifier
                .aspectRatio(ratio = AspectRatio.FOUR_BY_THREE.ratio)

            val hazeState = remember { HazeState() }

            UrlImage(
                modifier = imageSizeModifier.then(
                    if (tile.containsVideo) {
                        Modifier.haze(
                            state = hazeState,
                            backgroundColor = SkyTheme.skyColors.black.copy(alpha = 0.2f),
                            tint = SkyTheme.skyColors.black.copy(alpha = 0.2f)
                        )
                    } else {
                        Modifier
                    }
                ),
                url = tile.images.fourByThree ?: tile.images.sixteenByNine,
                mode = AspectRatio.FOUR_BY_THREE,
                imageCropsConfig = imageCropsConfig
            )

            if (tile.containsVideo) {
                TileVideoContentTimestampComponent(
                    tile = tile,
                    modifier = Modifier
                        .padding(start = 8.dp, bottom = 8.dp)
                        .hazeChild(state = hazeState, shape = RoundedCornerShape(2.dp))
                        .align(Alignment.BottomStart),
                    carouselTile = false
                )
                if (tile.contentLocked) {
                    LockedComponent(
                        contentTitle = tile.title,
                        modifier = imageSizeModifier
                    )
                }
            }
        }
@wezley98 wezley98 changed the title Paparazzi Snapshot tests failing between 0.4.2 -> 0.4.3 Paparazzi Snapshot tests failing between 0.4.1 -> 0.4.2 Jan 9, 2024
@wezley98
Copy link
Author

wezley98 commented Jan 9, 2024

Updated version, we are on 0.4.1

@chrisbanes
Copy link
Owner

chrisbanes commented Jan 9, 2024

So this is likely because of #72 and #70 which where merged to actually get Haze working in previews and screenshot tests.

Looking at your screenshots, the difference is that the TileVideoContentTimestampComponent is now missing a background. Where was that background coming from? Haze?

Paparazzi uses SDK 33 iirc, so I'm surprised this didn't crash previously tbh (for the same reason as #71).

@wezley98
Copy link
Author

wezley98 commented Jan 9, 2024

@chrisbanes Originally the background was set on the TileVideoContentTimestampComponent like this:

@Composable
fun TileVideoContentTimestampComponent(
    tile: Component.Tile,
    modifier: Modifier = Modifier,
    carouselTile: Boolean
) = Row(
        modifier = modifier
            .clip(RoundedCornerShape(2.dp))
            .background(SkyTheme.skyColors.black.copy(alpha = 0.6f))
    ) {
.....
}

After adding haze we removed it and relied on haze's backgroundColor which seem to work ok.

@Composable
fun TileVideoContentTimestampComponent(
    tile: Component.Tile,
    modifier: Modifier = Modifier,
    carouselTile: Boolean
) = Row(modifier = modifier) {
    Icon(
        painter = painterResource(id = R.drawable.ic_video_play_arrow),
        contentDescription = "",
        modifier = Modifier
            .align(Alignment.CenterVertically),
        tint = SkyTheme.skyColors.white
    )

    TileVideoDuration(
        videoDuration = tile.videoDuration,
        carouselTile = carouselTile
    )
}

If haze won't appear in screenshots for now, that's ok we just need to let the team know.

@chrisbanes
Copy link
Owner

chrisbanes commented Jan 9, 2024

Sounds good. The background will be shown again as soon as #71 is fully fixed (I'm working on it this week).

I just don't know how it worked previously in Paparazzi 🤔. What version of Paparazzi are you using?

@wezley98
Copy link
Author

wezley98 commented Jan 9, 2024

Currently on latest stable 1.3.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants