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 icon for Bureau_de_change #3301

Merged
merged 4 commits into from
Jul 17, 2018
Merged

Add icon for Bureau_de_change #3301

merged 4 commits into from
Jul 17, 2018

Conversation

jragusa
Copy link
Contributor

@jragusa jragusa commented Jul 13, 2018

Fixes #3185

Changes proposed in this pull request:
Render amenity=bureau_de_change from z17

Test rendering with links to the example places:
https://www.openstreetmap.org/node/2140012378

Before
bureau_de_change_before

After
bureau_de_change 2

@kocio-pl
Copy link
Collaborator

Works for me. Please check also way rendering (with name label).

@jragusa
Copy link
Contributor Author

jragusa commented Jul 13, 2018

@kocio-pl it works correctly on way
https://www.openstreetmap.org/way/384831563
bureau_de_change_way

@kocio-pl
Copy link
Collaborator

Vertical line on "$" symbol is too weak for me, so it looks like "S", could you tune it a bit?

@Tomasz-W
Copy link

@jragusa
Copy link
Contributor Author

jragusa commented Jul 14, 2018

thanks @Tomasz-W
bureau_de_change 3

@kocio-pl
Copy link
Collaborator

Thanks. Some additional thoughts: maybe adding 2 verticals would be even better? Or maybe these are not needed inside the letter S, only 2 endings on top and on the bottom would be good?

@Tomasz-W
Copy link

With pixel alligned $:

amenity bureau_de_change2

Gist link: https://gist.github.com/Tomasz-W/b438cd0bd7305fec2c29b6bdc766e48f

I tried with 2 vertical bars, but it didn't work well.

@jragusa
Copy link
Contributor Author

jragusa commented Jul 14, 2018

bureau_de_change 4

@kocio-pl kocio-pl merged commit 4bcca2a into gravitystorm:master Jul 17, 2018
@kocio-pl
Copy link
Collaborator

Thanks, this code has been tested in some other places and it works as expected.

@jragusa jragusa deleted the change branch November 8, 2018 17:17
@pnorman pnorman mentioned this pull request May 5, 2019
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.

3 participants