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

Exit fullscreen when on ExitVR() #2264

Merged
merged 1 commit into from
Jan 10, 2017
Merged

Conversation

fernandojsg
Copy link
Member

Right now if we call scene.exitVR() it will just go back to the standard state if we are on VR but not if we're just in fullscreen mode. This PR introduces a call to exitFullScreen on the exitVR function.

if (!this.is('vr-mode')) { return Promise.resolve(); }
if (checkHeadsetConnected() || this.isMobile) {
return this.effect.exitPresent().then(exitVRSuccess, exitVRFailure);
}
exitVRSuccess();
function exitFullscreen () {
Copy link
Member

Choose a reason for hiding this comment

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

Could move this down to where the enter fullscreen function is

Copy link
Member Author

Choose a reason for hiding this comment

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

sure!

@@ -184,11 +184,21 @@ module.exports = registerElement('a-scene', {
exitVR: {
value: function () {
var self = this;
exitFullscreen();
Copy link
Member

Choose a reason for hiding this comment

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

We should call this after the not-in-VR check below

Copy link
Member

Choose a reason for hiding this comment

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

I think it won't work. Just entering fullscreen does not set the state to vr-mode

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep

@ngokevin
Copy link
Member

r+wc

@fernandojsg fernandojsg merged commit a6100f8 into aframevr:master Jan 10, 2017
@fernandojsg fernandojsg deleted the fullscreen branch January 10, 2017 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants