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 (purple) #4535

Merged
merged 1 commit into from
Jun 4, 2022

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 label purple to match most other shop=* tags.

Fixes #2658


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

Either one is probably better than: nope


Before

17-now

18-now

After

17-purple

18-purple

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

This commit makes the label purple to match most other `shop=*` tags.

Fixes gravitystorm#2658
@jdhoek jdhoek changed the title Fix shop=car_repair label/icon colour mismatch Fix shop=car_repair label/icon colour mismatch (purple) 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 looks fine and renders as expected. Nice to see this being fixed.

I don't have any strong preference regarding the color, both choices seem in principle fine with me. So far we don't have a very consistent scheme of colors with symbols. I would suggest to:

  • think about a practically verifiable criterion to determine if some feature is to be rendered in shop color or a different color.
  • Then apply it to the choice here and
  • document it so others can follow the same principle with other changes

And the criterion should of course not be the key used by the tag - that does not follow any consistent rules.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Jun 3, 2022

I approve this change (I have not tested the rendering but the code change looks fine)

@pnorman pnorman merged commit afbfbcc into gravitystorm:master 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
4 participants