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

Check exitVR property of keyboard-shortcuts to enable Escape shortcut #5512

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mrxz
Copy link
Contributor

@mrxz mrxz commented Apr 8, 2024

Description:
Noticed that the keyboard-shortcuts component only had an option to disable the enterVR shortcut, but it actually disabled both the enter and exit shortcuts. This doesn't match the documentation nor the user's expectation, so this PR splits it into a enterVR and exitVR property to configure the enter and exit shortcuts respectively.

Changes proposed:

  • Introduce exitVR property to keyboard-shortcuts similar to the existing enterVR property

@dmarcos
Copy link
Member

dmarcos commented Apr 8, 2024

Interesting. In what scenario did you notice the inconsistency? On desktop fullscreen, the esc key is the browser standard shortcut to exit. On mobile it doesn't apply. In headset each system has a different way to disengage immersive mode, usually with controllers or hand tracking where keyboard is not avail.

Wondering if we should revisit the logic

@mrxz
Copy link
Contributor Author

mrxz commented Apr 8, 2024

I do occasionally use a Bluetooth keyboard with the Quest and also sometimes use PCVR to record WebXR sessions. In both cases the keyboard remains available to the user while in session. But that wasn't actually how I discovered it, that was simply through the immersive web emulator.

I customized the enter-vr experience and disabled the shortcut to prevent people from skipping it. At that point I noticed this also prevented me from hitting escape to exit the emulated WebXR session.

@dmarcos
Copy link
Member

dmarcos commented Apr 8, 2024

Wonder if esc to exit VR should be a browser / standard behavior. Similar to fullscreen. I think this PR is redundant for desktop / fullscreen?

@dmarcos
Copy link
Member

dmarcos commented Apr 8, 2024

Are there scenarios where one wants one are not the other (exit / enter)?

@mrxz
Copy link
Contributor Author

mrxz commented Apr 8, 2024

Wonder if esc to exit VR should be a browser behavior. Similar to fullscreen. I think this PR is redundant for desktop / fullscreen?

It's pretty much redundant for fullscreen on desktop (although the spec doesn't seem to prescribe Escape, just that "User agents should provide a means of exiting fullscreen that always works")

Are there scenarios where one wants one are not the other (exit / enter)?

It's not uncommon to have some steps before entering a session (e.g. selecting an avatar, typing a name, etc...), in which case just disabling the enter vr shortcut makes sense.

@dmarcos
Copy link
Member

dmarcos commented Apr 8, 2024

Are there scenarios where one wants one are not the other (exit / enter)?

It's not uncommon to have some steps before entering a session (e.g. selecting an avatar, typing a name, etc...), in which case just disabling the enter vr shortcut makes sense.

For the sake of simplicity can we just keep it to one property? When concrete limitations arise we can add a second one. Not sure how much the shortcuts are actually used today.

@mrxz
Copy link
Contributor Author

mrxz commented Apr 9, 2024

For the sake of simplicity can we just keep it to one property? When concrete limitations arise we can add a second one. Not sure how much the shortcuts are actually used today.

There are two distinct questions:

  • Should either or both of the shortcuts be removed (as they might not be used)?
  • Should it be possible to disable them individually or have one toggle?

While I can't comment on how often people use them, I do use exit-vr (esc) on a daily basis with the emulator. Wanting to disable the enter-vr shortcut lead to both being disabled, which contradicts both the name (enterVR) and the docs. From that POV, removing exit-vr harms DX and having only one property is a tad cumbersome.

@vincentfretin
Copy link
Contributor

I sometimes inadvertently press "f" because it's too close to "d" key.
On Chrome desktop (that have the VR button) it makes the experience full screen and you can't rotate, not great for a user that don't know what's going on.
On firefox (that have the fullscreen button) the "f" shortcut enter fullscreen and works great.
I usually disable the shortcut in my experiences and I have a custom VR button only showing if a headset is connected after some other ui screen to type the name and select avatar, so I didn't know the esc key wasn't working. Doing that, I know that I don't support VR from PC, it's ok for my use cases.
Having the option to disable the "f" key and still be able to use "esc" is good indeed.

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.

None yet

3 participants