fix: cleanup window listeners #15
Conversation
feat: Convert to advanced plugin
brandonocasey
force-pushed
the
fix/cleanup-window-listeners
branch
from
d3a2253
to
edf450c
Compare
|
|
||
| // register the plugin with video.js | ||
| videojs.registerPlugin('vr', vr); | ||
| VR.prototype.requestAnimationFrame = Component.prototype.requestAnimationFrame; |
There was a problem hiding this comment.
Using these rather than window for auto cleanup
src/plugin.js
Outdated
|
|
||
| // if the current type has a case insensitivie match from the list above | ||
| // this is hls | ||
| return hlsTypes.some((type) => (new RegExp(type, 'i')).test(currentType)); |
There was a problem hiding this comment.
Should this either be type.toLowerCase() === currentType.toLowerCase() or alternately new RegExp(^${type}$, 'i')? My concern is that we'd get false matches for things that include these as part of the currentType.
There was a problem hiding this comment.
yeah we should probably do a full match here
src/plugin.js
Outdated
| this.vrDisplay = displays[0]; | ||
| this.log(this.vrDisplay); | ||
| } else { | ||
| this.triggerError({code: 'web-vr-no-devices-found', dismiss: false}); |
There was a problem hiding this comment.
There are a few cases where this is being called as triggerError, but it appears the function is triggerError_. It seems it would... ironically... trigger an error! 😆
There was a problem hiding this comment.
haha, yeah must have failed to convert the name over when I switched in it all places
chore: Swap webvr for webxr polyfill packages (PR fix ups - adding 180 top/bottom)
feat: Convert to advanced plugin