Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fullscreen #1504

Closed
wants to merge 4 commits into from
Closed

Fullscreen #1504

wants to merge 4 commits into from

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Sep 15, 2014

  • Make sure that isFullScreen gets exported correctly.
  • in enterFullscreen in the html5 tech, add some functionality to toggle isFullscreen and trigger the vjs fullscreenchange event when on ios.

@heff
Copy link
Member

heff commented Sep 15, 2014

Up to this point we haven't really needed those events to fire when enterFullScreen is used and the native controls take over, because no buttons or other components are visible at least. What's the new case where we need those events in enterfullscreen mode?

@@ -79,7 +79,8 @@ goog.exportProperty(vjs.Player.prototype, 'supportsFullScreen', vjs.Player.proto
goog.exportProperty(vjs.Player.prototype, 'currentType', vjs.Player.prototype.currentType);
goog.exportProperty(vjs.Player.prototype, 'requestFullScreen', vjs.Player.prototype.currentType);
goog.exportProperty(vjs.Player.prototype, 'cancelFullScreen', vjs.Player.prototype.currentType);
goog.exportProperty(vjs.Player.prototype, 'isFullScreen', vjs.Player.prototype.currentType);
Copy link
Member

Choose a reason for hiding this comment

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

omg...

@gkatsev
Copy link
Member Author

gkatsev commented Sep 15, 2014

@heff I'm not sure there really is any need for the videojs' fullscreenchange to fire. I can remove these if you think it's not worth having yet.

@heff
Copy link
Member

heff commented Sep 15, 2014

If there's no specific use case behind it yet I think we can leave it out and save ourselves some longterm testing and maintenance overhead. If we get a real use case we can always come back to this.

The export mistakes still need to be fixed though. :)

@gkatsev
Copy link
Member Author

gkatsev commented Sep 15, 2014

export mistakes have been fixed.

This event isn't really being used for this usecase right now, so, no
need to trigger it.
@gkatsev
Copy link
Member Author

gkatsev commented Sep 15, 2014

@heff alright, the exports should be fixed now and we're no longer triggering the fullscreenchange event.

This was referenced Sep 16, 2014
@heff
Copy link
Member

heff commented Sep 17, 2014

Being handled by #1511 now.

@heff heff closed this Sep 17, 2014
@gkatsev gkatsev deleted the fullscreen branch August 12, 2015 18:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants