Styles: Adjust and even-out padding for buttons and entry - #980
Conversation
danirabbit
left a comment
danirabbit
left a comment
There was a problem hiding this comment.
Looks like something has gone wrong here. I'm guessing you're using some kind of text scaling? These don't match up with default text scaling on my end
| min-height: 24px; | ||
| min-width: 24px; |
There was a problem hiding this comment.
I think these are min 16px because of icons. Probably don't want to touch the min size here and just change padding?
|
@danirabbit Yeah I forgot I use text scaling. It's properly aligned at 1x now, but because icons resize at thresholds and don't add any padding to compensate between icon and text size differences, image buttons linked or inline with a text button will be rectangular. Setting a minimum size of 16px for other types of buttons and such fixes the problem on the minimized end, but I'm not sure how to compensate on the magnified end. Ex. 
(Looks like we might have some double border issue with the linked button borders 🙃 ) |
| @extend .circular; | ||
|
|
||
| padding: rem(4px); | ||
| padding: calc(0.5rem - 2px); |
There was a problem hiding this comment.
Can we make a constant for this and comment what the magic number here is?
| padding: rem(2px) rem(4px); | ||
| min-height: 16px; | ||
| min-width: 16px; | ||
| padding: calc(0.5rem - 2px); |
There was a problem hiding this comment.
In text buttons its intentional for x padding to be double
|
Looks good I'd just like to see this abstracted so it stays in sync |
3 participants
Might be a little contentious? I think the little bit of extra room on buttons gives a little better rhythm when types are mixed, but I tried not to turn them into balloons.
Before
After