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

Make markers anchored at their center by default #2900

Closed
jensstalder opened this issue Jul 21, 2016 · 7 comments
Closed

Make markers anchored at their center by default #2900

jensstalder opened this issue Jul 21, 2016 · 7 comments
Labels
breaking change ⚠️ Requires a backwards-incompatible change to the API feature 🍏

Comments

@jensstalder
Copy link

https://www.mapbox.com/mapbox-gl-js/example/custom-marker-icons/

The images are anchored on the top left!!!!????

That makes them unusable.

If they were centered it would make 100000% more sense.

@jensstalder jensstalder changed the title ustom-marker-icons Wrong default anchorpoint custom-marker-icons Wrong default anchorpoint Jul 21, 2016
@tmcw
Copy link
Contributor

tmcw commented Jul 21, 2016

Hello!

Custom marker icons can be anchored any number of ways by using margins. This example doesn't do any modification of the anchor, and you're right - they would make more sense if they were anchored at the center. I'll make a note to fix the example.

Thanks for the issue - in the future, please spare the extra zeros, exclamation marks, and question marks: we can understand the intent of the issues without extra hyperbole.

  • Tom

@jensstalder
Copy link
Author

jensstalder commented Jul 21, 2016

^^ cool

So your saying something like that?

el.style.marginLeft = '-'+(imagewidth/2)+'px';

@tmcw
Copy link
Contributor

tmcw commented Jul 21, 2016

@jensstalder yes, exactly. Setting negative margins will shift the anchor point of the marker.

@jensstalder
Copy link
Author

ok cool.

@mourner
Copy link
Member

mourner commented Jul 21, 2016

Also note that there will be an offset option in the next release: #2885

@1ec5
Copy link
Contributor

1ec5 commented Jul 21, 2016

I think this is an issue with the library, not the example. I think the default should be at the center, for consistency with symbols. The proper anchor of course depends on the image content (since a pushpin could be drawn to face in any direction), so #2885 is a welcome addition. But the most common marker images would either have a natural anchor at the center or the bottom-center, not the top-left, and I think it’s important for the various APIs for implementing a marker to have consistent behavior.

@1ec5 1ec5 reopened this Jul 21, 2016
@1ec5 1ec5 changed the title custom-marker-icons Wrong default anchorpoint Marker should be anchored at the center by default Jul 21, 2016
@lucaswoj lucaswoj changed the title Marker should be anchored at the center by default Make markers anchored at their center by default Jul 29, 2016
@andrewharvey
Copy link
Collaborator

I think this is an issue with the library, not the example. I think the default should be at the center, for consistency with symbols. The proper anchor of course depends on the image content (since a pushpin could be drawn to face in any direction), so #2885 is a welcome addition. But the most common marker images would either have a natural anchor at the center or the bottom-center, not the top-left, and I think it’s important for the various APIs for implementing a marker to have consistent behavior.

Spot on @1ec5. I agree.

One case for defaulting to the center is the userLocation marker proposed in #4479. In that PR if someone wants to change the marker dot style, eg. make it bigger or smaller, it's quite easy to override the default CSS, however the offset is hardcoded at https://github.com/mapbox/mapbox-gl-js/pull/4479/files#diff-8c498cf163efa60ad84199c531ab7c5aR283.

I understand that a workaround is to use a margin property in the marker css, but it just doesn't feel as simple or clean as defaulting to the center.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ⚠️ Requires a backwards-incompatible change to the API feature 🍏
Projects
None yet
Development

No branches or pull requests

5 participants