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

Initial support for tags and extra metadata on components #309

Merged
merged 17 commits into from Apr 3, 2023

Conversation

pmecho
Copy link
Contributor

@pmecho pmecho commented Mar 27, 2023

This starts the support for adding generic strings to tag and label components to help with filtering and custom categorization. Tags are added to the browser for filtering while metadata is only accessible from the generated ShowkaseCodegenMetadata object. A follow-up PR will add visual tag filtering to the built in browser.

*
* @param tags Various string values that will be propagated to the Showkase browser to allow additional
* filtering or categorization.
* @param extraMetadata Various string values that are **not** propagated through the Showkase browser
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add more information about how it's primarily meant to add more information for custom use cases that you might have

componentKDoc = "",
componentKey = """com.airbnb.android.showkase_processor_testing_null_group_name_0_null""",
isDefaultStyle = false,
tags = listOf("tag A", "tag B"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

you want the extra metadata to be available here as well no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops I missed that the other classes were internal 🤦

Copy link
Collaborator

@vinaygaba vinaygaba left a comment

Choose a reason for hiding this comment

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

Had one main suggestion that needs to be incorporated

@pmecho pmecho force-pushed the pe--tag-and-metadata-support branch from c596824 to edc4f1d Compare March 31, 2023 20:44
Copy link
Contributor Author

@pmecho pmecho left a comment

Choose a reason for hiding this comment

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

@vinaygaba I also can't seem to get CI to fully pass. These jobs never start and I don't see a way to manually start them.

image

@@ -80,6 +87,8 @@ interface ShowkaseScreenshotTest {
name: String,
group: String,
styleName: String? = null,
tags: List<String> = emptyList(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vinaygaba Are you okay with adding the tags and metadata here as well? We need this for correct attribution of our screenshots since we will likely have duplicate group/name/styleName and are using the extraMetadata to handle additional categorization.

This would also be a breaking change.

@vinaygaba vinaygaba merged commit 7555419 into master Apr 3, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants