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.

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

Fix popups/featureClick positions when scrolled #2179

Merged
merged 1 commit into from
Jul 17, 2018

Conversation

rjimenezda
Copy link
Contributor

@rjimenezda rjimenezda commented Jul 16, 2018

Related issue: https://github.com/CartoDB/support/issues/1659

So this has been broken since we introduced zera, but probably never seen it because we almost never have maps that are not full screen (iframes work just fine). I've attached an example that would be broken with the previous implementation if you scroll vertically or horizontally. Feel free to try it out on master.

I assume there's some difference between what we're getting with zera and what we used to get with wax. The thing is, there's apparently a Leaflet method that retrieves the containerPoint from the layerPoint, it seems to work perfectly.

@rjimenezda
Copy link
Contributor Author

@ivanmalagon @IagoLast ☝️

@ivanmalagon
Copy link
Contributor

Regarding the code, nothing to say there. It makes sense to use the native Leaflet method for it.

I've tried with master distribution and with this fix and it works as expected. In that sense, :bandera:

Do you have any clue about what's different between what wax sent and zera? I guess not because wax is totally wiped out and it's not easy to compare both versions. Perhaps, @IagoLast has an idea of what's the root cause of this.

@rjimenezda
Copy link
Contributor Author

I have the old version around as well to compare.

It's interesting, wax fires two events (pointerup and click), and it looks like it doesn't use Leaflet's event API for that at all, which zera does. Instead, it adds listeners to the DOM element and calculates where it lands on the map, gets the grid tile, etc etc. Iago probably knows this better.

@rjimenezda rjimenezda merged commit 5f934bb into master Jul 17, 2018
@rjimenezda rjimenezda deleted the fix-displaced-popups-scroll branch July 17, 2018 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants