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

Refactor Add support for heightDp, widthDp, showBackground, backgroundColor #577

Merged
merged 18 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,26 @@ import sergio.sastre.composable.preview.scanner.core.preview.ComposablePreview

@ExperimentalRoborazziApi
fun ComposablePreview<AndroidPreviewInfo>.captureRoboImage(
filePath: String = DefaultFileNameGenerator.generateFilePath(),
roborazziOptions: RoborazziOptions = RoborazziOptions(),
filePath: String,
roborazziOptions: RoborazziOptions = provideRoborazziContext().options,
applierBuilder: RoborazziComposeApplierBuilder = RoborazziComposeApplierBuilder()
Copy link
Contributor

Choose a reason for hiding this comment

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

it is true that it adds some complexity, but brings the benefit of making the api more flexible.

Therefore, I have a couple of points:

  1. We are inconsistent in the way we handle fontSize, UiMode, Locale, device...etc. vs. sized and colored.
    I think it would become much more intuitive if all of them would be handled the same way, e.g.
RoborazziComposeApplierBuilder()
   .sized(
       widthDp = previewInfo.widthDp,
       heightDp = previewInfo.heightDp
     )
     // I'd rather call this background than coloured. Both properties include background inn their names indeed. One could also call them : `show` instead of `showBackground` and `color` instead of `backgroundColor`
     .background( 
       showBackground = previewInfo.showBackground,
       backgroundColor = previewInfo.backgroundColor
     )
     .uiMode(
        configurationUiMode  = previewInfo.uiMode
     )
     .locale(
        bcp47LanguageTag = previewInfo.locale
     )
     ...

I think symmetry in code also makes it more understandable

  1. The name of this variable is a bit ambiguous. I'm not sure what to expect from this as a user of this method. I think something in the direction of previewOptions and RoborazziPreviewOptionsBuilder or composableConfigand RoborazziComposableConfigBuilder is more understandable

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thank you for your review. That makes sense.
I've migrated the settings such as local to the applier and renamed the applier to config.
a899893
https://github.com/takahirom/roborazzi/pull/579/files

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me.
The only improvements I’d propose are input validations (e.g. fontScale should not be lower than 0 and the like) and logging messages for invalid inputs.

apart from that feel free to merge 😊

.sized(
widthDp = previewInfo.widthDp,
heightDp = previewInfo.heightDp
)
.colored(
showBackground = previewInfo.showBackground,
backgroundColor = previewInfo.backgroundColor
)
) {
if (!roborazziOptions.taskType.isEnabled()) return
val composablePreview = this
composablePreview.applyToRobolectricConfiguration()
captureRoboImage(filePath = filePath, roborazziOptions = roborazziOptions) {
captureRoboImage(filePath, roborazziOptions, applierBuilder) {
composablePreview()
}
}

/**
* ComposePreviewTester is an interface that allows you to define a custom test for a Composable preview.
* The class that implements this interface should have a parameterless constructor.
Expand Down
1 change: 1 addition & 0 deletions roborazzi-compose/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ dependencies {

testImplementation libs.androidx.compose.runtime
compileOnly libs.androidx.compose.ui
compileOnly libs.androidx.compose.foundation
compileOnly libs.androidx.activity.compose
compileOnly libs.robolectric
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.github.takahirom.roborazzi

import android.app.Application
import android.content.ComponentName
import androidx.test.core.app.ApplicationProvider
import org.robolectric.Shadows

/**
* Workaround for https://github.com/takahirom/roborazzi/issues/100
*/
internal fun registerActivityToRobolectricIfNeeded() {
try {
val appContext: Application = ApplicationProvider.getApplicationContext()
Shadows.shadowOf(appContext.packageManager).addActivityIfNotPresent(
ComponentName(
appContext.packageName,
RoborazziTransparentActivity::class.java.name,
)
)
} catch (e: ClassNotFoundException) {
// Configured to run even without Robolectric
e.printStackTrace()
}
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
package com.github.takahirom.roborazzi

import android.app.Application
import android.content.ComponentName
import android.view.ViewGroup
import androidx.activity.ComponentActivity
import androidx.activity.compose.setContent
import androidx.compose.runtime.Composable
import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.platform.ViewRootForTest
import androidx.test.core.app.ActivityScenario
import androidx.test.core.app.ApplicationProvider
import org.robolectric.Shadows
import java.io.File


fun captureRoboImage(
filePath: String = DefaultFileNameGenerator.generateFilePath(),
roborazziOptions: RoborazziOptions = provideRoborazziContext().options,
Expand All @@ -28,40 +26,87 @@ fun captureRoboImage(
file: File,
roborazziOptions: RoborazziOptions = provideRoborazziContext().options,
content: @Composable () -> Unit,
) {
captureRoboImage(
file = file,
roborazziOptions = roborazziOptions,
applierBuilder = RoborazziComposeApplierBuilder(),
content = content
)
}

@ExperimentalRoborazziApi
fun captureRoboImage(
filePath: String = DefaultFileNameGenerator.generateFilePath(),
roborazziOptions: RoborazziOptions = provideRoborazziContext().options,
applierBuilder: RoborazziComposeApplierBuilder = RoborazziComposeApplierBuilder(),
content: @Composable () -> Unit,
) {
captureRoboImage(
file = fileWithRecordFilePathStrategy(filePath),
roborazziOptions = roborazziOptions,
applierBuilder = applierBuilder,
content = content
)
}

@ExperimentalRoborazziApi
fun captureRoboImage(
file: File,
roborazziOptions: RoborazziOptions = provideRoborazziContext().options,
applierBuilder: RoborazziComposeApplierBuilder = RoborazziComposeApplierBuilder(),
content: @Composable () -> Unit,
) {
if (!roborazziOptions.taskType.isEnabled()) return
registerRoborazziActivityToRobolectricIfNeeded()
val activityScenario = ActivityScenario.launch(RoborazziTransparentActivity::class.java)
activityScenario.use {
activityScenario.onActivity { activity ->
activity.setContent(content = content)
val composeView = activity.window.decorView
.findViewById<ViewGroup>(android.R.id.content)
.getChildAt(0) as ComposeView
val viewRootForTest = composeView.getChildAt(0) as ViewRootForTest
viewRootForTest.view.captureRoboImage(file, roborazziOptions)
launchRoborazziTransparentActivity { activityScenario ->
val appliedContent = applierBuilder
.apply(activityScenario) {
content()
}
activityScenario.captureRoboImage(file, roborazziOptions) {
appliedContent()
}

// Closing the activity is necessary to prevent memory leaks.
// If multiple captureRoboImage calls occur in a single test,
// they can lead to an activity leak.
}
}

/**
* Workaround for https://github.com/takahirom/roborazzi/issues/100
*/
private fun registerRoborazziActivityToRobolectricIfNeeded() {
try {
val appContext: Application = ApplicationProvider.getApplicationContext()
Shadows.shadowOf(appContext.packageManager).addActivityIfNotPresent(
ComponentName(
appContext.packageName,
RoborazziTransparentActivity::class.java.name,
)
)
} catch (e: ClassNotFoundException) {
// Configured to run even without Robolectric
e.printStackTrace()
}
private fun launchRoborazziTransparentActivity(block: (ActivityScenario<RoborazziTransparentActivity>) -> Unit = {}) {
registerActivityToRobolectricIfNeeded()

val activityScenario = ActivityScenario.launch(RoborazziTransparentActivity::class.java)

// Closing the activity is necessary to prevent memory leaks.
// If multiple captureRoboImage calls occur in a single test,
// they can lead to an activity leak.
return activityScenario.use { block(activityScenario) }
}


private fun ActivityScenario<out ComponentActivity>.captureRoboImage(
filePath: String,
roborazziOptions: RoborazziOptions = provideRoborazziContext().options,
content: @Composable () -> Unit,
) {
captureRoboImage(
file = fileWithRecordFilePathStrategy(filePath),
roborazziOptions = roborazziOptions,
content = content
)
}

private fun ActivityScenario<out ComponentActivity>.captureRoboImage(
file: File,
roborazziOptions: RoborazziOptions = provideRoborazziContext().options,
content: @Composable () -> Unit,
) {

onActivity { activity ->
activity.setContent(content = { content() })

val composeView = activity.window.decorView
.findViewById<ViewGroup>(android.R.id.content)
.getChildAt(0) as ComposeView

val viewRootForTest = composeView.getChildAt(0) as ViewRootForTest
viewRootForTest.view.captureRoboImage(file, roborazziOptions)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
package com.github.takahirom.roborazzi

import android.app.Activity
import android.graphics.Color
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.width
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.dp
import androidx.test.core.app.ActivityScenario
import org.robolectric.Shadows.shadowOf
import org.robolectric.shadows.ShadowDisplay.getDefaultDisplay
import kotlin.math.roundToInt

interface RoborazziComposeApplier

@ExperimentalRoborazziApi
interface RoborazziComposeActivityScenarioApplier : RoborazziComposeApplier {
fun applyToActivityScenario(scenario: ActivityScenario<out Activity>)
}

@ExperimentalRoborazziApi
interface RoborazziComposeComposableApplier : RoborazziComposeApplier {
fun applyToComposable(content: @Composable () -> Unit): @Composable () -> Unit
}


@ExperimentalRoborazziApi
class RoborazziComposeApplierBuilder {
private val activityScenarioAppliers =
mutableListOf<RoborazziComposeActivityScenarioApplier>()
private val composableAppliers = mutableListOf<RoborazziComposeComposableApplier>()

fun with(applier: RoborazziComposeApplier): RoborazziComposeApplierBuilder {
if (applier is RoborazziComposeActivityScenarioApplier) {
activityScenarioAppliers.add(applier)
}
if (applier is RoborazziComposeComposableApplier) {
composableAppliers.add(applier)
}
return this
}

fun sized(widthDp: Int = 0, heightDp: Int = 0): RoborazziComposeApplierBuilder {
return with(RoborazziComposeSizeApplier(widthDp, heightDp))
}

fun colored(
showBackground: Boolean,
backgroundColor: Long = 0L
): RoborazziComposeApplierBuilder {
return with(RoborazziComposeBackgroundApplier(showBackground, backgroundColor))
}

@InternalRoborazziApi
fun apply(
scenario: ActivityScenario<out Activity>,
content: @Composable () -> Unit
): @Composable () -> Unit {
activityScenarioAppliers.forEach { it.applyToActivityScenario(scenario) }
var appliedContent = content
composableAppliers.forEach { applier ->
appliedContent = applier.applyToComposable(appliedContent)
}
return {
appliedContent()
}
}

@ExperimentalRoborazziApi
data class RoborazziComposeSizeApplier(val widthDp: Int, val heightDp: Int) :
RoborazziComposeActivityScenarioApplier,
RoborazziComposeComposableApplier {
override fun applyToActivityScenario(scenario: ActivityScenario<out Activity>) {
scenario.onActivity { activity ->
activity.setDisplaySize(widthDp = widthDp, heightDp = heightDp)
}
}

private fun Activity.setDisplaySize(
widthDp: Int,
heightDp: Int
) {
if (widthDp <= 0 && heightDp <= 0) return

val display = shadowOf(getDefaultDisplay())
val density = resources.displayMetrics.density
if (widthDp > 0) {
val widthPx = (widthDp * density).roundToInt()
display.setWidth(widthPx)
}
if (heightDp > 0) {
val heightPx = (heightDp * density).roundToInt()
display.setHeight(heightPx)
}
recreate()
}

override fun applyToComposable(content: @Composable () -> Unit): @Composable () -> Unit {
/**
* WARNING:
* For this to work, it requires that the Display is within the widthDp and heightDp dimensions
* You can ensure that by calling [Activity.setDisplaySize] before
*/
val modifier = when {
widthDp > 0 && heightDp > 0 -> Modifier.size(widthDp.dp, heightDp.dp)
widthDp > 0 -> Modifier.width(widthDp.dp)
heightDp > 0 -> Modifier.height(heightDp.dp)
else -> Modifier
}
return {
Box(modifier = modifier) {
content()
}
}
}
}
}

@ExperimentalRoborazziApi
data class RoborazziComposeBackgroundApplier(
val showBackground: Boolean,
val backgroundColor: Long
) : RoborazziComposeActivityScenarioApplier {
override fun applyToActivityScenario(scenario: ActivityScenario<out Activity>) {
when (showBackground) {
false -> {
scenario.onActivity { activity ->
activity.window.decorView.setBackgroundColor(Color.TRANSPARENT)
}
}

true -> {
val color = when (backgroundColor != 0L) {
true -> backgroundColor.toInt()
false -> Color.WHITE
}
scenario.onActivity { activity ->
activity.window.decorView.setBackgroundColor(color)
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package com.github.takahirom.roborazzi

import android.os.Bundle
import android.os.PersistableBundle
import androidx.activity.ComponentActivity

class RoborazziTransparentActivity: ComponentActivity() {
override fun onCreate(savedInstanceState: Bundle?, persistentState: PersistableBundle?) {
super.onCreate(savedInstanceState, persistentState)
override fun onCreate(savedInstanceState: Bundle?) {
setTheme(android.R.style.Theme_Translucent_NoTitleBar_Fullscreen)
super.onCreate(savedInstanceState)
}
}
1 change: 1 addition & 0 deletions roborazzi/src/main/res/values/themes.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<resources xmlns:tools="http://schemas.android.com/tools">
<!-- Base application theme. -->
<!-- It seems that this theme isn't used because of test resource merger issue -->
<style name="RoborazziTransparentTheme">
<item name="android:colorBackground">@android:color/transparent</item>
<item name="android:windowActionBar">false</item>
Expand Down
2 changes: 2 additions & 0 deletions sample-generate-preview-tests/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ android {
isIncludeAndroidResources = true
all {
it.systemProperties["robolectric.pixelCopyRenderMode"] = "hardware"
// For large preview
it.maxHeapSize = "4096m"
}
}
}
Expand Down
Loading
Loading