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

Render leisure=dance #4996

Merged
merged 2 commits into from
Oct 16, 2024
Merged

Render leisure=dance #4996

merged 2 commits into from
Oct 16, 2024

Conversation

wyskoj
Copy link
Contributor

@wyskoj wyskoj commented Jul 30, 2024

Changes proposed in this pull request:

Add rendering for leisure=dance

lesiure=dance is used 11k times across a wide geographic area, and it has been approved since 2009.

Icon

I designed a simple icon.

dance

I did my best to follow as many icon style guidelines as possible, most notably aligning to a 14x14 pixel grid:

image

Test rendering

Location 1

On a way with building=*.

Before

No rendering (name is rendered with building).

image

After

z19

image

z18

image

z17

image

<=z16

No render, consistent with other leisure and amenity features.

Location 2

On a node.

Before

No rendering.

image

After

z19

image

z18

Text occluded by other features.

image

z17

image

Icon happens to have precedent here.

<=z16

No render, consistent with other leisure and amenity features.

Notes

  • Style is grouped in with leisure=sauna for ease of implementation. I don't have a strong preference for this feature's color, but I think it makes sense since most other leisure features use this color.

Thanks for considering my PR!

@imagico
Copy link
Collaborator

imagico commented Aug 2, 2024

Thanks for the pull request.

I think the tag is suitable for rendering, no competing tag with comparable adoption, geographically broad use by individual mappers in different parts of the world, well established over several years.

The symbol design principle seems suitable as well - dancing human figure and music notes to avoid confusion with sports.

I am a bit wary about the readability of the symbol because of the elements being very small. One thing that might help is avoiding the bottom diagonal line formed by the feet of the figure and the music notes - which could be misread as the two symbols components forming a geometric unit. Maybe you could try re-arranging - mirroring/swapping the two elements in some form that disrupts this line.

Side note: I generally dislike the use of green for social amenity symbols because IMO green should be reserved for vegetation related features. But that is not a matter significant for this PR, it is a more general comment.

@dch0ph
Copy link
Contributor

dch0ph commented Aug 2, 2024

Nice job!

I agree that the symbol works - figure + note unlikely to be confused with a sport symbol. The musical notes are quite small and not instantly recognizable. You could try using a mitre join for the top bar (rather than rounded corners). A thicker top bar (1.5 px), as in conventional musical notation, might also help?

@wyskoj
Copy link
Contributor Author

wyskoj commented Aug 3, 2024

Maybe you could try re-arranging - mirroring/swapping the two elements in some form that disrupts this line.

You could try using a mitre join for the top bar (rather than rounded corners). A thicker top bar (1.5 px), as in conventional musical notation, might also help?

I've enacted both of these suggestions, and I think they improve readability.

image

I generally dislike the use of green for social amenity symbols because IMO green should be reserved for vegetation related features.

I'm happy to set a different color if there's one you prefer—should be a simple fix.

@imagico imagico merged commit 49e5c27 into gravitystorm:master Oct 16, 2024
2 checks passed
@imagico
Copy link
Collaborator

imagico commented Oct 16, 2024

Merging this to be included in the next release, sorry for the long wait time. The use of green color for not vegetation related symbols is a different and more general issue, which will need to be discussed separately.

Nice work on the design.

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