Kaaveh / ComposeNews

A playground about best practices, using updated libraries and solutions in the Android world!
Apache License 2.0
287 stars 49 forks source link

[refactor] Use `DevicesPreviews` annotation #166

Closed mhmd-android closed 10 months ago

mhmd-android commented 10 months ago

Removed individual @Preview functions for different device types and configurations and used the @DevicesPreviews function

123 close

Kaaveh commented 10 months ago

Great job, @mhmd-android! Could you please provide a screenshot of the new previews in Android Studio?

mhmd-android commented 10 months ago

@Kaaveh, Yes sure, but I don't know Why my Android Studio doesn't show the preview, this problem is only in this preview Anyway, previews are like this:

2023-10-29_133017

Kaaveh commented 10 months ago

@mhmd-android I faced this issue before, so I made it a GitHub issue. Can you investigate the root cause?

mhmd-android commented 10 months ago

@Kaaveh, As long as I remember, I had this problem and I didn't understand what the problem is, but if you want to merge this PR and after that make it new an issue, if I succeed, I will definitely solve it.

Kaaveh commented 10 months ago

I reviewed the code and noticed that WindowsSizeClass requires the screen size as a parameter. Can you modify it to retrieve the screen size automatically?

Screenshot 2023-10-29 at 5 23 58 PM

mhmd-android commented 10 months ago

I think it cannot be handled with just DevicesPreviews annotation because we have different previews, each of which receives a different width and height, so, do you have any opinions?

Kaaveh commented 10 months ago

Unfortunately, no. However, I think Jetpack Compose has made it easier to preview multiple screens in production. @mhmd-android

mhmd-android commented 10 months ago

@Kaaveh I think this is why we used several previews in the previous code because the value of WindowSizeClassis different in each preview. That DevicesPreviews works when we don't have such a dependency

Kaaveh commented 10 months ago

Right, so let's close this PR. Thanks for your time ✌🏻

mhmd-android commented 10 months ago

Thanks, dear @Kaaveh, And as the last issue, I should point out that we can't use code generation?

Kaaveh commented 10 months ago

Thanks, dear @Kaaveh, And as the last issue, I should point out that we can't use code generation?

What do you mean exactly?

mhmd-android commented 10 months ago

For example, let's write code generation (of course, I have to point out that I have not done this so far) that the annotation takes two inputs as width and height, and then the code is generated for us in the body of ComposeNewsAppPreview as an annotation processor. And then for each size, we can do it like this. In fact, DevicesPreviews turns into an annotation processor.

Something like this code:

@AutoService(Processor::class)
@SupportedSourceVersion(SourceVersion.RELEASE_8)
class DevicesPreviewsProcessor : AbstractProcessor() {
    override fun getSupportedAnnotationTypes(): MutableSet<String> {
        return mutableSetOf(DevicesPreviews::class.java.name)
    }

    override fun process(annotations: MutableSet<out TypeElement>?, roundEnv: RoundEnvironment?): Boolean {
        val generatedCode = StringBuilder()

        roundEnv?.getElementsAnnotatedWith(DevicesPreviews::class.java)?.forEach { element ->
            val annotation = element.getAnnotation(DevicesPreviews::class.java)
            val width = annotation.width
            val height = annotation.height

            generatedCode.appendLine(
                """
                @Composable
                fun ${element.simpleName}() {
                    ComposeNewsTheme {
                        ComposeNewsApp(
                            windowSize = WindowSizeClass.calculateFromSize(DpSize($width.dp, $height.dp)),
                            displayFeatures = persistentListOf(),
                            uiState = MainContract.State(),
                            closeDetailScreen = {},
                        )
                    }
                }
                """
            )
        }
        return true
    }
}

And finally, be used in this way:

@DevicesPreviews(width = 400, height = 900)
@Composable
fun ComposeNewsAppPreview() {
    Generated.ComposeNewsAppPreview()
}

Of course, the problem here is that this annotation processor only produces this part of the code:

               @Composable
                fun ${element.simpleName}() {
                    ComposeNewsTheme {
                        ComposeNewsApp(
                            windowSize = WindowSizeClass.calculateFromSize(DpSize($width.dp, $height.dp)),
                            displayFeatures = persistentListOf(),
                            uiState = MainContract.State(),
                            closeDetailScreen = {},
                        )
                    }
                }

and I think we cannot use it anywhere else.

Anyway, I said, keep this in mind, if you think this is a suitable solution, it should be researched and finally used.

Kaaveh commented 10 months ago

Great idea! Would you like to work on this in the future? There is no rush.

mhmd-android commented 10 months ago

Yes, I would, of course

Kaaveh commented 10 months ago

Yes, I would, of course

Could you please comment on #169 to assign it to you?

mhmd-android commented 10 months ago

Yes, I would, of course

Could you please comment on #169 to assign it to you?

Done 🙌