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.

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

Jump to bottom

Chore: Scrubber/Media fixes for mobile #185

Merged
merged 3 commits into from
Jul 5, 2017
Merged

Chore: Scrubber/Media fixes for mobile #185

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76d5541
Chore: Scrubber/Media fixes for mobile
Jun 26, 2017
39daa8c
Merge branch 'master' into bug-fixes
Jul 5, 2017
cf2035e
Merge branch 'master' into bug-fixes
Jul 5, 2017
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
6 changes: 5 additions & 1 deletion src/lib/viewers/media/MediaBaseViewer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ const EMIT_WAIT_TIME_IN_MILLIS = 100;
this.mediaUrl = this.createContentUrlWithAuthParams(template);
this.mediaEl.addEventListener('loadeddata', this.loadeddataHandler);
this.mediaEl.addEventListener('error', this.errorHandler);
this.mediaEl.setAttribute('title', this.options.file.name);

if (Browser.isIOS()) {
// iOS doesn't fire loadeddata event till some data loads
// iOS doesn't fire loadeddata event until some data loads
// Adding autoplay helps with that and itself won't autoplay.
// https://webkit.org/blog/6784/new-video-policies-for-ios/
this.mediaEl.autoplay = true;
Expand Down Expand Up @@ -155,6 +156,7 @@ const EMIT_WAIT_TIME_IN_MILLIS = 100;
* Handles media element loading errors.
*
* @private
* @param {Error} err - error object
* @emits error
* @return {void}
*/
Expand Down Expand Up @@ -263,6 +265,7 @@ const EMIT_WAIT_TIME_IN_MILLIS = 100;
*
* @private
* @param {double} time - Time in seconds
* @return {void}
*/
setMediaTime(time) {
this.mediaEl.currentTime = time;
Expand All @@ -273,6 +276,7 @@ const EMIT_WAIT_TIME_IN_MILLIS = 100;
*
* @private
* @param {number} volume - Must be a number between [0,1], per HTML5 spec
* @return {void}
*/
setVolume(volume) {
cache.set(MEDIA_VOLUME_CACHE_KEY, volume);
Expand Down
65 changes: 49 additions & 16 deletions src/lib/viewers/media/Scrubber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import autobind from 'autobind-decorator';
import EventEmitter from 'events';
import scrubberTemplate from './Scrubber.html';
import Browser from '../../Browser';
Copy link
Contributor

Choose a reason for hiding this comment

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

is this needed? Didn't see it used anywhere in the code you added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Browser.hasTouch() on line 61


const MIN_VALUE = 0;
const MAX_VALUE = 1;
const CLASS_SCRUBBER_HOVER = 'bp-media-scrubber-hover';
const CLASS_SCRUBBER_TOUCH = 'bp-media-scrubber-touch';

@autobind class Scrubber extends EventEmitter {
/**
Expand Down Expand Up @@ -56,14 +58,21 @@ const CLASS_SCRUBBER_HOVER = 'bp-media-scrubber-hover';
this.playedEl = this.scrubberEl.querySelector('.bp-media-scrubber-played');
this.handleEl = this.scrubberEl.querySelector('.bp-media-scrubber-handle');

this.hasTouch = Browser.hasTouch();

// Set the provided initial values
this.setConvertedValue(convertedValue);
this.setBufferedValue(bufferedValue);
this.setValue(value);

this.playedEl.addEventListener('mousedown', this.mouseDownHandler);
this.convertedEl.addEventListener('mousedown', this.mouseDownHandler);
this.handleEl.addEventListener('mousedown', this.mouseDownHandler);
this.playedEl.addEventListener('mousedown', this.pointerDownHandler);
this.convertedEl.addEventListener('mousedown', this.pointerDownHandler);
this.handleEl.addEventListener('mousedown', this.pointerDownHandler);

if (this.hasTouch) {
this.scrubberContainerEl.addEventListener('touchstart', this.pointerDownHandler);
this.scrubberWrapperEl.classList.add(CLASS_SCRUBBER_TOUCH);
}
}

/**
Expand All @@ -74,9 +83,11 @@ const CLASS_SCRUBBER_HOVER = 'bp-media-scrubber-hover';
destroy() {
this.removeAllListeners();
this.destroyDocumentHandlers();
this.playedEl.removeEventListener('mousedown', this.mouseDownHandler);
this.convertedEl.removeEventListener('mousedown', this.mouseDownHandler);
this.handleEl.removeEventListener('mousedown', this.mouseDownHandler);
this.playedEl.removeEventListener('mousedown', this.pointerDownHandler);
this.convertedEl.removeEventListener('mousedown', this.pointerDownHandler);
this.handleEl.removeEventListener('mousedown', this.pointerDownHandler);
this.scrubberContainerEl.removeEventListener('touchstart', this.pointerDownHandler);

this.scrubberContainerEl = undefined;
this.scrubberWrapperEl = undefined;
this.scrubberEl = undefined;
Expand All @@ -100,8 +111,8 @@ const CLASS_SCRUBBER_HOVER = 'bp-media-scrubber-hover';
/**
* Set aria-valuenow and aria-valuetext attributes
*
* @param {number} ariaValuenow
* @param {string} ariaValuetext
* @param {number} ariaValuenow - value for aria 'aria-valuenow'
* @param {string} ariaValuetext - value for aria 'aria-valuetext'
* @return {void}
*/
setAriaValues(ariaValuenow, ariaValuetext) {
Expand Down Expand Up @@ -191,7 +202,18 @@ const CLASS_SCRUBBER_HOVER = 'bp-media-scrubber-hover';
* @return {void}
*/
scrubbingHandler(event) {
const pageX = event.pageX;
// Stops vertical scrolling when scrubbing
event.preventDefault();

let pageX = event.pageX;

// Android Chrome fires both mousedown events and touchstart events. The touch start event
// does not include pageX, but pageX can be found in the touches list which is present for
// touch events across all browsers.
if (event.touches) {
pageX = event.touches[0].pageX;
}

const newValue = this.computeScrubberPosition(pageX);

this.setValue(newValue);
Expand All @@ -207,19 +229,25 @@ const CLASS_SCRUBBER_HOVER = 'bp-media-scrubber-hover';
* @param {Event} event - the instance of the class
* @return {void}
*/
mouseDownHandler(event) {
pointerDownHandler(event) {
// If this is not a left click, then ignore
// If this is a CTRL or CMD click, then ignore
if ((typeof event.button !== 'number' || event.button < 2) && !event.ctrlKey && !event.metaKey) {
this.scrubbingHandler(event);
// All events are attached to the document so that the user doesn't have to keep the mouse
// over the scrubber bar and has wiggle room. If the wiggling causes the mouse to leave
// the whole view (embed use case) then we stop tracking all events.
document.addEventListener('mouseup', this.mouseUpHandler);
document.addEventListener('mouseleave', this.mouseUpHandler);
document.addEventListener('mouseup', this.pointerUpHandler);
document.addEventListener('mouseleave', this.pointerUpHandler);
document.addEventListener('mousemove', this.scrubbingHandler);

this.scrubberWrapperEl.classList.add(CLASS_SCRUBBER_HOVER);
if (this.hasTouch) {
document.addEventListener('touchmove', this.scrubbingHandler);
document.addEventListener('touchend', this.pointerUpHandler);
} else {
this.scrubberWrapperEl.classList.add(CLASS_SCRUBBER_HOVER);
}

event.preventDefault();
}
}
Expand All @@ -230,7 +258,7 @@ const CLASS_SCRUBBER_HOVER = 'bp-media-scrubber-hover';
* @private
* @return {void}
*/
mouseUpHandler() {
pointerUpHandler() {
this.scrubberWrapperEl.classList.remove(CLASS_SCRUBBER_HOVER);
this.destroyDocumentHandlers();
}
Expand All @@ -242,8 +270,13 @@ const CLASS_SCRUBBER_HOVER = 'bp-media-scrubber-hover';
*/
destroyDocumentHandlers() {
document.removeEventListener('mousemove', this.scrubbingHandler);
document.removeEventListener('mouseup', this.mouseUpHandler);
document.removeEventListener('mouseleave', this.mouseUpHandler);
document.removeEventListener('mouseup', this.pointerUpHandler);
document.removeEventListener('mouseleave', this.pointerUpHandler);

if (this.hasTouch) {
document.removeEventListener('touchmove', this.scrubbingHandler);
document.removeEventListener('touchend', this.pointerUpHandler);
}
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/lib/viewers/media/Scrubber.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,14 @@
.bp-media-scrubber-container .bp-media-scrubber-hover .bp-media-scrubber-underlay {
transform: scaleY(2);
}

.bp-media-scrubber-container .bp-media-scrubber-touch .bp-media-scrubber-handle {
transform: scale(1);
}

.bp-media-scrubber-container .bp-media-scrubber-touch .bp-media-scrubber-converted,
.bp-media-scrubber-container .bp-media-scrubber-touch .bp-media-scrubber-buffered,
.bp-media-scrubber-container .bp-media-scrubber-touch .bp-media-scrubber-played,
.bp-media-scrubber-container .bp-media-scrubber-touch .bp-media-scrubber-underlay {
transform: scaleY(2);
}
65 changes: 53 additions & 12 deletions src/lib/viewers/media/__tests__/Scrubber-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,35 @@ describe('lib/viewers/media/Scrubber', () => {
});

describe('scrubbingHandler()', () => {
it('should adjust the scrubber value to the current scrubber handle position value in the video', () => {
beforeEach(() => {
stubs.setValue = sandbox.stub(scrubber, 'setValue');
stubs.emit = sandbox.stub(scrubber, 'emit');
stubs.scrubberPosition = sandbox.stub(scrubber, 'computeScrubberPosition').returns(0.5);
stubs.event = {
pageX: 50,
preventDefault: sandbox.stub()
};
});
it('should adjust the scrubber value to the current scrubber handle position value in the video', () => {
scrubber.scrubbingHandler(stubs.event);

scrubber.scrubbingHandler({ pageX: 50 });

expect(stubs.event.preventDefault).to.be.called;
expect(stubs.scrubberPosition).to.be.calledWith(50);
expect(stubs.setValue).to.be.calledWith(0.5);
expect(stubs.emit).to.be.calledWith('valuechange');
});

it('should use the touch list if the event contains touches', () => {
stubs.event.touches = [
{
pageX: 55
}
];

scrubber.scrubbingHandler(stubs.event);

expect(stubs.scrubberPosition).to.be.calledWith(55);
});
});

describe('computeScrubberPosition()', () => {
Expand Down Expand Up @@ -167,7 +185,7 @@ describe('lib/viewers/media/Scrubber', () => {
});
});

describe('mouseDownHandler()', () => {
describe('pointerDownHandler()', () => {
beforeEach(() => {
stubs.scrub = sandbox.stub(scrubber, 'scrubbingHandler');
stubs.event = {
Expand All @@ -179,35 +197,48 @@ describe('lib/viewers/media/Scrubber', () => {
});

it('should ignore if event is not a left click', () => {
scrubber.mouseDownHandler(stubs.event);
scrubber.pointerDownHandler(stubs.event);
expect(stubs.scrub).to.not.be.called;
});

it('should ignore if event is a CTRL click', () => {
stubs.event.ctrlKey = '';
scrubber.mouseDownHandler(stubs.event);
scrubber.pointerDownHandler(stubs.event);
expect(stubs.scrub).to.not.be.called;
});

it('should ignore if event is a CMD click', () => {
stubs.event.metaKey = '';
scrubber.mouseDownHandler(stubs.event);
scrubber.pointerDownHandler(stubs.event);
expect(stubs.scrub).to.not.be.called;
});

it('should set the mouse move state to true and calls the mouse action handler', () => {
scrubber.hasTouch = false;
stubs.event.button = 1;
scrubber.mouseDownHandler(stubs.event);
scrubber.pointerDownHandler(stubs.event);

expect(stubs.scrub).to.be.calledWith(stubs.event);
expect(scrubber.scrubberWrapperEl).to.have.class(CLASS_SCRUBBER_HOVER);
});

it('should add touch events if the browser has touch', () => {
stubs.event.button = 1;
scrubber.hasTouch = true;
stubs.addEventListener = sandbox.stub(document, 'addEventListener');

scrubber.pointerDownHandler(stubs.event);

expect(stubs.addEventListener).to.be.calledWith('touchmove', scrubber.scrubbingHandler);
expect(stubs.addEventListener).to.be.calledWith('touchend', scrubber.pointerUpHandler);
expect(scrubber.scrubberWrapperEl).to.not.have.class(CLASS_SCRUBBER_HOVER);
});
});

describe('mouseUpHandler()', () => {
describe('pointerUpHandler()', () => {
it('should set the mouse move state to false thus stopping mouse action handling', () => {
stubs.destroy = sandbox.stub(scrubber, 'destroyDocumentHandlers');
scrubber.mouseUpHandler(stubs.event);
scrubber.pointerUpHandler(stubs.event);
expect(stubs.destroy).to.be.called;
expect(scrubber.scrubberWrapperEl).to.not.have.class(CLASS_SCRUBBER_HOVER);
});
Expand All @@ -219,8 +250,18 @@ describe('lib/viewers/media/Scrubber', () => {
scrubber.destroyDocumentHandlers();

expect(stubs.remove).to.be.calledWith('mousemove', scrubber.scrubbingHandler);
expect(stubs.remove).to.be.calledWith('mouseup', scrubber.mouseUpHandler);
expect(stubs.remove).to.be.calledWith('mouseleave', scrubber.mouseUpHandler);
expect(stubs.remove).to.be.calledWith('mouseup', scrubber.pointerUpHandler);
expect(stubs.remove).to.be.calledWith('mouseleave', scrubber.pointerUpHandler);
});

it('should remove touch events if the browser has touch', () => {
scrubber.hasTouch = true;
stubs.remove = sandbox.stub(document, 'removeEventListener');

scrubber.destroyDocumentHandlers();

expect(stubs.remove).to.be.calledWith('touchmove', scrubber.scrubbingHandler);
expect(stubs.remove).to.be.calledWith('touchend', scrubber.pointerUpHandler);
});
});

Expand Down
1 change: 1 addition & 0 deletions src/lib/viewers/media/_mediaBase.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
.bp-media-container {
outline: 0 none;
position: relative;
user-select: none; // Prevents copy paste dialog from appearing on mobile
}

.bp-media-play-button {
Expand Down