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

Fix bug in getShowkaseMetadataFromPreview where the preview width was being set as the showkase metadata height #280

Conversation

stefankram
Copy link
Contributor

This PR contains a fix for an issue I discovered when trying to get the height information of a @Preview from a ShowkaseBrowserComponent.

Please let me know if I need to add any other tests to get this merged.

@@ -287,7 +287,7 @@ internal fun getShowkaseMetadataFromPreview(
showkaseName = showkaseName,
showkaseGroup = showkaseGroup,
showkaseWidthDp = if (width == -1) null else width,
showkaseHeightDp = if (height == -1) null else width,
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh great catch!

@vinaygaba
Copy link
Collaborator

@stefankram seems like some tests are failing. Will you do a pass and fix them 🙏

@stefankram
Copy link
Contributor Author

@vinaygaba yep will take a look!

@stefankram stefankram force-pushed the bugfix/height_not_mapped_correctly_in_generated_code branch from ca702b2 to 212963a Compare December 6, 2022 16:32
@stefankram
Copy link
Contributor Author

@vinaygaba this should be good to re-test

@stefankram
Copy link
Contributor Author

Hey @vinaygaba I noticed a new version was created. I was hoping to get this PR merged and into that release...

Is there anything else I can help with to get this merged down?

@vinaygaba
Copy link
Collaborator

@stefankram about to merge this! Just re-running the build

@vinaygaba vinaygaba merged commit 8c3bf63 into airbnb:master Dec 16, 2022
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