search

mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.07k stars 2.21k forks source link

dynamic popup anchor with a non bottom preference #6965

Open andrewharvey opened 6 years ago

andrewharvey commented 6 years ago

The Popup anchor option is documented as

A string indicating the part of the Popup that should be positioned closest to the coordinate set via Popup#setLngLat . Options are 'center' , 'top' , 'bottom' , 'left' , 'right' , 'top-left' , 'top-right' , 'bottom-left' , and 'bottom-right' . If unset the anchor will be dynamically set to ensure the popup falls within the map container with a preference for 'bottom' .

I'd like to retain the dynamic feature to ensure the popup doesn't fall off the map, while setting a preference other than bottom.

I think the best solution is to change the definition of the anchor option to be the preferred anchor and introduce a new fixedAnchor option to tell if you want it to avoid falling of the map or not.

This is a backwards incompatible change, but I think any change made in a backwards compatible way would introduce to much complexity to the API (for example leaving anchor as is and adding a new anchorPreference option).

I don't mind trying to do a PR, but what do people think about this breaking change?

ryanhamley commented 6 years ago

Doesn't anchor already operate as the "preferred anchor"? it simply falls back to bottom if nothing else is set. Would fixedAnchor just be a boolean that determines whether the collision detection with the edge of the viewport happens?

andrewharvey commented 6 years ago

http://jsbin.com/laderob/edit?html,output

If I set anchor then it will happily get clipped off the map, so right now I can't prefer an anchor while avoiding the popup getting clipped, it's one or the other but not both.

Would fixedAnchor just be a boolean that determines whether the collision detection with the edge of the viewport happens?

Correct

jfirebaugh commented 6 years ago

Yeah, I like the fixedAnchor approach. If we default it to true it's not even a breaking change, right?

andrewharvey commented 6 years ago

Yeah, I like the fixedAnchor approach. If we default it to true it's not even a breaking change, right?

I like it too. Currently:

{ anchor: 'bottom' } is fixed { anchor: 'left' } is fixed { } is dynamic

We have to decide between:

  1. being 100% backwards compatible but having a more complicated API, where the default for fixedAnchor depends on if anchor is defined, or
  2. a breaking change but a simpler API where the default for fixedAnchor is always false
alex-semenk commented 3 years ago

I want to leave my vote for the feature.

Typically monitors are more wide, than tall. So, from user experience perspective it is more friendly to display large popups on the left or right (where we much more space), rather then trying to fit it in the top or bottom half of the screen (like default behavior does).

anthony-hull commented 2 years ago

can we remove the breaking change tag? The implementation in the PR is backwards compatible with the current API