Skip to content
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

Fixed displaying the LIVE control bar item #2551

Merged
merged 1 commit into from
Sep 8, 2015
Merged

Conversation

heff
Copy link
Member

@heff heff commented Sep 3, 2015

...and also the layout in IE8 when live. Both are currently broken in the new skin.

Related to #2507

@mmcc
Copy link
Member

mmcc commented Sep 3, 2015

lgtm (update: I have edits)

.video-js .vjs-live-control {
.vjs-live .vjs-time-control,
.vjs-live .vjs-time-divider,
.vjs-live .vjs-progress-control {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In keeping with the vjs-hidden stuff we were talking about, should we add the class to these in updateShowing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mean have the live component manage the visibility of the other components, I think that would become a mess. I did consider having each of those components hide itself when the player is live, but I'm on the fence and not sure where to draw the line of when we use CSS states and when we use javascript for hiding. I'm also not sure how Live+DVR will affect these, so for now I think I'd lean towards leaving them like this.

@heff heff force-pushed the fix-live branch 3 times, most recently from 6c5a1c6 to 025c77d Compare September 4, 2015 19:59
@heff heff added this to the v5.0.0 milestone Sep 8, 2015
...and also the layout in IE8 when live

Reorganized the styles some to group the vjs-live changes
with the components their affecting.

closes videojs#2551
@heff heff merged commit f7702f8 into videojs:master Sep 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants