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

All compose screenshots now have transparent background #598

Closed
pedromfmachado opened this issue Dec 9, 2024 · 25 comments
Closed

All compose screenshots now have transparent background #598

pedromfmachado opened this issue Dec 9, 2024 · 25 comments

Comments

@pedromfmachado
Copy link
Contributor

Hey there 👋

In one of the latest releases, 1.35.0 to be precise, the introduction of this line seems to override the application theme.

+    setTheme(android.R.style.Theme_Translucent_NoTitleBar_Fullscreen)

This makes most of our tests fail because they're now transparent instead of following the colors defined by the theme applied in our application class. I've created a workaround to manually apply the desired theme to all our Roborazzi screenshot tests, but would definitely like to move on to a more robust solution.

Is this intended behaviour? I'd be glad to contribute with a fix if you have any idea how to better deal with it! 🙂

@takahirom
Copy link
Owner

@pedromfmachado
Thank you for reporting this. I think we should have mentioned this in the release notes. The new RoborazziComposeOptions might be helpful. Could you try using the background option in RoborazziComposeOptions?

@OptIn(ExperimentalRoborazziApi::class)
@Test
fun captureComposeLambdaImage() {
    captureRoboImage(
        roborazziComposeOptions = RoborazziComposeOptions {
            background(
                showBackground = true
            )
        }
    ) {
        Text("Hello Compose!")
    }
}

@takahirom
Copy link
Owner

I've added this to the release notes.
https://github.com/takahirom/roborazzi/releases/tag/1.35.0

@pedromfmachado
Copy link
Contributor Author

pedromfmachado commented Dec 10, 2024

Thanks for the suggestion, @takahirom! In this case, because we're not defining the background color within RoborazziComposeOptions it defaults to white.

Would it make sense to allow a different activity class to be passed instead of forcing the usage of RoborazziTransparentActivity?

@pedromfmachado
Copy link
Contributor Author

pedromfmachado commented Dec 10, 2024

Just FYI and if someone is facing the same problem, we decided to use

composeRule.setContent(composable)
composeRule.onRoot().captureRoboImage(...)

rather than captureRoboImage { composable() } as this allows for the flexibility of using our own activity and theme.

@takahirom
Copy link
Owner

I think it would be good to provide a RoborazziComposeActivityThemeOption. Do you have time to add this?

@pedromfmachado
Copy link
Contributor Author

I'll find some time to create a PR soon, sure!

@mattinger
Copy link

mattinger commented Dec 11, 2024

@takahirom I'm assuming this should probably be made part of the rule?

We are having a somewhat related problem in that when we use this type of test after upgrading AGP to 8.7.3 (this seems to apply to roborazzi 1.34+ haven't tested further back)

runComposeTest { 
   setContent {
      ...
   }
   onRoot().captureRoboImage()
}

We're now getting the built in action bar on ComponentActivity, and for some reason that is overlayed over our content. I'm assuming this would also apply to people using createComposeRule as well, since that's essentially the same thing as what runComposeTest does.

I found a simple workaround is to create an AndroidManifest.xml in your src/test folder set the theme on application object.

<application android:theme="@style/Theme.AppCompat.Light.NoActionBar" />

Another option is to change the activity altogether by using the runAndroidComposeUiTest function which lets you specify which activity you want to use (runComposeTest sets that to ComponentActivity)

@takahirom
Copy link
Owner

Thank you for letting me know. It seems that SDK 35 doesn't provide padding for the ActionBar, which is causing the overlapping issue.

The same problem is occurring in NIA. I found that we need to adjust the theme to resolve it. However, I didn’t know that this could be done by placing the AndroidManifest in the test/ directory, which is very interesting.

android/nowinandroid#1719

@takahirom
Copy link
Owner

I have filed the issue.
https://issuetracker.google.com/issues/383368165

@takahirom
Copy link
Owner

I'm considering adding this kind of workaround to Roborazzi.


image

@takahirom
Copy link
Owner

I think it is necessary to use window rendering to display shadows. However, I'm not sure about Compose and the current situation.

@takahirom
Copy link
Owner

I think this workaround doesn't work because the View is a window here, and the window has the action bar.

@takahirom
Copy link
Owner

I think we can show warning or something at least.

@mattinger
Copy link

Thanks @takahirom . Given that this is an sdk 35 problem, that opens up other alternatives, such as adding the following to the robolectric.properties file

sdk=34

or doing that in the @config:

@Config(sdk=[34])

@takahirom
Copy link
Owner

Thank you for providing alternatives.

I'm considering adding this workaround. However, using the view's draw method doesn't capture the background, resulting in a transparent background. Despite this, it might be a better experience than having the action bar overlap issue.
@mattinger Could you take a look this code?
#604

@sergio-sastre
Copy link
Contributor

sergio-sastre commented Dec 12, 2024

What if you add the proposed theme

android:theme="@style/Theme.AppCompat.Light.NoActionBar"

to the theme of the RoborazziTransparentActivity?

Or there is a problem with the manifest merging?

@sergio-sastre
Copy link
Contributor

Or alternatively in RoborazziTransparentActivity:

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);

    // Request features before setting the content view
    Window windowToUse = getWindow();
    windowToUse.requestFeature(Window.FEATURE_NO_TITLE);

    setContentView(R.layout.activity_main);
}

@takahirom
Copy link
Owner

Thanks. I think it is better to have results similar to Android Studio's Composable Preview. In Composable Preview, if there is no background, the preview does not include a background. Therefore, I think it would be better to have a transparent color as the default.
image

@sergio-sastre
Copy link
Contributor

sergio-sastre commented Dec 13, 2024

Thanks. I think it is better to have results similar to Android Studio's Composable Preview. In Composable Preview, if there is no background, the preview does not include a background. Therefore, I think it would be better to have a transparent color as the default. image

I agree… However, I’m skeptical about implementing a workaround that does not fully work because you have to give up on some other functionality (i.e. shadows)

better to experiment a bit and see how we could make it work properly.

there are several options to try here… like allowing to pass a custom Activity or using ContextWrapper to apply a theme and likely some others that are worth trying

@takahirom
Copy link
Owner

I agree… However, I’m skeptical about implementing a workaround that does not fully work because you have to give up on some other functionality (i.e. shadows)
better to experiment a bit and see how we could make it work properly.
there are several options to try here… like allowing to pass a custom Activity or using ContextWrapper to apply a theme and likely some others that are worth trying

@sergio-sastre
Thanks. I tried to see if the shadow rendering is appropriate. And I can't find the area we need to fix. Could you elaborate on where we need to improve?

image

@sergio-sastre
Copy link
Contributor

Thank you for providing alternatives.

I'm considering adding this workaround. However, using the view's draw method doesn't capture the background, resulting in a transparent background. Despite this, it might be a better experience than having the action bar overlap issue. @mattinger Could you take a look this code? #604

I meant the workaround in this PR. I have currently no computer where I can check it though, but I see on my phone that the new screenshot has no shadow?

@takahirom
Copy link
Owner

Ah, I see what you mean. I have a reason for using this approach, but I’ll think about it a bit more and get back to you tomorrow.

@takahirom
Copy link
Owner

@pedromfmachado I apologize for the confusion. I believe the RoborazziComposeActivityThemeOption is still necessary for captureRoboImage{} use cases. This is distinct from the composeRule.onRoot().captureRoboImage() use cases we discussed previously, which have already been addressed.

@pedromfmachado
Copy link
Contributor Author

Hey @takahirom, thanks for clarifying, I was not on-par with the discussion yet. 🙂 I was not able to do it so far, but will create a PR in the next couple of days.

@pedromfmachado
Copy link
Contributor Author

Thanks for the support, @takahirom, I think we can close this now!

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

No branches or pull requests

4 participants