From c4c8fc1caf780fb2351c6f9d662c845dc1b9f0d4 Mon Sep 17 00:00:00 2001 From: Samuel Burbano Date: Mon, 31 Aug 2020 11:29:25 -0500 Subject: [PATCH] fix: Better mouse position handling (#5773) 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. --- .../progress-control/progress-control.js | 2 +- .../progress-control/time-tooltip.js | 2 +- src/js/utils/dom.js | 58 +++++++++---------- test/unit/utils/dom.test.js | 15 +++-- 4 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/js/control-bar/progress-control/progress-control.js b/src/js/control-bar/progress-control/progress-control.js index b0c943a840..3d15307681 100644 --- a/src/js/control-bar/progress-control/progress-control.js +++ b/src/js/control-bar/progress-control/progress-control.js @@ -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 diff --git a/src/js/control-bar/progress-control/time-tooltip.js b/src/js/control-bar/progress-control/time-tooltip.js index 402320647f..0ba497ebb3 100644 --- a/src/js/control-bar/progress-control/time-tooltip.js +++ b/src/js/control-bar/progress-control/time-tooltip.js @@ -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; diff --git a/src/js/utils/dom.js b/src/js/utils/dom.js index 8fbac22be9..e29ecef0d9 100644 --- a/src/js/utils/dom.js +++ b/src/js/utils/dom.js @@ -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 }; } @@ -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; } diff --git a/test/unit/utils/dom.test.js b/test/unit/utils/dom.test.js index 420147f31a..3e2e4a7205 100644 --- a/test/unit/utils/dom.test.js +++ b/test/unit/utils/dom.test.js @@ -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' ); });