-
Notifications
You must be signed in to change notification settings - Fork 206
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
Workaround: Inject annotation values to screenshot testing environment at runtime through predefined constants #649
Conversation
Showkase should pick up and propagete these values even if custom previews are repeated reflective annotations. This workaround just estimates preview configurations from a group and a component key. I guess this is fragile a little bit.
@ShowkaseMultiplePreviewsWorkaround | ||
@Composable | ||
private fun ProvidesPreviewValues(group: String, componentKey: String, content: @Composable () -> Unit) { | ||
val components = componentKey.split("-") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was ambiguous, so fixed in 89bdf36.
@VisibleForTesting | ||
@Retention(SOURCE) | ||
@Target(CLASS, FUNCTION) | ||
annotation class ShowkaseMultiplePreviewsWorkaround |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For removing someday? This seems useful 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! We should remark code hunks for this workaround cuz they are large and are crosscutting concerns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks absolutely better than what we have now!
Previous implementaion led ambiguous
|
||
object Japanese { | ||
const val Name = "Japanese" | ||
const val Locale = "ja_JP" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed this to a locale with a country because Android always provides a country code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Thank you for your reviews~ |
Issue
Overview (Required)
Links
Screenshot