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

Formalize tech "features" #1474

Open
heff opened this issue Sep 2, 2014 · 5 comments
Open

Formalize tech "features" #1474

heff opened this issue Sep 2, 2014 · 5 comments
Labels
enhancement pinned Things that stalebot shouldn't close automatically
Milestone

Comments

@heff
Copy link
Member

heff commented Sep 2, 2014

We're currently using the "features" prefix on tech properties to provide access to whether or not specific techs support different features.

  • featuresVolumeControl
  • featuresFullscreenResize
  • featuresPlaybackRate
  • featuresProgressEvents
  • featuresTimeupdateEvents

Some of these are hard-coded into the tech and some are discovered when the tech is loaded. They should all be booleans.

This has worked ok so far but it could probably be improved on. Specifically, there should be a way that external plugins can access whether or not the current tech supports specific features (currently these are only accessible in core). At minimum this should be documented so that tech writers can know how to use them.

We did run into an issue with them in #1407, and had to move them directly to the prototype of the tech instead of under a .features. object.

Related: #1147

@heff heff added this to the Techs 2.0 milestone Sep 2, 2014
@heff
Copy link
Member Author

heff commented Oct 2, 2014

I'm currently not sure what featuresFullscreenResize does any more. Nothing in core at least. I think it's a relic.

@gkatsev
Copy link
Member

gkatsev commented Oct 2, 2014

Yeah, doesn't seem to be used anywhere.

@heff
Copy link
Member Author

heff commented Dec 15, 2014

Adding track related features to this list with #1736. With captions, the captions menu needs to know if the track is using native captions vs. the custom captions, so it can enable the captions settings menu when custom captions are used. It's similar to the native controls issue we had to solve for, however usingNativeControls doesn't exactly account for if the tech does have native controls or not. It probably should.

Our native fullscreen check should be considered in this too.

The can[something] could probably just be combined with the feature setting instead of being their own function.

I thinking 'supports' would be a clearer prefix for these names. "supportsVolumeControl, supportsFullscreenResize". Would changing this be a breaking change for any external code?

We still need to find the right way to expose this at the player level so you don't need to have access to the tech directly. Maybe something like player.supports('VolumeControl').

@heff
Copy link
Member Author

heff commented Mar 11, 2015

I have some new thoughts on this one where I think we can avoid most of these extra variables.

I think we should provide a player level method for checking the current support of different features that plugins/components can use to change their state.

player.supports('playbackRate');
player.supports('volume');

For example, the playbackRate menu would check player.supports('playbackRate') and only show itself if it's true. It would need to do this on the loadstart event to support tech changes where the value might change.

Inside supports() the player would check for the playbackRate method on the tech. For HTML5, some browsers support playbackRate and some don't. In this case the tech should only add a playbackRate method in the case where it does support it. I think this cleaner than having an additional variable of featuresPlaybackRate.

For the supported events variables, I think we should make it the tech's responsibility to enable the manualTimeupdateEvents instead of having the MediaTechController enable them if featuresTimeupdateEvents is false.

featuresProgressEvents
featuresTimeupdateEvents

The last piece I'm trying to think through is the case where BOTH native and non-native features are supported and we need to know which is being used.

  • using built-in or custom controls
  • using the native or custom captions display
    These may require additional APIs like usingNativeControls but I'm wondering if there's a cleaner method.

UPDATE 2015-04-01

For native vs custom controls, I'm thinking we should remove to the option to set that through the player api (usingNativeControls), and instead have that only be a getter. A preference could be set in the tech options, updating the tech when it's loaded, but after that it shouldn't change.

The controls API could check if controls:true, or check when initializing the tech and update its own state. Or the state could be moved to the tech.

@heff
Copy link
Member Author

heff commented Aug 23, 2016

Not sure how relevant this still is but was just reading up on feature queries in CSS (@supports) and it made me think of this issue. Might be something we could pull inspiration from.
https://hacks.mozilla.org/2016/08/using-feature-queries-in-css/

@gkatsev gkatsev added the pinned Things that stalebot shouldn't close automatically label Jul 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pinned Things that stalebot shouldn't close automatically
Projects
None yet
Development

No branches or pull requests

3 participants