Skip to content

Commit

Permalink
Fix(Draggable): compensate for container CSS scale (Leaflet#6055)
Browse files Browse the repository at this point in the history 
* Test(MapDragSpec): add mouse drag with CSS scaled container

This test is failing as of this commit.

* Fix(Draggable): measure drag compensating for CSS scale

so that the computed offset (i.e. user drag length) is divided by the CSS scale applied on the `_element`'s container. Later on when `Draggable` updates the `_element`'s position, the latter is affected by the CSS scale by the browser.
Added a `getSizedParentNode` function in `DomUtil` in order to automatically retrieve the closest parent node in the DOM hierarchy tree that has a non null size (so that we can compute the scale in `DomEvent.getMousePosition`), without having to specify the parent node explicitly (which is ugly).

* Fix(getMousePosition): getBoundingClientRect is in page scale

therefore it must also be divided by the container scale in order to compute the relative position of the event in the scaled container.

* Test(MarkerDragSpec): add drag with CSS scaled container

this should pass automatically at this commit, thanks to the previous modification of Draggable.

* Debug(map-scaled): add draggable marker

as well as another CSS scaled wrapper, plus some border and padding, and debugging console logs, to make sure the computations of `getMousePosition` are correct (correctly removing the border width, and compensating for CSS scale).

* Docs(getMousePosition): explicitly exclude border

* Docs(DomUtil): add return value of getSizedParentNode

was missing in docstring.

* Feat(DomUtil): add getScale function

so that it can be used by DomEvent.getMousePosition and Draggable.

* Refactor(Draggable): cache container scale

to avoid triggering browser reflow continuously during _onMove, but only on drag start (_onDown).
By compensating for the scale within Draggable instead of delegating to DomEvent.getMousePosition, it also becomes possible to check first for the clickTolerance, so that we prevents user's small movement based on screen, irrespective of rendered scale.

* Test(Map+MarkerDrag): remove scale on initial movement

that is intended to overcome Draggable clickTolerance, since now it is based on screen pixels, irrespective of applied CSS scale on the map.
Thanks to the tolerance in the expected final position, this has no effect on the result of the test, even though the overall drag is now slightly shorter.

* Refactor(DomEvent): use DomUtil.getScale in getMousePosition

to factorize scale computation (also used within Draggable).
  • Loading branch information
@ghybs
ghybs authored Mar 16, 2018
1 parent c49833d commit fe9e0f2
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 11 deletions.
31 changes: 29 additions & 2 deletions debug/map/map-scaled.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,31 @@
<link rel="stylesheet" href="../css/screen.css" />

<style>
html, body {
margin: 0;
padding: 0;
}
#wrapper {
transform: scale(.5, .25); /* scaleX0 = .5 ; scaleY0 = .25 */
transform-origin: 0 0;
padding: 40px 100px; /* displayed padding-top = scaleY0 * 40px = 10px ; displayed padding-left = scaleX0 * 100px = 50px */
}
#map {
width: 400px;
height: 300px;
transform: scale(1.5, 1.5);
transform: scale(3, 8); /* scaleX = .5 * 3 = 1.5 ; scaleY = .25 * 8 = 2 */
transform-origin: 0 0;
border-width: 30px 70px; /* displayed border-top-width = scaleY * 30px = 60px ; displayed border-left-width = scaleX * 70px = 105px */
}
</style>

<script src="../leaflet-include.js"></script>
</head>
<body>

<div id="map"></div>
<div id="wrapper">
<div id="map"></div>
</div>

<script>

Expand All @@ -33,6 +45,21 @@
var map = L.map('map')
.setView([50.5, 30.51], 15)
.addLayer(osm);

var mapContainer = map.getContainer();

var marker = L.marker([50.5, 30.51], {
draggable: true
}).addTo(map);

map.on('drag', function (event) {
console.log('map:');
console.log(L.DomEvent.getMousePosition(event.originalEvent, mapContainer));
});
marker.on('drag', function (event) {
console.log('marker:');
console.log(L.DomEvent.getMousePosition(event.originalEvent, mapContainer));
});
</script>
</body>
</html>
44 changes: 44 additions & 0 deletions spec/suites/layer/marker/Marker.DragSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,50 @@ describe("Marker.Drag", function () {
.down().moveBy(5, 0, 20).moveBy(256, 32, 1000).wait(100).up().wait(100);
});

describe("in CSS scaled container", function () {
var scaleX = 2;
var scaleY = 1.5;

beforeEach(function () {
div.style.webkitTransformOrigin = 'top left';
div.style.webkitTransform = 'scale(' + scaleX + ', ' + scaleY + ')';
});

afterEach(function () {
div.style.webkitTransformOrigin = '';
div.style.webkitTransform = '';
});

it("drags a marker with mouse, compensating for CSS scale", function (done) {
var marker = new L.Marker([0, 0], {
draggable: true
});
map.addLayer(marker);

var hand = new Hand({
timing: 'fastframe',
onStop: function () {
var center = map.getCenter();
expect(center.lat).to.be(0);
expect(center.lng).to.be(0);

var markerPos = marker.getLatLng();
// Marker drag is very timing sensitive, so we can't check
// exact values here, just verify that the drag is in the
// right ballpark
expect(markerPos.lat).to.be.within(-50, -30);
expect(markerPos.lng).to.be.within(340, 380);

done();
}
});
var toucher = hand.growFinger('mouse');

toucher.wait(100).moveTo(scaleX * 300, scaleY * 280, 0)
.down().moveBy(5, 0, 20).moveBy(scaleX * 256, scaleY * 32, 1000).wait(100).up().wait(100);
});
});

it("pans map when autoPan is enabled", function (done) {
var marker = new L.Marker([0, 0], {
draggable: true,
Expand Down
42 changes: 42 additions & 0 deletions spec/suites/map/handler/Map.DragSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,48 @@ describe("Map.Drag", function () {
.down().moveBy(5, 0, 20).moveBy(256, 32, 200).up();
});

describe("in CSS scaled container", function () {
var scaleX = 2;
var scaleY = 1.5;

beforeEach(function () {
container.style.webkitTransformOrigin = 'top left';
container.style.webkitTransform = 'scale(' + scaleX + ', ' + scaleY + ')';
});

afterEach(function () {
container.style.webkitTransformOrigin = '';
container.style.webkitTransform = '';
});

it("change the center of the map, compensating for CSS scale", function (done) {
var map = new L.Map(container, {
dragging: true,
inertia: false
});
map.setView([0, 0], 1);

var hand = new Hand({
timing: 'fastframe',
onStop: function () {
var center = map.getCenter();
var zoom = map.getZoom();
expect(center.lat).to.be.within(21.9430, 21.9431);
expect(center.lng).to.be(-180);
expect(zoom).to.be(1);

done();
}
});
var mouse = hand.growFinger('mouse');

// We move 5 pixels first to overcome the 3-pixel threshold of
// L.Draggable.
mouse.wait(100).moveTo(200, 200, 0)
.down().moveBy(5, 0, 20).moveBy(scaleX * 256, scaleY * 32, 200).up();
});
});

it("does not change the center of the map when mouse is moved less than the drag threshold", function (done) {
var map = new L.Map(container, {
dragging: true,
Expand Down
15 changes: 9 additions & 6 deletions src/dom/DomEvent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as Util from '../core/Util';
import * as Browser from '../core/Browser';
import {addPointerListener, removePointerListener} from './DomEvent.Pointer';
import {addDoubleTapListener, removeDoubleTapListener} from './DomEvent.DoubleTap';
import {getScale} from './DomUtil';

/*
* @namespace DomEvent
Expand Down Expand Up @@ -214,19 +215,21 @@ export function stop(e) {

// @function getMousePosition(ev: DOMEvent, container?: HTMLElement): Point
// Gets normalized mouse position from a DOM event relative to the
// `container` or to the whole page if not specified.
// `container` (border excluded) or to the whole page if not specified.
export function getMousePosition(e, container) {
if (!container) {
return new Point(e.clientX, e.clientY);
}

var rect = container.getBoundingClientRect();
var scale = getScale(container),
offset = scale.boundingClientRect; // left and top values are in page scale (like the event clientX/Y)

var scaleX = rect.width / container.offsetWidth || 1;
var scaleY = rect.height / container.offsetHeight || 1;
return new Point(
e.clientX / scaleX - rect.left - container.clientLeft,
e.clientY / scaleY - rect.top - container.clientTop);
// offset.left/top values are in page scale (like clientX/Y),
// whereas clientLeft/Top (border width) values are the original values (before CSS scale applies).
(e.clientX - offset.left) / scale.x - container.clientLeft,
(e.clientY - offset.top) / scale.y - container.clientTop
);
}

// Chrome on Win scrolls double the pixels as in other platforms (see #4538),
Expand Down
23 changes: 23 additions & 0 deletions src/dom/DomUtil.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,3 +319,26 @@ export function restoreOutline() {
_outlineStyle = undefined;
DomEvent.off(window, 'keydown', restoreOutline);
}

// @function getSizedParentNode(el: HTMLElement): HTMLElement
// Finds the closest parent node which size (width and height) is not null.
export function getSizedParentNode(element) {
do {
element = element.parentNode;
} while ((!element.offsetWidth || !element.offsetHeight) && element !== document.body);
return element;
}

// @function getScale(el: HTMLElement): Object
// Computes the CSS scale currently applied on the element.
// Returns an object with `x` and `y` members as horizontal and vertical scales respectively,
// and `boundingClientRect` as the result of [`getBoundingClientRect()`](https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect).
export function getScale(element) {
var rect = element.getBoundingClientRect(); // Read-only in old browsers.

return {
x: rect.width / element.offsetWidth || 1,
y: rect.height / element.offsetHeight || 1,
boundingClientRect: rect
};
}
15 changes: 12 additions & 3 deletions src/dom/Draggable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,14 @@ export var Draggable = Evented.extend({
// Fired when a drag is about to start.
this.fire('down');

var first = e.touches ? e.touches[0] : e;
var first = e.touches ? e.touches[0] : e,
sizedParent = DomUtil.getSizedParentNode(this._element);

this._startPoint = new Point(first.clientX, first.clientY);

// Cache the scale, so that we can continuously compensate for it during drag (_onMove).
this._parentScale = DomUtil.getScale(sizedParent);

DomEvent.on(document, MOVE[e.type], this._onMove, this);
DomEvent.on(document, END[e.type], this._onUp, this);
},
Expand All @@ -134,12 +138,17 @@ export var Draggable = Evented.extend({
}

var first = (e.touches && e.touches.length === 1 ? e.touches[0] : e),
newPoint = new Point(first.clientX, first.clientY),
offset = newPoint.subtract(this._startPoint);
offset = new Point(first.clientX, first.clientY)._subtract(this._startPoint);

if (!offset.x && !offset.y) { return; }
if (Math.abs(offset.x) + Math.abs(offset.y) < this.options.clickTolerance) { return; }

// We assume that the parent container's position, border and scale do not change for the duration of the drag.
// Therefore there is no need to account for the position and border (they are eliminated by the subtraction)
// and we can use the cached value for the scale.
offset.x /= this._parentScale.x;
offset.y /= this._parentScale.y;

DomEvent.preventDefault(e);

if (!this._moved) {
Expand Down

0 comments on commit fe9e0f2

Please sign in to comment.