Slider handle current time #2255
Conversation
| right: -1.5em; | ||
| font-size: 0.6em; | ||
| color: $primary-bg; | ||
| content: attr(data-current-time); |
There was a problem hiding this comment.
This has a much higher browser usage than I though, cool.
There was a problem hiding this comment.
I was honestly pretty astonished to see it works in IE8. TIL.
|
One question, LGTM otherwise. |
|
@gkatsev I like that idea. Last commit splits out that logic (even though I think it's functionally the same now). |
| /* Also show the current time tooltip */ | ||
| .video-js .vjs-progress-control:hover .vjs-play-progress:after { | ||
| display: block; | ||
| font-size: 0.6em; |
There was a problem hiding this comment.
If someone wants it always up (a la Vimeo), they could just set the main
one to display: block. If we don't change the font size on hover, it's
comically huge.
On Wed, Jun 10, 2015 at 6:33 PM Gary Katsevman notifications@github.com
wrote:
In src/css/components/_progress.scss
#2255 (comment):@@ -27,6 +27,12 @@
/* We need an increased hit area on hover */
.video-js .vjs-progress-control:hover .vjs-progress-holder { font-size: 1.666666666666666666em }+/* Also show the current time tooltip */
+.video-js .vjs-progress-control:hover .vjs-play-progress:after {
- display: block;
- font-size: 0.6em;
why does the font-size change?
—
Reply to this email directly or view it on GitHub
https://github.com/videojs/video.js/pull/2255/files#r32183524.
There was a problem hiding this comment.
I'm still not quite following, sorry.
There was a problem hiding this comment.
If you set .video-js .vjs-play-progress:after to display: block, it would look fine at 0.9em. If we don't reduce the font size here on hover, however, it ends up way too big (imo).
There was a problem hiding this comment.
Ah, I see. On hover, you want it slightly smaller following the mouse, but if you want it always on, you want it slightly bigger.
The only issue is that if you turn it on manually to always show, on hover, the font-size will change and it will flicker.
There was a problem hiding this comment.
Well it still grows a tiny bit, it's just a smaller percentage of the bigger progress holder font-size.
But yes, you're right. There's definitely a hitch when you roll off. Bummer.
mmcc
force-pushed
the
slider-handle-current-time
branch
from
75121f9
to
f52d8a3
Compare
|
LGTM |
mmcc
force-pushed
the
slider-handle-current-time
branch
from
f52d8a3
to
35a5bdb
Compare
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 35a5bdb (Please note that this is a fully automated comment.) |
|
@pam retry |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 35a5bdb (Please note that this is a fully automated comment.) |
| right: -1.5em; | ||
| font-size: 0.9em; | ||
| color: $primary-bg; | ||
| content: attr(data-current-time); |
|
Merged in. |
This adds a
data-current-timeattr to the play progress bar, and adds CSS necessary to show that when you hover over the progress control.