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 shop=massage #3309

Merged
merged 7 commits into from
Aug 5, 2018
Merged

Add icon for shop=massage #3309

merged 7 commits into from
Aug 5, 2018

Conversation

jragusa
Copy link
Contributor

@jragusa jragusa commented Jul 18, 2018

Fixes #3287

Changes proposed in this pull request:
Render shop=massage from z18 using leisure-green colour

Test rendering with links to the example places:

Node
https://www.openstreetmap.org/node/4992712423
massage_point

Way
https://www.openstreetmap.org/way/183708628
massage_way

@kocio-pl
Copy link
Collaborator

After merging #3293 this code needs to be updated to resolve conflicts.

@jragusa
Copy link
Contributor Author

jragusa commented Jul 19, 2018

Done :)

@kocio-pl
Copy link
Collaborator

Hm, it works strange - for z17 instead of dot (which color should it be?) it shows miniaturized icon, for example:

screenshot_2018-07-19 jybq9re1 png obraz png 3155 x 3105 pikseli

@Tomasz-W Tomasz-W mentioned this pull request Jul 19, 2018
26 tasks
@jragusa
Copy link
Contributor Author

jragusa commented Jul 20, 2018

I see why there is an small icon at z17. But before to make changes, I would like to know whether we leave a dot for z17 like other shops or we bypass rendering for z17 ?

@lakedistrictOSM
Copy link

I think that we should bypass rendering at z17. z18 is good enough for massage places.

Purple would be the wrong colour and green dots aren't yet used (and also might look like trees).

@jragusa
Copy link
Contributor Author

jragusa commented Jul 22, 2018

After correction:

z17
name appears as there is no pink dot (I suppose)
massage_z17
A suggestion would be to shift rendering at z17 such as fitness_center

z18
massage_z18

z19
massage_z19

@kocio-pl
Copy link
Collaborator

I guess it's safe to render from z18. I don't have good idea for not typical colors of shops - BTW similar problem is with shop=car_repair (#1778).

@kocio-pl
Copy link
Collaborator

kocio-pl commented Aug 3, 2018

Could you update the code once again? I plan to merge it soon.

BTW: please, add some line breaks aligned to the lines above, so the code is more readable and easier to edit.

@jragusa
Copy link
Contributor Author

jragusa commented Aug 5, 2018

Done, I also put some line breaks as suggested.

Is it correct for you ?

@kocio-pl kocio-pl merged commit e58cc3c into gravitystorm:master Aug 5, 2018
@kocio-pl
Copy link
Collaborator

kocio-pl commented Aug 5, 2018

Thanks, this is good!

@jragusa jragusa deleted the massage branch August 21, 2018 13:10
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