-
Notifications
You must be signed in to change notification settings - Fork 3
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
[Snapshots] Fix device specs #285
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
📸 Snapshot Test73 modified, 3 added, 51 unchanged
🛸 Powered by Emerge Tools |
} | ||
|
||
private fun dpToPx(dp: Int, scalingFactor: Float): Int { | ||
return (dp * scalingFactor).toInt() |
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'm guessing this just needs to be explicitly rounded down.
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 updated to use toInt()
in the pixel conversion later. Ended up with the proper dimensions going with that approach
snapshots/snapshots/src/main/kotlin/com/emergetools/snapshots/compose/DeviceUtils.kt
Outdated
Show resolved
Hide resolved
853f85a
to
dfaaaf4
Compare
Fixes device specs to be true-to-size to reference devices. Also adjusts to properly scale custom specs. Added a repro case of a troublesome client snapshot that wasn't rendering in landscaped.
Verified with a sxs run against Google's compose preview snapshotting library, Emerge matches 1:1 on all device specs now. Android Studio images (copied directly from the previews) appear smaller, and dimensions do not match the reference device specs, so I believe matching the expected device dimensions like we do now is preferred over 1:1 matching of AS.
Also checked all reference devices against the outputted dimensions from Emerge snapshots after this change and all match 1:1.