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

Add 'anchor' option for Markers #5640

Closed
anandthakker opened this issue Nov 9, 2017 · 5 comments
Closed

Add 'anchor' option for Markers #5640

anandthakker opened this issue Nov 9, 2017 · 5 comments
Assignees

Comments

@anandthakker
Copy link
Contributor

Following up from #4751, it would be nice to add an 'anchor' option for Marker, similar to the one in Popup:

* @param {string} [options.anchor] - A string indicating the popup's location relative to
* the coordinate set via {@link Popup#setLngLat}.
* Options are `'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'`.

@jmandel1027
Copy link
Contributor

jmandel1027 commented Jan 16, 2018

Happy to take a crack at it if no one has it in their queue n_n

Do we want to export a MarkerOptions type as done in popup as well?

export type PopupOptions = {
closeButton: boolean,
closeOnClick: boolean,
anchor: Anchor,
offset: Offset
};

@anandthakker
Copy link
Contributor Author

@jay-manday I don't think anyone's claimed it yet, so please feel free!

Do we want to export a MarkerOptions type as done in popup as well?

Couldn't hurt!

@jmandel1027
Copy link
Contributor

jmandel1027 commented Jan 19, 2018

Hi @anandthakker

I thiiiink i have things working as needed though would love a review of my work before making a PR to the master branch on your end. I did some digging on the origins of the issue and was a pretty cool learning experience! 🌀💡🎉✨

Heres the work done on my fork:
jmandel1027@47d1243

I also went ahead and manually wrote the test for the marker file too. n_n

@andrewharvey
Copy link
Collaborator

Hi @jay-manday nice to see you implementing this! Feel free to go ahead and open that PR as that's where the review typically takes place, even if you feel it's not 100% ready, you can just mention it's a WIP.

The options for Popup's anchor are 'top', 'bottom', 'left', 'right', 'top-left', 'top-right', 'bottom-left', and `'bottom-right'. I think that's what we should implement for the Marker anchor.

@jmandel1027
Copy link
Contributor

jmandel1027 commented Jan 21, 2018

Hi @andrewharvey, totally! I'll make a PR and we can further discuss the work so far. Thanks for pointing me in the right direction as to the desired anchor options.

I think I went down a wrong turn with my reasoning, while investigating how the marker renders onto the map I made the assumption that we wanted to anchor the marker via pixel values rather than css. I will definitely will factor this into my PR. n_n

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

No branches or pull requests

4 participants