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

Unexport and undocument LogoControl #4244

Merged
merged 1 commit into from
Feb 9, 2017
Merged

Conversation

jfirebaugh
Copy link
Contributor

Other than the logoPosition map option, there's no public API -- it's shown/hidden automatically.

@mollymerp
Copy link
Contributor

mollymerp commented Feb 9, 2017

It's not yet shown/hidden automatically for all users because there are still remaining API changes that are blocked. Some users still need to add the logo manually, but if we want to make the Control private and just have them use the HTML snippet to add it as has been the practice up til now that's fine too.

@jfirebaugh I think there's also a line in documentation.yml for LogoControl that should be removed if we do decide to make it completely private

@jfirebaugh
Copy link
Contributor Author

Some users still need to add the logo manually

I assumed this wasn't possible/needed because all instances of LogoControl are going to be either shown or hidden based on what _updateLogo() returns. So if a user added one manually in addition to the automatic one, there would be either none visible still, or two visible. Do we need to handle this some other way?

Copy link
Contributor

@mollymerp mollymerp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you're right about _updateLogo -- I'm good with making this private because the API changes should ship soon so it might not be worth it to make an alternative in the meantime.

LGTM once https://github.com/mapbox/mapbox-gl-js/blob/unexport-logocontrol/documentation.yml#L21 is also removed.

Other than the logoPosition map option, there's no public API -- it's shown/hidden automatically.
@jfirebaugh jfirebaugh merged commit fb82550 into master Feb 9, 2017
@jfirebaugh jfirebaugh deleted the unexport-logocontrol branch February 9, 2017 20:51
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