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

Retrieve Google attributions automatically via their API and provide an example showing how to add the Google logo #15598

Merged
merged 16 commits into from
Mar 1, 2024

Conversation

fnicollet
Copy link
Contributor

Fixes issue #15589
To avoid adding to OL's bundle size, I decided not to include the Google logo in the source code. It also allows developpers to chose the logo they prefer/need (color/white and high DPI or not).
Instead I added an example of how to add a custom OL Control in the bottom-left corner, with a simple logo added as base64.

For the attributions, they are now retrieved via their API. Using ViewHint.ANIMATING / ViewHint.INTERACTING was still resulting in many identical API calls, so I added a little memory of the last API parameters requested, to avoid requesting the same viewport informations many time.

The new example can be tested easily by entering your own Google API Key. Then you need to zoom in somewhere to see the attributions change:
image

On Google's documentation, section "Displaying third-party data attributions":

https://developers.google.com/maps/documentation/tile/policies#displaying-third-party-data-attributions

It says:
+++++
When you use the Map Tiles API to display a Google Maps data as a basemap and overlay third-party (non-Google) geospatial data, you may be required to display attribution from the third-party data provider. You must not overlap or obscure the Google data attribution in any way with the data attribution of the third-party data. The attribution of third-party data must clearly be disassociated from Google's data attributions. In addition, it must be clear that Google's logo and Google's data attribution are associated with the basemap and with each other.
+++++

So I added a "Google : " prefix, to avoid conflict with other source attributions.

To be able to retrieve attributions in an async way, I changed the way the setAttributions function works, adding the patch from @ahocevar in the original issue.

I also added more information in the text of the example, to explain how you can comply with Google's policies and linked back to documentation.

I tried running the linter but on Windows, it gave me thousands of errors:
image

So hopefully the formatting is OK

Copy link

📦 Preview the website for this branch here: https://deploy-preview-15598--ol-site.netlify.app/.

@ahocevar
Copy link
Member

@fnicollet Thanks for your effort on this! Can you please run npm run lint -- --fix to auto-fix the linter errors?

@mike-000
Copy link
Contributor

So I added a "Google : " prefix, to avoid conflict with other source attributions.

By default attributions for all sources appear as a single line so adding a prefix might not help. Multiple attributions can be shown on separate lines by adding

.map .ol-attribution li {
  display: block;
}

to the application css.

@mike-000
Copy link
Contributor

To avoid the large base64 string in the example code you could link the logo https://developers.google.com/static/maps/documentation/images/google_on_white.png directly (in the same way as the IGN logo in https://openlayers.org/en/latest/examples/wmts-ign.html).

@fnicollet
Copy link
Contributor Author

Following @mike-000 comments, I removed the prefix (devs can add the CSS to display lines as block in its on code, if mixing with other sources) and linked to the image URL instead.

@ahocevar ran the linter, which fixed a couple of issues (didn't the the thousands of errors when using the --fix option)

@mike-000
Copy link
Contributor

didn't the the thousands of errors when using the --fix option

It looks like your editor is changing line endings from LF to CRLF.

Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

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

This is coming together nicely! I added a few more suggestions.

src/ol/source/Google.js Outdated Show resolved Hide resolved
src/ol/source/Google.js Outdated Show resolved Hide resolved
src/ol/source/Google.js Show resolved Hide resolved
examples/google.js Outdated Show resolved Hide resolved
Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

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

Great work! Thanks, @fnicollet

@ahocevar ahocevar merged commit 55f7a61 into openlayers:main Mar 1, 2024
8 checks passed
@fnicollet
Copy link
Contributor Author

Great, thanks both of you for your help on this!

@tschaub
Copy link
Member

tschaub commented Mar 1, 2024

Thank you @fnicollet for taking time to read the terms of service, open an issue, and contribute this enhancement. And thanks @mike-000 and @ahocevar for the time spent reviewing.

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.

4 participants