Skip to content

Commit

Permalink
Fix: fullscreen controls not shown in IE11 (#588)
Browse files Browse the repository at this point in the history
* Fix: fullscreen controls not shown in IE11

* Fix: Add UT for Fullscreen.js fullscreenchange

Slight refactor of Fullscreen.js to have separate methods for binding and unbinding events for testing purposes as spies werent working. Also added destroy function as events werent being unbound.
  • Loading branch information
DanDeMicco authored Jan 17, 2018
1 parent 2b6fa14 commit e3d0f5c
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 13 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"fetch-mock": "^5.13.1",
"fetch-mock-forwarder": "^1.0.0",
"file-loader": "^1.1.5",
"fscreen": "^1.0.2",
"husky": "^0.14.3",
"i18n-webpack-plugin": "^1.0.0",
"jsuri": "^1.3.1",
Expand Down
42 changes: 38 additions & 4 deletions src/lib/Fullscreen.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import EventEmitter from 'events';
import fscreen from 'fscreen';

import { CLASS_FULLSCREEN } from './constants';

class Fullscreen extends EventEmitter {
Expand All @@ -10,10 +12,42 @@ class Fullscreen extends EventEmitter {
constructor() {
super();

document.addEventListener('webkitfullscreenchange', this.fullscreenchangeHandler);
document.addEventListener('mozfullscreenchange', this.fullscreenchangeHandler);
document.addEventListener('MSFullscreenChange', this.fullscreenchangeHandler);
document.addEventListener('fullscreenchange', this.fullscreenchangeHandler);
this.bindDOMListeners();
}

/**
* Binds DOM listeners for Fullscreen.
*
* @protected
* @return {void}
*/
bindDOMListeners() {
// as of now (1/12/18) fullscreenchange is not universally adopted, fscreen will
// detect and add the appropriate vendor prefixed event
fscreen.addEventListener('fullscreenchange', this.fullscreenchangeHandler);
}

/**
* Binds DOM listeners for Fullscreen.
*
* @protected
* @return {void}
*/
unbindDOMListeners() {
// as of now (1/12/18) fullscreenchange is not universally adopted, fscreen will
// detect and add the appropriate vendor prefixed event
fscreen.removeEventListener('fullscreenchange', this.fullscreenchangeHandler);
}

/**
* [destructor]
*
* @protected
* @return {void}
*/
destroy() {
this.unbindDOMListeners();
this.removeAllListeners();
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/lib/__tests__/Fullscreen-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@ describe('lib/Fullscreen', () => {

expect(fullscreen.emit).to.have.been.calledWith('exit');
});

it('should be called only once when the fullscreenchange event is emitted', () => {
const spy = sandbox.spy(fullscreen, 'fullscreenchangeHandler');
//rebind the dom listeners to use the spy
fullscreen.bindDOMListeners();

const event = new Event('webkitfullscreenchange');

window.document.dispatchEvent(event);
expect(spy).to.be.called.once;
});
});

describe('toggle()', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class BaseViewer extends EventEmitter {
});
}

fullscreen.removeAllListeners();
fullscreen.destroy();
document.defaultView.removeEventListener('resize', this.debouncedResizeHandler);
this.removeAllListeners();

Expand Down
8 changes: 0 additions & 8 deletions src/lib/viewers/media/Dash.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@
}
}

.bp-is-fullscreen {
.bp-media-dash {
.bp-media-container {
width: 100%;
}
}
}

.bp-media-filmstrip-container {
border: 1px solid $sunset-grey;
bottom: 60px;
Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3300,6 +3300,10 @@ fs.realpath@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/fs.realpath/-/fs.realpath-1.0.0.tgz#1504ad2523158caa40db4a2787cb01411994ea4f"

fscreen@^1.0.2:
version "1.0.2"
resolved "https://registry.yarnpkg.com/fscreen/-/fscreen-1.0.2.tgz#c4c51d96d819d75a19d728e0df445f9be9bb984f"

fsevents@^1.0.0:
version "1.1.2"
resolved "https://registry.yarnpkg.com/fsevents/-/fsevents-1.1.2.tgz#3282b713fb3ad80ede0e9fcf4611b5aa6fc033f4"
Expand Down

0 comments on commit e3d0f5c

Please sign in to comment.