Add support for heightDp, widthDp, showBackground, backgroundColor #576
Conversation
roborazzi-compose/src/main/java/com/github/takahirom/roborazzi/RoborazziComposePreview.kt
Outdated
Show resolved
Hide resolved
roborazzi-compose/src/main/java/com/github/takahirom/roborazzi/RoborazziComposePreview.kt
Outdated
Show resolved
Hide resolved
roborazzi-compose/src/main/java/com/github/takahirom/roborazzi/RoborazziComposePreview.kt
Outdated
Show resolved
Hide resolved
| ComponentActivity::class.java.name, | ||
| ) | ||
| ) | ||
| } catch (e: ClassNotFoundException) { |
There was a problem hiding this comment.
I'm curious about this.
You are directly calling both robolectric and ComponentActivity. Wouldn't it fail with an error (NoClassDefFoundError , or similar) rather than a ClassNotFoundException? I normally assume ClassNotFoundException is for reflection usage with a checked exception.
There was a problem hiding this comment.
I agree and haven’t realised that since I copy pasted it from the analogue method in RoborazziCompose.kt file.
worth checking 👍
roborazzi-compose/src/main/java/com/github/takahirom/roborazzi/RoborazziComposePreview.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@takahirom
Feel free to review in detail. It's likely you'd like to do something differently to allow users of the api to set the background and dimensions via the PreviewTester?
roborazzi/src/main/java/com/github/takahirom/roborazzi/RoborazziComposePreviewsActivity.kt
Outdated
Show resolved
Hide resolved
| captureRoboImage( | ||
| filePath = filePath, | ||
| roborazziOptions = roborazziOptions, | ||
| content = { activityScenario -> |
There was a problem hiding this comment.
I think it was practical to add such a captureRoboImage method to allow users to set properties through the activityScenario. Open to other opinions
There was a problem hiding this comment.
Thank you. Designing these APIs is a bit challenging. I'm giving a talk at a technology conference this Saturday, so I'm a little busy at the moment. I apologize for the delay, but I’d like to take some time over the weekend to consider this. I’ll get back to you soon.
|
I’m a bit unsure why the builds fail? |
roborazzi-compose/src/main/java/com/github/takahirom/roborazzi/RoborazziCompose.kt
Outdated
Show resolved
Hide resolved
|
I noticed that there was a shadow at the top in the comparison images, but it has now been removed. It looks great! I’ll review the changes tomorrow. |
| import org.robolectric.shadows.ShadowDisplay.getDefaultDisplay | ||
| import kotlin.math.roundToInt | ||
|
|
||
| fun ActivityScenario<out Activity>.setBackgroundColor( |
There was a problem hiding this comment.
These 2 Methods together with the captureRoboImageWithActivityScenarioSetup() would allow Roborazzi users to set the background and size of their Composables under tests also for non-preview tests.
maybe worth documenting a bit more about these options to arise awareness?
| val activityScenario = ActivityScenario.launch(RoborazziTransparentActivity::class.java) | ||
|
|
||
| activityScenario.use { | ||
| val sizedPreview = content(activityScenario) |
There was a problem hiding this comment.
better to name it "preview" instead of "sizedPreview". That is a leftover of the clean up...
| widthDp: Int, | ||
| heightDp: Int | ||
| ) { | ||
| if (widthDp <= 0 && heightDp <= 0) return |
There was a problem hiding this comment.
maybe desirable to log a warning or debug message if it returns here?
| } | ||
|
|
||
| @Preview( | ||
| name = "Preview width & height", |
There was a problem hiding this comment.
"Preview width & height small"?
|
@sergio-sastre |
I’ll do it either today or tomorrow when I find time😊 |
This addresses #559
Still very open for discussion