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

Fix shop=car_repair label/icon colour mismatch (brown) #4536

Closed

Conversation

jdhoek
Copy link
Contributor

@jdhoek jdhoek commented Apr 21, 2022

The colour of the icon (purple) and the label (brown) don't match.

This commit makes the icon brown in line with other service providing
amenities.

Fixes #2658


This is one of two ways to solve this. The other colour choice is shown in #4535. These two PR's are mutually exclusive.

Either one is probably better than: nope


Before

17-now

18-now

After

17-brown

18-brown

The colour of the icon (purple) and the label (brown) don't match.

This commit makes the icon brown in line with other service providing
amenities.

Fixes gravitystorm#2658
@jdhoek jdhoek changed the title Fix shop=car_repair label/icon colour mismatch Fix shop=car_repair label/icon colour mismatch (brown) Apr 21, 2022
Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

This in principle works fine as well, however it introduces brown dot rendering at z17 - which would be a novelty for this style. Unlike shops where almost all variants start at the same zoom level brown symbols start at very different zoom levels so introducing a dot only rendering at the starting zoom level could seem a bit weird. Still this could be discussed as a new design idea, however if you want to do so i would suggest to separate that from the bug fix.

As written in #4535 i am not partial to either of the color choices but would like to see a consistent criterion being developed, applied and documented for that.

@quinncnl
Copy link

quinncnl commented May 21, 2022

however it introduces brown dot rendering at z17 - which would be a novelty for this style.

It does not introduce a dot at z17. A purple dot(as shops) is rendered in the current rendering. See https://www.openstreetmap.org/#map=18/51.42782/5.44144

Car repair shops belong to amenity shop types like car_wash. I think it's correct to show them at z17 and use brown color.

Copy link
Collaborator

@jeisenbe jeisenbe left a comment

Choose a reason for hiding this comment

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

I believe shop icons should be consistent in one color, so I recommend closing this PR without merging

@pnorman
Copy link
Collaborator

pnorman commented Jun 4, 2022

I also agree on the purple colour being the better option. Closing in favour of that PR.

@pnorman pnorman closed this Jun 4, 2022
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.

shop=car_repair color inconsistency dot vs icon
5 participants