-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Progress bars: center labels vertically and set height on parent .progress #22703
Changes from 5 commits
62e5c7a
bbdf54c
24565ae
b7b7b61
b69c814
6bf769b
13cd716
a0b39a9
7e0912a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,32 +1,37 @@ | ||
// Progress animations | ||
@keyframes progress-bar-stripes { | ||
from { background-position: $progress-height 0; } | ||
to { background-position: 0 0; } | ||
} | ||
|
||
// Basic progress bar | ||
.progress { | ||
display: flex; | ||
align-items: center; | ||
height: $progress-height; | ||
overflow: hidden; // force rounded corners by cropping it | ||
font-size: $progress-font-size; | ||
line-height: $progress-height; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably safe to remove this? Or we might need to change the value at least. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, it looks like align-items is indeed safe to remove, but the height is needed (empty bars will collapse to zero with no explicit height). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Height is good to stay here, but I meant we can probably remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, gotcha, removing. (align-items seems to also be safe with having it on the |
||
text-align: center; | ||
background-color: $progress-bg; | ||
@include border-radius($progress-border-radius); | ||
@include box-shadow($progress-box-shadow); | ||
} | ||
|
||
.progress-bar { | ||
height: $progress-height; | ||
line-height: $progress-height; | ||
display: flex; | ||
align-items: center; | ||
justify-content: center; | ||
height: 100%; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't necessary as far as I can tell. See https://jsbin.com/wahusavehu/edit?html,css,output. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great, I'll remove height. :) |
||
color: $progress-bar-color; | ||
background-color: $progress-bar-bg; | ||
@include transition($progress-bar-transition); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @davethegr8 @mdo I see that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe so, probably an oversight on my part. |
||
} | ||
|
||
// Striped | ||
.progress-bar-striped { | ||
@include gradient-striped(); | ||
background-size: $progress-height $progress-height; | ||
} | ||
|
||
// Animated | ||
.progress-bar-animated { | ||
animation: progress-bar-stripes $progress-bar-animation-timing; | ||
} |
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.
You can drop these newly added comments—they don't add any new context that the code itself doesn't already tell you :).
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.
Oh whoops, sorry. I was editing the npm package for testing in my project, and those snuck into the dist scss files.