-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add ImageWithBaseline
#7262
Add ImageWithBaseline
#7262
Conversation
518f1c1
to
d8a8555
Compare
object IconsWithBaseline { | ||
object Filled { | ||
val warning: ImageWithBaseline by lazy { | ||
ImageWithBaseline(image = MaterialIcons.Filled.Warning, baseline = 21.dp) |
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.
Where is baseline = 21.dp
derived from? Is it universally usable or bound to a specific context?
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.
val iconScalingFactor = iconSize / warningIcon.image.defaultHeight | ||
val iconBaseline = warningIcon.baseline * iconScalingFactor | ||
|
||
Icon( |
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 could be an IconWithBaseline
that does the calculation internally, based on the requested size.
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.
Agreed. But to discuss the approach it felt easier to leave everything in one place.
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 is a small visual change. But it introduces
ImageWithBaseline
that can be used as a building block when an icon (with it's own baseline) should be aligned to the baseline of some text next to it.The main purpose of this pull request is to get the design of the "image with baseline" concept right.