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

Popups automatically wrap with multiple world views #4577

Closed
ryanbaumann opened this issue Apr 11, 2017 · 12 comments
Closed

Popups automatically wrap with multiple world views #4577

ryanbaumann opened this issue Apr 11, 2017 · 12 comments
Assignees
Labels

Comments

@ryanbaumann
Copy link
Contributor

mapbox-gl-js version: 0.35.0 (does not exist on 0.34.0)

Steps to Trigger Behavior

  1. Create a popup under a mouse location on a zoom 0 map with multiple world views.
  2. Mouse over a wrapped world map location.
  3. Popup will automatically wrap.

Possible culprit

It looks like this commit https://github.com/mapbox/mapbox-gl-js/pull/4555/files may be automatically be wrapping popup.setLngLat()

Expected Behavior

The popup appears at unwrapped world location under the mouse cursor.

Actual Behavior

The popup appears at wrapped world location

@jfirebaugh
Copy link
Contributor

This is the intended behavior as of #4452. Can you say more about why it's an issue for you?

@andrewharvey
Copy link
Collaborator

The intended behaviour was to ensure that markers and popups (which aren't duplicated) appear in the best world based on the current view, see #4452 (comment).

@andrewharvey
Copy link
Collaborator

@ryanbaumann The intention for the popup wrapping feature was driven for when popups are attached to a geographic location based on data.

In your use case you're attaching a popup based on the cursor location.

I think the solution here might be to add an option to Popup to differentiate these two use cases, as in the former the wrapping is desired but the latter it is not. Thoughts?

@ryanbaumann
Copy link
Contributor Author

@andrewharvey Correct - I want to place a popup under the user's mouse pointer (for large polygon features). An option in the popup to control world wrap behaviour would work great.

Basic Example

@jfirebaugh
Copy link
Contributor

Rather than adding another option, could we adjust the wrapping so that it happens only when a move event occurs and the prior location of the popup would now be outside the viewport?

@andrewharvey
Copy link
Collaborator

Rather than adding another option, could we adjust the wrapping so that it happens only when a move event occurs and the prior location of the popup would now be outside the viewport?

Agreed that if we can avoid an option that's best.

Since we have automatic jumping back to world 0 markers and popups can jump to the center before they fall off the screen though.

The automatic jumping back to world 0 helps because it means w0 is never too far away from the center (without it w0 could be all the way over on the right, and adding new Markers to the edge of the screen isn't great)
selection_550

The logic that determines the best world helps because in the above example adding a point at long -170 at point (1) shows up in w1 at point (2) which is better because it's closer to the center of the screen.

That's ideal when creating new Markers or Popups from data, but if the popup is driven by a mouse event at (1) we want the popup to appear there.

Not sure how to do the best for both situations without somehow flagging "this popup is from a mouse interaction so needs to happen in the same world as the mouse" vs "this popup is created programatically from some other data so we want it to happen in the best world for the current view to place it as centrally as possible." 😕

@jfirebaugh
Copy link
Contributor

I think when Markers or Popups are newly positioned programatically, it's reasonable to respect the given initial position (as long it's actually visible on screen). In cases where multiple world copies are visible, and someone desires placement in a particular copy, they can perform the appropriate wrapping/unwrapping before placing the control.

@andrewharvey
Copy link
Collaborator

In cases where multiple world copies are visible, and someone desires placement in a particular copy, they can perform the appropriate wrapping/unwrapping before placing the control.

This would change the behaviour introduced in #4452 which placed Markers more centrally within the viewport by default which avoided placing markers or popups on the viewport edge pixels (which would mean that the marker might be cut off the screen even when there is another place in the viewport it could be at which isn't cut off).

I this this is best handled in the core since

  1. it needs to recalculate this placement every time the map moves
  2. it means that users get this behaviour out of the box without doing additional development on their end

But I don't see how both of these use cases can be supported together without a flag to indicate if the placement should be "best" (when not linked to the cursor) or "as provided" (when linked to the cursor).

@jfirebaugh
Copy link
Contributor

The behavior in 2c1615e feels pretty good to me.

  • Popups are always initially placed exactly where specified, without wrapping.
  • Only when the map moves do we consider wrapping the popup:
    • When the map center crosses the antimeridian and wraps, the popup position wraps in the opposite direction, to maintain object constancy.
    • When a popup would go off-screen, it's wrapped the minimum amount to get it back on screen (if it's indeed possible to get it on screen).

If everyone likes this then we can port the behavior to markers as well and hopefully have these positioning nuances finally nailed.

@andrewharvey
Copy link
Collaborator

I tried to test out https://github.com/mapbox/mapbox-gl-js/compare/fix-4577 too ensure I understand the behaviour you're proposing but it doesn't seem this._wrappedLngLat is being set by calling Popup.setLngLat eg. http://jsbin.com/wapuyen/edit?html,output

I'm still feeling it's better to place markers and popups centrally within the viewport rather than on the edge pixels where it has a choice as I think this is a better user experience, and it seems this suggestion would miss that?

@jfirebaugh
Copy link
Contributor

Good catch, fixed in 9979710.

I'm still feeling it's better to place markers and popups centrally within the viewport rather than on the edge pixels where it has a choice as I think this is a better user experience, and it seems this suggestion would miss that?

For me, it feels better to preserve object constancy when possible, and only move markers and popups when necessary to keep them on screen. And with this solution, if you do desire to wrap them more aggressively, you can add your own event handler that does so using setLngLat.

@andrewharvey
Copy link
Collaborator

For me, it feels better to preserve object constancy when possible, and only move markers and popups when necessary to keep them on screen. And with this solution, if you do desire to wrap them more aggressively, you can add your own event handler that does so using setLngLat.

I see what you mean. You have my 👍 on this approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants