Skip to content

Commit

Permalink
fix: Better mouse position handling (#5773)
Browse files Browse the repository at this point in the history
This uses offsetX and offsetY on the MouseEvents which helps account for transforms on the player. Unfortunately, this isn't available on TouchEvents, so, while this helps desktop devices with using a mouse, it doesn't help mobile devices using touch.

Fixes #6726, fixes #1102.
  • Loading branch information
iosamuel authored and gkatsev committed Aug 31, 2020
1 parent c64239f commit c4c8fc1
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 39 deletions.
2 changes: 1 addition & 1 deletion src/js/control-bar/progress-control/progress-control.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class ProgressControl extends Component {
}

const seekBarEl = seekBar.el();
const seekBarRect = Dom.getBoundingClientRect(seekBarEl);
const seekBarRect = Dom.findPosition(seekBarEl);
let seekBarPoint = Dom.getPointerPosition(seekBarEl, event).x;

// The default skin has a gap on either side of the `SeekBar`. This means
Expand Down
2 changes: 1 addition & 1 deletion src/js/control-bar/progress-control/time-tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class TimeTooltip extends Component {
* from the left edge of the {@link SeekBar}
*/
update(seekBarRect, seekBarPoint, content) {
const tooltipRect = Dom.getBoundingClientRect(this.el_);
const tooltipRect = Dom.findPosition(this.el_);
const playerRect = Dom.getBoundingClientRect(this.player_.el());
const seekBarPointPx = seekBarRect.width * seekBarPoint;

Expand Down
58 changes: 26 additions & 32 deletions src/js/utils/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -551,34 +551,31 @@ export function getBoundingClientRect(el) {
* The position of the element that was passed in.
*/
export function findPosition(el) {
let box;

if (el.getBoundingClientRect && el.parentNode) {
box = el.getBoundingClientRect();
}

if (!box) {
if (!el || (el && !el.offsetParent)) {
return {
left: 0,
top: 0
top: 0,
width: 0,
height: 0
};
}
const width = el.offsetWidth;
const height = el.offsetHeight;
let left = 0;
let top = 0;

const docEl = document.documentElement;
const body = document.body;
do {
left += el.offsetLeft;
top += el.offsetTop;

const clientLeft = docEl.clientLeft || body.clientLeft || 0;
const scrollLeft = window.pageXOffset || body.scrollLeft;
const left = box.left + scrollLeft - clientLeft;
el = el.offsetParent;
} while (el);

const clientTop = docEl.clientTop || body.clientTop || 0;
const scrollTop = window.pageYOffset || body.scrollTop;
const top = box.top + scrollTop - clientTop;

// Android sometimes returns slightly off decimal values, so need to round
return {
left: Math.round(left),
top: Math.round(top)
left,
top,
width,
height

This comment has been minimized.

Copy link
@phloxic

phloxic Sep 11, 2020

Contributor

Shouldn't width and height added here as well?

};
}

Expand Down Expand Up @@ -611,23 +608,20 @@ export function findPosition(el) {
*/
export function getPointerPosition(el, event) {
const position = {};
const boxTarget = findPosition(event.target);
const box = findPosition(el);
const boxW = el.offsetWidth;
const boxH = el.offsetHeight;

const boxY = box.top;
const boxX = box.left;
let pageY = event.pageY;
let pageX = event.pageX;
const boxW = box.width;
const boxH = box.height;
let offsetY = event.offsetY - (box.top - boxTarget.top);
let offsetX = event.offsetX - (box.left - boxTarget.left);

if (event.changedTouches) {
pageX = event.changedTouches[0].pageX;
pageY = event.changedTouches[0].pageY;
offsetX = event.changedTouches[0].pageX - box.left;
offsetY = event.changedTouches[0].pageY + box.top;
}

position.y = Math.max(0, Math.min(1, ((boxY - pageY) + boxH) / boxH));
position.x = Math.max(0, Math.min(1, (pageX - boxX) / boxW));

position.y = Math.max(0, Math.min(1, (offsetY + boxH) / boxH));
position.x = Math.max(0, Math.min(1, offsetX / boxW));
return position;
}

Expand Down
15 changes: 10 additions & 5 deletions test/unit/utils/dom.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,26 +286,31 @@ QUnit.test('Dom.findPosition should find top and left position', function(assert
const d = document.createElement('div');
let position = Dom.findPosition(d);

d.style.width = '100px';
d.style.height = '50px';
d.style.top = '10px';
d.style.left = '20px';
d.style.position = 'absolute';

assert.deepEqual(
position,
{left: 0, top: 0},
{left: 0, top: 0, width: 0, height: 0},
'If element isn\'t in the DOM, we should get zeros'
);

document.body.appendChild(d);
position = Dom.findPosition(d);
assert.deepEqual(position, {left: 20, top: 10}, 'The position was not correct');
assert.deepEqual(position.left, 20, 'The position left was not correct');
assert.deepEqual(position.top, 10, 'The position top was not correct');
assert.deepEqual(position.width, 100, 'The dimension width was not correct');
assert.deepEqual(position.height, 50, 'The dimension height was not correct');

d.getBoundingClientRect = null;
d.style.display = 'none';
position = Dom.findPosition(d);
assert.deepEqual(
position,
{left: 0, top: 0},
'If there is no gBCR, we should get zeros'
{left: 0, top: 0, width: 0, height: 0},
'If there is no offsetParent, we should get zeros'
);
});

Expand Down

0 comments on commit c4c8fc1

Please # to comment.