Support multi-preview annotations (annotations annotated with @Preview)

[alexvanyo]

In the latest alpha releases of Compose, the @Preview annotation can now be applied to other annotation classes: https://android-review.googlesource.com/c/platform/frameworks/support/+/1964936. This will cause those other annotation classes to be treated as previews. This is most useful for bundling common sets of previews together (since the custom annotation class can be annotated with multiple @Previews), instead of having to copy around multiple groups @Preview everywhere.

Right now, the Showkase process throws an exception of "Only composable methods can be annotated with Preview" upon seeing a custom annotation annotated with @Preview.

[vinaygaba]

Thanks for flagging this! I think I should be able to skip this check when its applied to an annotation class. I believe I should have this information available and should be a relatively straightforward fix!

[oas004]

When trying to add the last step for supporting multi-preview annotations I stumbled on two issues that I would love to have a discussion on. Let's say you have a custom annotation like:

@Preview(name = "phone", device = "spec:shape=Normal,width=360,height=640,unit=dp,dpi=480")
@Preview(name = "foldable", device = "spec:shape=Normal,width=673,height=841,unit=dp,dpi=480")
@Preview(name = "tablet", device = "spec:shape=Normal,width=1280,height=800,unit=dp,dpi=480")
annotation class CustomDevicePreview

For this type of custom annotation, IMO it will not make sense to render 4 showkase previews for each time you use it on a composable. That is because they would all look the same since Showkase is running on an actual device. However, let's say you have something like this

@Preview(name="Composable in 400dp width", widthDp=400)
@Preview(name="Composable in 200dp width", widthDp=200)
annotation class CustomSizePreview

IMO, in this case it would make sense for showkase to render both permutations for each usage of the annotation. Here the showkase previews will actually differ in contrast to the first case.

Do you think we should provide a way for the developer to choose if they would like to render all permutations or only one? Wdyt? :)

The second issue I ran into was in regards to the naming of these components. Since we are using the preview names, we risk getting a lot of components with the same name. I tried fixing this by adding the element name as well. However, we would still get the same component names if the user do not provide any preview names. If I add the index to the name it looks very ugly. Do you have any input on how to solve this more elegantly? 🤔 😄 Any input is appreciated 😄

[alexvanyo]

One note is that this isn't unique to multi-preview annotations, the same questions would also apply with just multiple preview annotations being applied directly (or with combinations):

@Preview(name = "phone", device = "spec:shape=Normal,width=360,height=640,unit=dp,dpi=480")
@Preview(name = "foldable", device = "spec:shape=Normal,width=673,height=841,unit=dp,dpi=480")
@Preview(name = "tablet", device = "spec:shape=Normal,width=1280,height=800,unit=dp,dpi=480")
@ThemePreviews
@Composable
fun MyComponent() {
    // ...
}

I feel like a general solution on the information-gathering side would be to transitively collect a list of all @Previews applied to a @Composable. So perhaps ShowkaseBrowserComponent would include a List of Preview (or some other identical looking object), where that includes all Previews declared directly on the @Composable, as well as all transitive @Previews from multi-preview annotations.

That would admittedly be a fairly large change, and also doesn't answer the question about if the Showkase runtime should actually render all N versions, where N is the size of that list.

[vinaygaba]

@alexvanyo Actually @oas004 has already implemented and merged stacking previews functionally in this PR - https://github.com/airbnb/Showkase/pull/259

So we are very close to having proper multi preview support (final PR here - https://github.com/airbnb/Showkase/pull/263). What's blocking him are these open questions and I'm a bit split on how to handle this from the perspective of what's rendered in the ShowkaseBrower (and thus the metadata object which is also used for screenshot testing)

[oas004]

That is a good point. In the implementation of stacked previews from #259 we are rendering one component for each preview annotation. So for instance

@Preview(name = "phone", device = "spec:shape=Normal,width=360,height=640,unit=dp,dpi=480")
@Preview(name = "foldable", device = "spec:shape=Normal,width=673,height=841,unit=dp,dpi=480")
@Preview(name = "tablet", device = "spec:shape=Normal,width=1280,height=800,unit=dp,dpi=480")
@Composable
fun MyComponent() {
    // ...
}

Would result in 3 views. That would potentially lead to duplicate screenshot tests in the end. I think that if we would come to the conclusion to provide a way to filer it out, it would be nice to have it for the stacked previews like the case above as well :)

Just thinking, but would it be something that could be filtered out by the user of the ShowkaseBrowserComponent? I think if Showkase would provide a filtering it could potentially lead to some confusion (at least if we do it in a way the user can't control). On the other side, that could also take away some of the seamlessness of the library and lead to more configuration setup for the user. Also it would still lead to Showkase rendering all the equal permutations if they are not using the ShowkaseBrowserComponent directly.

[vinaygaba]

@elihart @pmecho I'd love to get y'all to chime in as well. What according to y'all should be the ideal behavior?

[alexvanyo]

Oh that's awesome, I hadn't seen that!

For screenshot tests specifically, there's an open question I think about which tool should be responsible for what.

If we had all of the information in the @Preview (device, locale, font size, dark mode, etc.) then in theory you could use that to configure the screenshot tool to take different screenshots that match up with those different @Preview annotations.

That doesn't really solve the case for the component browser, as that shares a fairly similar problem of running a @Preview directly on device with Android Studio as well. In those situations, the extra information in the @Preview would go unused (as the real device/emulator configuration is used instead).

[oas004]

Yeah I agree. Maybe it makes sense to mimic Android Studio on this? We could at least provide the type from the custom annotation that is used to generate the metadata I think, but that would not solve the case for stacked previews.

[pmecho]

I like the idea of mimicking Android Studio. For the browser, what about filtering out previews that are larger than the device's screen bounds? This could include a stub message to the user saying as such to reduce confusion on a potential missing screenshot.

[oas004]

I have updated my pr with the same behaviour as the AS preview. After trying a bit with the AS preview deploy to device functionality, I have realised that that has a bit different approach to this problem as it seems to ignore multiple previews. Might be wrong about this, but to me it seems like that at least. Maybe it makes more sense to mimic that functionality instead of the embedded preview? At least for the size, UIMode and device? If that is the case I can just filter them out before generating the metadata :) Furthermore, it would be nice if someone could review my PR if anyone has time 😄