Fixed dom event coordinates for scaled container #10096

kawsndriy · 2020-11-13T02:10:20Z

When map container belongs to a CSS transformed tree, the getBoundingClientRect() based solutions report incorrect
coordinates.

This change accounts for scale during DOM events processing and fixes #6079

This solution would not work for the cases when css transform includes rotation or skew. But should serve as a decent band-aid for the case of simple scaling.

Launch Checklist

Verified that the issue described in Mouse events are shifted after a css transform scale on the map container #6079 is fixed with this change.
Verified for both touch and mouse devices
Verified that there is no jumping behavior when mouse leaves map container

The following items have not been done. Please let me know if these are blockers for the merge.

include before/after visuals or gifs if this PR includes visual changes
write tests for all new functionality
post benchmark scores
add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog></changelog>

When map container belongs to a CSS transformed tree, the `getBoundingClientRect()` based solutions report incorrect coordinates. This change accounts for scale during DOM events processing and fixes mapbox#6079 This solution would not work for the cases when css transform includes rotation or skew. But should serve as a decent band-aid for the case of simple scaling.

kawsndriy · 2020-11-13T20:30:44Z

I'm trying to fix the unit tests, but on my osx I'm getting Failed to initialize WebGL error when running yarn test-unit ui/map.test.js - not sure yet how to fix this. Tips are highly appreciated.

kawsndriy · 2020-11-13T21:12:27Z

Ah, I think my webgl issues are due to node 12.13 version. As mentioned in https://github.com/stackgl/headless-gl:

Note [macOS only]: due to an inadvertant low-level breaking change in libuv's process handling code, this package doesn't return a gl context when running nodejs version 12.13.1 through to 12.16.1, and 13.0.0 through to 13.6.0 on macOS. A fix has been released in Node.js versions 12.16.2 and 13.7.0. Other platforms are unaffected.

Checking on 12.16.3 now.

kawsndriy · 2020-11-17T00:28:33Z

@ryanhamley can you please check this one? It avoids using offsetX/offsetY properties, so the map panning experience remains the same

edgars-vasiljevs · 2020-11-17T23:37:23Z

thanks @kawsndriy, fix works great.

kawsndriy · 2020-11-30T17:06:32Z

Friends, can you please check this out?

ryanhamley · 2020-11-30T17:10:05Z

Hi @kawsndriy sorry for the delay. We're working on some deadlines right now and most other work has been put on the back burner. I will try to give this a thorough review on Thursday or Friday of this week.

CLAassistant · 2020-12-08T16:49:13Z

All committers have signed the CLA.

ryanhamley · 2021-01-12T18:30:12Z

@kawsndriy if you still want to contribute this, you'll need to sign our new Contributor License Agreement. I'm going to close this as stale but if you sign the CLA, we can re-open it.

rgazelot · 2021-01-12T19:14:28Z

I was waiting for excited news about this issue :'(

ryanhamley · 2021-01-12T20:44:42Z

No judgement on this PR. We'll be happy to re-open and review it once the CLA is signed or if anyone else wants to take up this work.

kawsndriy · 2021-01-19T21:52:36Z

CLA signed. Please consider reopening @ryanbaumann

ryanhamley · 2021-01-20T02:10:37Z

@kawsndriy are you sure it worked? i didn't get an email notifying me of it and the link above still shows it not being sign. Maybe try again? Either way, I'll reopen this in anticipation of getting that resolved. Edit: Nevermind. It took a bit of time for the CLA to update. Thanks for signing!

ryanhamley

This looks good! Thanks for the contribution. I think it's fine that it only handles scale at this point since that's likely to be the most common transform used on maps.

adamcbrewer · 2021-02-13T12:07:54Z

So happy to see this go in, thanks again for the work @kawsndriy

blq · 2021-03-26T10:01:33Z

src/util/dom.js

+    // Note: `el.offsetWidth === rect.width` eliminates the `0/0` case.
+    const scaling = el.offsetWidth === rect.width ? 1 : el.offsetWidth / rect.width;
+    return new Point(
+        (e.clientX - rect.left) * scaling,


el.clientLeft/Top is not used anymore in this version, is that correct?

andresgutgon · 2022-06-07T08:44:10Z

Thanks @kawsndriy this PR pointed me in the right direction 🙌

Rmkasun-8 approved these changes Nov 13, 2020

View reviewed changes

Fixed 0 == 0 edge case

0e5e64d

ryanhamley closed this Jan 12, 2021

ryanhamley reopened this Jan 20, 2021

shiftonetothree approved these changes Feb 7, 2021

View reviewed changes

ryanhamley approved these changes Feb 12, 2021

View reviewed changes

ryanhamley merged commit c8490fa into mapbox:main Feb 12, 2021

ryanhamley mentioned this pull request Feb 12, 2021

CSS transform issue #7701

Open

karimnaaji added this to the v2.2 milestone Mar 1, 2021

karimnaaji added the bug 🐞 label Mar 2, 2021

blq reviewed Apr 11, 2021

View reviewed changes

JannikGM mentioned this pull request Apr 12, 2021

How to use new contributions under the 3-Clause BSD license, without asking for permission on each PR #10543

Closed

steff-o mentioned this pull request Jan 24, 2022

Wrong place for selected polygon in print preview origo-map/origo#1422

Closed

andresgutgon mentioned this pull request Jun 7, 2022

Fix tooltip positioning when charts are rendered inside an scaled div plouc/nivo#2034

Merged

huanglii mentioned this pull request Dec 21, 2023

Incorrect dom event coordinates for scaled container maplibre/maplibre-gl-js#3506

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed dom event coordinates for scaled container #10096

Fixed dom event coordinates for scaled container #10096

kawsndriy commented Nov 13, 2020 •

edited

Loading

kawsndriy commented Nov 13, 2020

kawsndriy commented Nov 13, 2020

kawsndriy commented Nov 17, 2020

edgars-vasiljevs commented Nov 17, 2020

kawsndriy commented Nov 30, 2020

ryanhamley commented Nov 30, 2020 •

edited

Loading

CLAassistant commented Dec 8, 2020 •

edited

Loading

ryanhamley commented Jan 12, 2021 •

edited

Loading

rgazelot commented Jan 12, 2021

ryanhamley commented Jan 12, 2021

kawsndriy commented Jan 19, 2021

ryanhamley commented Jan 20, 2021 •

edited

Loading

ryanhamley left a comment

adamcbrewer commented Feb 13, 2021

blq Mar 26, 2021

andresgutgon commented Jun 7, 2022

Fixed dom event coordinates for scaled container #10096

Fixed dom event coordinates for scaled container #10096

Conversation

kawsndriy commented Nov 13, 2020 • edited Loading

Launch Checklist

kawsndriy commented Nov 13, 2020

kawsndriy commented Nov 13, 2020

kawsndriy commented Nov 17, 2020

edgars-vasiljevs commented Nov 17, 2020

kawsndriy commented Nov 30, 2020

ryanhamley commented Nov 30, 2020 • edited Loading

CLAassistant commented Dec 8, 2020 • edited Loading

ryanhamley commented Jan 12, 2021 • edited Loading

rgazelot commented Jan 12, 2021

ryanhamley commented Jan 12, 2021

kawsndriy commented Jan 19, 2021

ryanhamley commented Jan 20, 2021 • edited Loading

ryanhamley left a comment

Choose a reason for hiding this comment

adamcbrewer commented Feb 13, 2021

blq Mar 26, 2021

Choose a reason for hiding this comment

andresgutgon commented Jun 7, 2022

kawsndriy commented Nov 13, 2020 •

edited

Loading

ryanhamley commented Nov 30, 2020 •

edited

Loading

CLAassistant commented Dec 8, 2020 •

edited

Loading

ryanhamley commented Jan 12, 2021 •

edited

Loading

ryanhamley commented Jan 20, 2021 •

edited

Loading