Skip to content

Commit f6a39fc

Browse files
committed
chore: @xram review changes
1 parent 7a7fdbb commit f6a39fc

File tree

7 files changed

+113
-10
lines changed

7 files changed

+113
-10
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const MyComponent = mock();
3636
- [`invert()`](./docs/invert.md)
3737
- Sensors
3838
- [`<BatterySensor>`](./docs/BatterySensor.md), [`withBattery()`](./docs/BatterySensor.md#withbattery), and [`@withBattery`](./docs/BatterySensor.md#withbattery-1)
39-
- [`<MediaDeviceSensor>`](./docs/MediaDeviceSensor.md), [`withMediaDevices`](./docs/MediaDeviceSensor.md#withmediadevices), and [`@withMediaDevices`](./docs/MediaDeviceSensor.md#withmediadevices-1)
39+
- [`<MediaDeviceSensor>`](./docs/MediaDeviceSensor.md), [`withMediaDevices()`](./docs/MediaDeviceSensor.md#withmediadevices), and [`@withMediaDevices`](./docs/MediaDeviceSensor.md#withmediadevices-1)
4040
- [`<MediaSensor>`](./docs/MediaSensor.md), [`withMedia()`](./docs/MediaSensor.md#withmedia), and [`@withMedia`](./docs/MediaSensor.md#withmedia-1)
4141
- [`<NetworkSensor>`](./docs/NetworkSensor.md) and [`withNetwork()`](./docs/NetworkSensor.md#withnetwork)
4242
- [`<LightSensor>`](./docs/LightSensor.md)

docs/BatterySensor.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const MyCompWithBattery = withBattery(MyComp, null);
5252

5353
## `@withBattery`
5454

55-
Statefull component class decorator that injects `battery` prop into your component.
55+
Stateful component class decorator that injects `battery` prop into your component.
5656

5757
```js
5858
import {withBattery} from 'libreact/lib/BatterySensor';

docs/MediaDeviceSensor.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const MyCompDevices = withMediaDevices(MyComp);
2727

2828
## `@withMediaDevices`
2929

30-
Statefull component class decorator that injects `devices` prop into your component.
30+
Stateful component class decorator that injects `devices` prop into your component.
3131

3232
```js
3333
import {withMediaDevices} from 'libreact/lib/MediaDeviceSensor';

docs/MediaSensor.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const MyCompWithSize = withMedia(MyComp, 'isBigScreen', '(min-width: 480px)');
2626

2727
## `@withMedia`
2828

29-
Statefull component class decorator that injects a boolean prop into your component that specifies if media query matches.
29+
Stateful component class decorator that injects a boolean prop into your component that specifies if media query matches.
3030

3131
```js
3232
import {withMedia} from 'libreact/lib/MediaSensor';

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "libreact",
3-
"version": "0.0.0-development",
3+
"version": "0.4.0",
44
"description": "React standard library",
55
"main": "lib/index.js",
66
"keywords": [

src/Media/__tests__/index.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import {Media} from '..';
2+
3+
const createMedia = () => new Media<any, any, any>({}, {});
4+
5+
describe('<Media>', () => {
6+
describe('play lock', () => {
7+
it('not locked by default', () => {
8+
const media = createMedia();
9+
10+
expect(media.lockPlay).toBe(false);
11+
});
12+
13+
it('calls DOM element play', () => {
14+
const media = createMedia();
15+
media.el = {
16+
play: jest.fn()
17+
} as any;
18+
19+
media.play();
20+
21+
expect(media.el.play).toHaveBeenCalledTimes(1);
22+
});
23+
24+
it('locks play() on call', () => {
25+
const media = createMedia();
26+
const play = jest.fn();
27+
const pause = jest.fn();
28+
29+
play.mockImplementation(() => Promise.resolve(null));
30+
31+
media.el = {
32+
play,
33+
pause
34+
} as any;
35+
36+
expect(media.lockPlay).toBe(false);
37+
38+
media.play();
39+
40+
expect(media.lockPlay).toBe(true);
41+
42+
media.play();
43+
media.play();
44+
45+
media.pause();
46+
media.pause();
47+
48+
expect(play).toHaveBeenCalledTimes(1);
49+
expect(pause).toHaveBeenCalledTimes(0);
50+
});
51+
52+
it('unlocks play() when promise resolves', () => {
53+
const media = createMedia();
54+
const play = jest.fn();
55+
const pause = jest.fn();
56+
57+
play.mockImplementation(() => Promise.resolve(null));
58+
59+
media.el = {
60+
play,
61+
pause
62+
} as any;
63+
64+
media.play();
65+
media.play();
66+
media.pause();
67+
68+
expect(play).toHaveBeenCalledTimes(1);
69+
expect(pause).toHaveBeenCalledTimes(0);
70+
71+
return new Promise((resolve, reject) => {
72+
setImmediate(() => {
73+
try {
74+
media.pause();
75+
media.play();
76+
77+
expect(play).toHaveBeenCalledTimes(2);
78+
expect(pause).toHaveBeenCalledTimes(1);
79+
80+
resolve();
81+
} catch (error) {
82+
reject(error);
83+
}
84+
});
85+
});
86+
});
87+
});
88+
});

src/Media/index.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export interface IMediaState {
6464
volume?: number;
6565
}
6666

67-
export abstract class Media<P extends IMediaProps<M>, S extends IMediaState, M extends IMedia> extends Component<P, S> implements IMedia {
67+
export class Media<P extends IMediaProps<M>, S extends IMediaState, M extends IMedia> extends Component<P, S> implements IMedia {
6868
tag: 'video' | 'audio' = 'video';
6969
props: P;
7070
el: HTMLMediaElement = null;
@@ -100,16 +100,31 @@ export abstract class Media<P extends IMediaProps<M>, S extends IMediaState, M e
100100
this.event('onUnmount')(this);
101101
}
102102

103+
// Some browsers return `Promise` on `.play()` and may throw errors
104+
// if one tries to execute another `.play()` or `.pause()` while that
105+
// promise is resolving. So we prevent that with this lock.
106+
// See: https://bugs.chromium.org/p/chromium/issues/detail?id=593273
107+
lockPlay: boolean = false;
108+
103109
play = () => {
104-
if (this.el) {
105-
// TODO: In some browsers `.play()` method returns a `Promise`, where you
106-
// TODO: cannot call `pause()` or `play()` again before that promise resolves.
110+
if (this.el && !this.lockPlay) {
107111
const promise = this.el.play();
112+
const isPromise = typeof promise === 'object';
113+
114+
if (isPromise) {
115+
this.lockPlay = true;
116+
117+
const resetLock = () => {
118+
this.lockPlay = false;
119+
};
120+
121+
promise.then(resetLock, resetLock);
122+
}
108123
}
109124
};
110125

111126
pause = () => {
112-
if (this.el) {
127+
if (this.el && !this.lockPlay) {
113128
this.el.pause();
114129
}
115130
};

0 commit comments

Comments
 (0)