feat: Updated route-pill with a new size, currently called "large-format" #2600
Conversation
| display: flex; | ||
| flex-direction: column; | ||
| justify-content: center; |
There was a problem hiding this comment.
question(non-blocking): why the switch to flex?
There was a problem hiding this comment.
After expanding the route pill width/height, I needed a way to get the text centered, and flex is what always comes to mind for positioning for me. But I also realize, I could instead expand line-height for the text to be the height of the route-pill, so line-height:2.25rem. (I didn't think of that since the Figma specified line-height 1.5rem, but it's a value that doesn't have impact.) Is this preferable?
.c-route-pill {
width: 2.875rem;
height: 1.5rem;
border-radius: 0.8125rem;
text-align: center;
font-size: 1rem;
line-height: 1.5rem;
font-weight: 700;
font-family: $font-family-route-pill;
&--large-format {
width: 4.5rem;
height: 2.25rem;
line-height: 2.25rem;
border-radius: 1.875rem;
font-size: $h3-font-size;
font-weight: 600;
}
}
There was a problem hiding this comment.
Would line-height: 1.5 work? That way it would auto-adjust based on the font size
https://developer.mozilla.org/en-US/docs/Web/CSS/line-height#number
There was a problem hiding this comment.
The height to font-size ratio is not consistent for both versions of this button. So font-size:height for the default version is 1:1.5 while font-size:height for the large format is 1.25:2.25 or 1:1.8
EDIT: so line-height cannot be simplified to one ratio or the other
There was a problem hiding this comment.
Ahhh - I had thought it was 1.5 : 2.25 and 1 : 1.5, which would have been consistent.
I'll back out of this conversation then, since I don't have an opinion on this (I would probably have gravitated towards flex too, fwiw)
Coverage of commit
|
|
I need to make a story for this! EDIT: ✅ done |
Coverage of commit
|
Coverage of commit
|
| @@ -1,6 +1,5 @@ | |||
| .c-route-pill { | |||
| min-width: 2.875rem; | |||
There was a problem hiding this comment.
I made one comment that I'm interested in your thoughts on, and I think this is 👍 either way
Coverage of commit
|
Asana Ticket: https://app.asana.com/0/1205385723132845/1207260183650054
Test new format by adding className
c-route-pill--large-formatto anyRoutePill. Otherwise,RoutePilldefaults to smaller format.