-
Notifications
You must be signed in to change notification settings - Fork 323
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
0.10.0 #298
0.10.0 #298
Conversation
…ndency. No longer rely on global WebVRConfig and instantiate as traditional constructor.
…ructor now, and no longer bake utility function results in closures as they're called only once/twice during initialization, and this makes testing easier
…on commands, modernize workflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome change!
test/init.js
Outdated
// Logic similar to https://github.com/webmodules/custom-event | ||
// but can't rely on it's feature detection due to timing | ||
// of including the shim and populating window/document | ||
global.CustomEvent = function CustomEvent (type, params) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no space after CustomEvent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, why not use (type, params) => {
Seems clearer...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lint fixed, and this is a constructor to simulate the global in cardboard-vr-display (which relies on globals unfortunately) as these run in a node environment
test/webvr-polyfill.test.js
Outdated
}); | ||
|
||
describe('hasNative', () => { | ||
it('false when no native support found', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
standard it string would be 'should be false when no native support found'. Same for below (add 'should be')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
…ee.js source code, fix #297 with global.document clobbering, and fix typo in webvr-polyfill.js
a60492c
to
65c6298
Compare
Have been working on these changes for a bit now, quite a big overhaul! Wanted to put up a PR to get comments and concerns from the WebVR community before landing. Changes and rationale from the release notes draft as follows:
v0.10.0
This is one of the largest updates for the Polyfill in quite some time! Most changes are refactoring, focusing on pulling pieces out to make it easier to scope, test, and work on future changes as the WebXR Device API approaches, so we can be more confident in changes without breaking edge cases. The polyfill was also scope creeping a bit, doing more than just providing a stable API surface for developers to target, and many of these features have been removed that are better off in userland and on a per-project basis.
Feature Changes
new window.WebVRPolyfill(options)
. Previously this was handled by defining options on a globalwindow.WebVRConfig
object, and the polyfill would instantiate itself immediately, or require the user to callwindow.InitializeWebVRPolyfill()
ifDEFER_INITIALIZATION
was set.DEFER_INITIALIZATION
has been removed, the polyfill no longer instantiates itself, and options are passed into the constructor rather than relying on a global object. This makes it easier to work with in module environments and other JS build systems.InstallWebVRSpecShim()
has been removed.navigator.vrEnabled
property has been removed. This was used to indicate if displays exist, which can be done by checking the return value ofgetVRDisplays()
.MouseKeyboardVRDisplay
has been removed. This provided a monoview VRDisplay with some primitive controls for a 3DOF experience. This is better handled with custom controls or OrbitControls depending on the experience.MOUSE_KEYBOARD_CONTROLS_DISABLED
option has been removed. This means on desktop, the only displays that can exist are native, so user code should handle when no displays are found.MouseKeyboardVRDisplay
, and the WebXR Device API only allowing one display rather than an array of displays,ALWAYS_APPEND_POLYFILL_DISPLAY
has been removed. A new option,PROVIDE_MOBILE_VRDISPLAY
has been created to provide aCardboardVRDisplay
when on mobile and the native API exists but no native displays exists.true
by default, this means that the polyfill is always injected on mobile when enabled, since we won't know immediately if any native displays exist.FORCE_ENABLE_VR
has been removed. This always provided aCardboardVRDisplay
, even on desktop, used mostly for debugging purposes.DEBUG
option. Previously this was triggered by a?debug
query parameter, which caused issues with other environments.main
package.json entry now points to the built output rather thansrc/node-entry
. This lets us take advantage of our build system and not require consumers to handle any transformations implemented.WebVRPolyfill
methods and properties have been removed:isFullScreenAvailable
,isCardboardCompatible
,isMobile
,NativeVRFrameData
,getVRDevices
,enableDeprecatedPolyfill
,isDeprecatedWebVRAvailable
,isWebVRAvailable
.Infrastructure Changes