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

Replace mapbox watermark on the maps with maplibre #643 and replace mapbox resource names #647

Merged
merged 9 commits into from
Jan 4, 2023

Conversation

louwers
Copy link
Collaborator

@louwers louwers commented Jan 3, 2023

Closes #643

You can already disable the icon, but @acalcutt @ntadej had the idea to make it configurable which icon is shown. We should create a ticket out of this.

@acalcutt
Copy link
Collaborator

acalcutt commented Jan 3, 2023

That was actually @ntadej that said to make the icon configurable.

I was just saying in maplibre-gl-js it used show based on a mapbox tilejson parameter. it had been changed at one point to look for a maplibre tilejson parameter, which meant it basically never was shown.

I had modified in the js version in maplibre/maplibre-gl-js#786 so it no longer shows based on TileJSON, but instead was a settable map option. The reason was more to repurpose this logo to give maplibre project a bit more visibility.

@louwers
Copy link
Collaborator Author

louwers commented Jan 3, 2023

Sorry for the wrong @ mention Andrew. From what I can tell whether the icon shows is not dependent on that on Android.

Copy link
Member

@birkskyum birkskyum left a comment

Choose a reason for hiding this comment

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

Transitioning the resourcePrefix from 'mapbox_' to resourcePrefix from 'maplibre_' almost deserves it's own PR, but in order to do it stepwise like this, I reckon it can be disabled to let the lint pass while it's ongoing.

@louwers
Copy link
Collaborator Author

louwers commented Jan 3, 2023

@birkskyum Since you said 'almost'...

I added mlns:tools="http://schemas.android.com/tools" tools:ignore="ResourceName" to attrs.xml and public.xml since they are part of the Public API. Maybe we can duplicate them and then phase them out.

@louwers louwers marked this pull request as draft January 3, 2023 19:28
@louwers louwers changed the title Replace mapbox watermark on the maps with maplibre #643 Replace mapbox watermark on the maps with maplibre #643 and replace mapbox resource names Jan 3, 2023
@louwers louwers marked this pull request as ready for review January 3, 2023 22:59
@birkskyum
Copy link
Member

Great! I'd like an Android changelog entry. Also, it's generally preferred to use rebate rather than merge when synching the branch to master as it simplifies the git history.

@louwers
Copy link
Collaborator Author

louwers commented Jan 4, 2023

Also, it's generally preferred to use rebate rather than merge when synching the branch to master

Thanks for the heads-up, I will do that next time. When I squash the commits when I merge later, the git history should be clean in this case right (because the branches haven't diverged)?

@birkskyum
Copy link
Member

Yes, it'll be clean in the main branch after squashing, so it's only related to the review process

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.

Replace mapbox watermark on the maps with maplibre
3 participants