Skip to content

Bugfix/use media capabilities #51

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.

Sign up for GitHub

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

Closed
wants to merge 10 commits into from
Closed

Bugfix/use media capabilities #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
371d010
doc: improved code styling in readme
anton-karlovskiy Mar 11, 2020
9940103
infra: uninstalled react-dom
anton-karlovskiy Mar 11, 2020
f64860d
chore: cleaned up
anton-karlovskiy Mar 11, 2020
06c2a0a
infra: extracted supported from hook in useMediaCapabilities
anton-karlovskiy Mar 11, 2020
def7edd
doc: cleaned code snippet for useMediaCapabilities in README
anton-karlovskiy Mar 13, 2020
5f7a6b4
Merge remote-tracking branch 'upstream/master'
anton-karlovskiy Mar 13, 2020
c232111
[infra] updated naming for media capabilities hook considering its co…
anton-karlovskiy Mar 16, 2020
aa6fa12
[doc] described mediaCapabilitiesInfo value
anton-karlovskiy Mar 16, 2020
1485417
fix: typo
anton-karlovskiy Mar 18, 2020
fdb2c3d
doc: useMediaCapabilitiesDecodingInfo added in UMD section
anton-karlovskiy Mar 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { useNetworkStatus } from 'react-adaptive-hooks/network';
import { useSaveData } from 'react-adaptive-hooks/save-data';
import { useHardwareConcurrency } from 'react-adaptive-hooks/hardware-concurrency';
import { useMemoryStatus } from 'react-adaptive-hooks/memory';
import { useMediaCapabilities } from 'react-adaptive-hooks/media-capabilities';
import { useMediaCapabilitiesDecodingInfo } from 'react-adaptive-hooks/media-capabilities';
```

and then use them in your components. Examples for each hook and utility can be found below:
Expand Down Expand Up @@ -160,16 +160,16 @@ const { deviceMemory } = useMemoryStatus(initialMemoryStatus);

### Media Capabilities

`useMediaCapabilities` utility for adapting based on the user's device media capabilities.
`useMediaCapabilitiesDecodingInfo` utility for adapting based on the user's device media capabilities.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wingleung cc @addyosmani

Looking into the implementation of this hook, I found that it just deals with decoding media capabilities, not all media capabilities (encoding and decoding). So I'd like to suggest updating its name to useMediaCapabilitiesDecodingInfo.
I've got inspired by this MediaCapabilities.decodingInfo().


**Use case:** this hook can be used to check if we can play a certain content type. For example, Safari does not support WebM so we want to fallback to MP4 but if Safari at some point does support WebM it will automatically load WebM videos.

```js
import React from 'react';

import { useMediaCapabilities } from 'react-adaptive-hooks/media-capabilities';
import { useMediaCapabilitiesDecodingInfo } from 'react-adaptive-hooks/media-capabilities';

const webmMediaConfig = {
const webmMediaDecodingConfig = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wingleung cc @addyosmani

According to MediaCapabilities.decodingInfo(), the argument type of mediaCapabilities.decodingInfo(MediaDecodingConfiguration) is MediaDecodingConfiguration.
So I'd like to suggest updating to webmMediaDecodingConfig.

type: 'file', // 'record', 'transmission', or 'media-source'
video: {
contentType: 'video/webm;codecs=vp8', // valid content type
Expand All @@ -180,15 +180,15 @@ const webmMediaConfig = {
}
};

const initialMediaCapabilities = {showWarning: true};
const initialMediaCapabilitiesInfo = {showWarning: true};

const MyComponent = ({ videoSources }) => {
const { mediaCapabilities } = useMediaCapabilities(webmMediaConfig, initialMediaCapabilities);
const { mediaCapabilitiesInfo } = useMediaCapabilitiesDecodingInfo(webmMediaDecodingConfig, initialMediaCapabilitiesInfo);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wingleung cc @addyosmani

According to MediaCapabilities.decodingInfo(), the return value of mediaCapabilities.decodingInfo(MediaDecodingConfiguration) is a promise fulfilling with a MediaCapabilitiesInfo interface containing three Boolean attributes: supported, smooth, and powerEfficient.
So I'd like to suggest updating to mediaCapabilitiesInfo and initialMediaCapabilitiesInfo.

Copy link
Contributor

Choose a reason for hiding this comment

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

the hook itself will return an object that will contain the 3 booleans, as mentioned in https://github.com/GoogleChromeLabs/react-adaptive-hooks/pull/51/files?file-filters%5B%5D=.js&file-filters%5B%5D=.md#r393336802


return (
<div>
<video src={mediaCapabilities.supported ? videoSources.webm : videoSources.mp4} controls>...</video>
{ mediaCapabilities.showWarning && (
<video src={mediaCapabilitiesInfo.supported ? videoSources.webm : videoSources.mp4} controls>...</video>
{ mediaCapabilitiesInfo.showWarning && (
<div class='muted'>
Defaulted to mp4.
Couldn't test webm support,
Expand All @@ -200,7 +200,9 @@ const MyComponent = ({ videoSources }) => {
};
```

This hook accepts a [media configuration](https://developer.mozilla.org/en-US/docs/Web/API/MediaConfiguration) object argument and an optional `initialMediaCapabilities` object argument, which can be used to provide a `mediaCapabilities` state value when the user's browser does not support the relevant [Media Capabilities API](https://developer.mozilla.org/en-US/docs/Web/API/Media_Capabilities_API) or no media configuration was given.
`mediaCapabilitiesInfo` value contains the three Boolean properties `supported`, `smooth`, and `powerEfficient`, which describe whether decoding the media described would be supported, smooth, and powerefficient.

This utility accepts a [MediaDecodingConfiguration](https://developer.mozilla.org/en-US/docs/Web/API/MediaDecodingConfiguration) object argument and an optional `initialMediaCapabilitiesInfo` object argument, which can be used to provide a `mediaCapabilitiesInfo` state value when the user's browser does not support the relevant [Media Capabilities API](https://developer.mozilla.org/en-US/docs/Web/API/Media_Capabilities_API) or no media configuration was given.
Copy link
Contributor Author

@anton-karlovskiy anton-karlovskiy Mar 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@wingleung cc @addyosmani

According to MediaCapabilities.decodingInfo():

A TypeError is raised if the MediaConfiguration passed to the decodingInfo() method is invalid, either because the type is not video or audio, the contentType is not a valid codec MIMME type, the media decoding configuration is not a valid value for the media decoding type, or any other error in the media configuration passed to the method, including omitting values required in the media decoding configuration.

From MediaCapabilities.decodingInfo()


### Adaptive Code-loading & Code-splitting

Expand Down Expand Up @@ -304,7 +306,8 @@ import {
useNetworkStatus,
useSaveData,
useHardwareConcurrency,
useMemoryStatus
useMemoryStatus,
useMediaCapabilitiesDecodingInfo
} from 'react-adaptive-hooks/dist/index.umd.js';
```

Expand Down
2 changes: 1 addition & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ export { useNetworkStatus } from './network';
export { useSaveData } from './save-data';
export { useMemoryStatus } from './memory';
export { useHardwareConcurrency } from './hardware-concurrency';
export { useMediaCapabilities } from './media-capabilities';
export { useMediaCapabilitiesDecodingInfo } from './media-capabilities';
18 changes: 9 additions & 9 deletions media-capabilities/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,22 @@

const supported = typeof window !== 'undefined' && 'mediaCapabilities' in navigator;

const useMediaCapabilities = (mediaConfig, initialMediaCapabilities = {}) => {
let mediaCapabilities = {
const useMediaCapabilitiesDecodingInfo = (mediaDecodingConfig, initialMediaCapabilitiesInfo = {}) => {
let mediaCapabilitiesInfo = {
supported,
hasMediaConfig: !!mediaConfig
hasMediaDecodingConfig: !!mediaDecodingConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the point of this? This seems like it should just be an internal variable.

Copy link
Contributor Author

@anton-karlovskiy anton-karlovskiy Mar 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@wingleung cc @addyosmani

I agree with @stramel.
I guess try/catch block should wrap navigator.mediaCapabilities.decodingInfo(mediaDecodingConfig) for handling exceptions without hasMediaDecodingConfig.

A TypeError is raised if the MediaConfiguration passed to the decodingInfo() method is invalid, either because the type is not video or audio, the contentType is not a valid codec MIMME type, the media decoding configuration is not a valid value for the media decoding type, or any other error in the media configuration passed to the method, including omitting values required in the media decoding configuration.

From MediaCapabilities.decodingInfo()

};

mediaCapabilities = (mediaCapabilities.supported && mediaCapabilities.hasMediaConfig)
? navigator.mediaCapabilities.decodingInfo(mediaConfig)
mediaCapabilitiesInfo = (mediaCapabilitiesInfo.supported && mediaCapabilitiesInfo.hasMediaDecodingConfig)
? navigator.mediaCapabilities.decodingInfo(mediaDecodingConfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

This still doesn't take into account the fact this returns a promise.

Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT about using an API like,

const { supported, decodingInfo, encodingInfo } = useMediaCapabilities(initialState)

either that or we need to wrap the async calls and expose them to show the state of the calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wingleung cc @addyosmani

What are your thoughts?
I agree with @stramel's API suggestion. I think it looks more descriptive rather than just mediaCapabilitiesInfo.
For now, I think it could be like so.

const { supported, smooth, powerEfficient } = useMediaCapabilities(initialState);

: {
...mediaCapabilities,
...initialMediaCapabilities
...mediaCapabilitiesInfo,
...initialMediaCapabilitiesInfo
};

return {mediaCapabilities};
return {mediaCapabilitiesInfo};
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be {...mediaCapabilitiesInfo}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wingleung cc @addyosmani

I agree with @stramel.

};

export {
useMediaCapabilities
useMediaCapabilitiesDecodingInfo
};
50 changes: 25 additions & 25 deletions media-capabilities/media-capabilities.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wingleung cc @addyosmani

Just in the scope of naming I'd like to suggest updating like the following.

import { renderHook } from '@testing-library/react-hooks';

const mediaConfig = {
const mediaDecodingConfig = {
type: 'file',
audio: {
contentType: 'audio/mp3',
Expand All @@ -39,65 +39,65 @@ afterEach(function() {
jest.resetModules();
});

describe('useMediaCapabilities', () => {
describe('useMediaCapabilitiesDecodingInfo', () => {
const navigator = window.navigator;

afterEach(() => {
if (!window.navigator) window.navigator = navigator;
});

test('should return supported flag on unsupported platforms', () => {
const { useMediaCapabilities } = require('./');
const { result } = renderHook(() => useMediaCapabilities(mediaConfig));
const { useMediaCapabilitiesDecodingInfo } = require('./');
const { result } = renderHook(() => useMediaCapabilitiesDecodingInfo(mediaDecodingConfig));

expect(result.current.mediaCapabilities).toEqual({hasMediaConfig: true, supported: false});
expect(result.current.mediaCapabilitiesInfo).toEqual({hasMediaDecodingConfig: true, supported: false});
});

test('should return supported and hasMediaConfig flags on unsupported platforms and no config given', () => {
const { useMediaCapabilities } = require('./');
const { result } = renderHook(() => useMediaCapabilities());
test('should return supported and hasMediaDecodingConfig flags on unsupported platforms and no config given', () => {
const { useMediaCapabilitiesDecodingInfo } = require('./');
const { result } = renderHook(() => useMediaCapabilitiesDecodingInfo());

expect(result.current.mediaCapabilities).toEqual({hasMediaConfig: false, supported: false});
expect(result.current.mediaCapabilitiesInfo).toEqual({hasMediaDecodingConfig: false, supported: false});
});

test('should return initialMediaCapabilities for unsupported', () => {
const initialMediaCapabilities = {
test('should return initialMediaCapabilitiesInfo for unsupported', () => {
const initialMediaCapabilitiesInfo = {
supported: true,
smooth: false,
powerEfficient: true
};
const { useMediaCapabilities } = require('./');
const { result } = renderHook(() => useMediaCapabilities(mediaConfig, initialMediaCapabilities));
const { useMediaCapabilitiesDecodingInfo } = require('./');
const { result } = renderHook(() => useMediaCapabilitiesDecodingInfo(mediaDecodingConfig, initialMediaCapabilitiesInfo));

expect(result.current.mediaCapabilities.supported).toBe(true);
expect(result.current.mediaCapabilities.smooth).toEqual(false);
expect(result.current.mediaCapabilities.powerEfficient).toEqual(true);
expect(result.current.mediaCapabilitiesInfo.supported).toBe(true);
expect(result.current.mediaCapabilitiesInfo.smooth).toEqual(false);
expect(result.current.mediaCapabilitiesInfo.powerEfficient).toEqual(true);
});

test('should return hasMediaConfig flag when no config given', () => {
test('should return hasMediaDecodingConfig flag when no config given', () => {
Object.defineProperty(window.navigator, 'mediaCapabilities', {
value: true,
configurable: true,
writable: true
});
const { useMediaCapabilities } = require('./');
const { result } = renderHook(() => useMediaCapabilities());
const { useMediaCapabilitiesDecodingInfo } = require('./');
const { result } = renderHook(() => useMediaCapabilitiesDecodingInfo());

expect(result.current.mediaCapabilities).toEqual({hasMediaConfig: false, supported: true});
expect(result.current.mediaCapabilitiesInfo).toEqual({hasMediaDecodingConfig: false, supported: true});
});

test('should return MediaDecodingConfiguration for given media configuration', () => {
test('should return MediaCapabilitiesInfo for given media configuration', () => {
Object.defineProperty(window.navigator, 'mediaCapabilities', {
value: {
decodingInfo: mediaConfig => mediaCapabilitiesMapper[mediaConfig.audio.contentType]
decodingInfo: mediaDecodingConfig => mediaCapabilitiesMapper[mediaDecodingConfig.audio.contentType]
},
configurable: true,
writable: true
});
const { useMediaCapabilities } = require('./');
const { result } = renderHook(() => useMediaCapabilities(mediaConfig));
const { useMediaCapabilitiesDecodingInfo } = require('./');
const { result } = renderHook(() => useMediaCapabilitiesDecodingInfo(mediaDecodingConfig));

expect(result.current.mediaCapabilities).toEqual({
expect(result.current.mediaCapabilitiesInfo).toEqual({
powerEfficient: true,
smooth: true,
supported: true
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@
"react",
"performance"
]
}
}