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

Show and hide controls with mousemove / Select 'Off' in subtitles menu by default #193

Closed
wants to merge 3 commits into from

Conversation

stevecochrane
Copy link
Contributor

Whoops, I intended these to be separate but it looks like all of my updates have been combined into one pull request now. Here's some info, I hope these are helpful!

  1. Show and hide controls with mousemove (src/controls.js, src/lib.js)
    I'm just now noticing that you have a new fullscreen events branch, so maybe this is redundant. No hard feelings if you don't want to use my update. Previously, the player would show the control bar whenever the mouse is hovering over the player, and since the mouse is always hovering over the player while in fullscreen mode, the controls would always be visible then. So I've replaced that with a new method involving mousemove events. To sum up, every time the mouse is moved over the player, the control bar fades in and then a timed event is set to fade out the control bar after 1.5 seconds. I left a lot of comments in the code so there's more explanation in there. Unfortunately I wasn't able to test this in IE8 and below since 3.2.2 seems broken in IE8 and below at the moment, but it seems to work well with everything else.

UPDATED: I fixed a rather obvious shortcoming with yesterday's code: touch devices weren't being accounted for. So I've added a new isTouchDevice() check to lib.js to ensure that non-iOS devices can be tested for, which uses the latest detection from Modernizr (http://stackoverflow.com/questions/4817029/whats-the-best-way-to-detect-a-touch-screen-device-using-javascript) and if that returns true, then the control bar is always shown just like the player's previous behavior for tablets. I've only tested this on an iPad (1st gen) so far but this should work in theory for Android tablets, Windows tablets, etc.

  1. Select 'Off' in subtitles menu by default (src/tracks.js)
    I thought it was a little weird that nothing in the subtitles menu is selected by default, and that you can then click to select 'Off' if you want but nothing changes. The menu already effectively defaults to 'Off' so this just confirms it.

@jordanj77
Copy link

Nice work! I haven't tested it too thoroughly, but I've implemented this code and so far it's working great.

Edit: I'm talking about the mousemove stuff, I didn't notice the subtitles fix ;)

@lalitkapoor
Copy link

Note: for version 3.2.0 I needed to replace the .on with .addEvent for the mouse idling feature that is a part of this pull request.

@heff
Copy link
Member

heff commented Apr 4, 2013

@stevecochrane Sorry I didn't get this pulled in. I made change (2) manually and I believe there's another pull request working on change (1) in the new 4.0 codebase. #403 . Let me know if you think that's a good approach. Thanks for your help.

@heff heff closed this Apr 4, 2013
@stevecochrane
Copy link
Contributor Author

That's totally fine. My pull request is 10 months old now so that new pull request must be better. I actually can't figure out how to make a build of the 4.0 version (not familiar with Node.js yet) so I haven't gotten to try it but if it's okay with you and the other contributors then it's okay with me.

@heff
Copy link
Member

heff commented Apr 8, 2013

Cool, thanks Steve. Shoot me an email if you want help setting up the new version.

On Apr 8, 2013, at 10:31 AM, Steve Cochrane [email protected] wrote:

That's totally fine. My pull request is 10 months old now so that new pull request must be better. I actually can't figure out how to make a build of the 4.0 version (not familiar with Node.js yet) so I haven't gotten to try it but if it's okay with you and the other contributors then it's okay with me.


Reply to this email directly or view it on GitHub.

@heff heff mentioned this pull request Aug 6, 2013
heff pushed a commit that referenced this pull request Aug 9, 2013
@heff
Copy link
Member

heff commented Aug 9, 2013

Thanks for you help on this @stevecochrane. I started from your controls hiding changes. If you have any feedback let me know.

@stevecochrane
Copy link
Contributor Author

Great, I'm glad that was helpful!

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.

4 participants