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

Adding rendering for shop=video_games #3167

Merged
merged 3 commits into from
Apr 16, 2018
Merged

Adding rendering for shop=video_games #3167

merged 3 commits into from
Apr 16, 2018

Conversation

Karthoo
Copy link
Contributor

@Karthoo Karthoo commented Apr 4, 2018

Fixes #3117

Changes proposed in this pull request:

  • Adding rendering for shop=video_games with a new icon

Test rendering with links to the example places:

Way z17 in Orange Walk Town (has no name):
video_games way z17
z18:
video_games way z18
Node z17 in East Greenwich:
video_games node z17
z18:
video_games node z18

* Update amenity-points.mss

* Update project.mml

* Create video_games.svg
@kocio-pl
Copy link
Collaborator

kocio-pl commented Apr 4, 2018

Could you please find a way with a name? It allows us to make sure that there's no problem with labels.

I guess you made the screenshots with Chrome, because colors seem to be odd.

@Karthoo
Copy link
Contributor Author

Karthoo commented Apr 4, 2018

So, yes, name rendering also works with ways (Kingston, Jamaica, z19):
shop video_games way z19

@kocio-pl
Copy link
Collaborator

kocio-pl commented Apr 6, 2018

The icon has two colors (cable is violet), could you fix it and make in monochromatic?

@Karthoo
Copy link
Contributor Author

Karthoo commented Apr 7, 2018

Yes, that's because I used a stroke for the cable. I guess, there is no way to make a stroke without a specific colour, or is it?

@kocio-pl
Copy link
Collaborator

kocio-pl commented Apr 7, 2018

I don't know, but make it black then probably, like the rest,that would make it monochrome.

@Karthoo
Copy link
Contributor Author

Karthoo commented Apr 7, 2018

I have this new version now, where I converted the cable into a path. Obviously, it didn't change anything on the rendering.

Copy link
Collaborator

@kocio-pl kocio-pl left a comment

Choose a reason for hiding this comment

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

After the line breaks the only thing would be probably to squash the commits into one and that's all.

project.mml Outdated
@@ -1481,7 +1481,7 @@ Layer:
'mobile_phone', 'motorcycle', 'musical_instrument', 'newsagent', 'optician', 'jewelry', 'jewellery',
'electronics', 'chemist', 'toys', 'travel_agency', 'car_parts', 'greengrocer', 'farm', 'stationery',
'laundry', 'dry_cleaning', 'beverages', 'perfumery', 'cosmetics', 'variety_store', 'wine', 'outdoor',
'copyshop', 'sports', 'deli', 'tobacco', 'art', 'tea', 'coffee', 'tyres', 'pastry', 'chocolate', 'music', 'medical_supply', 'dairy') THEN shop
'copyshop', 'sports', 'deli', 'tobacco', 'art', 'tea', 'coffee', 'tyres', 'pastry', 'chocolate', 'music', 'medical_supply', 'dairy', 'video_games') THEN shop
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would like to have some line breaks to make shop list more readable, probably before 'music'.

project.mml Outdated
@@ -1597,7 +1597,7 @@ Layer:
'mobile_phone', 'motorcycle', 'musical_instrument', 'newsagent', 'optician', 'jewelry', 'jewellery',
'electronics', 'chemist', 'toys', 'travel_agency', 'car_parts', 'greengrocer', 'farm', 'stationery',
'laundry', 'dry_cleaning', 'beverages', 'perfumery', 'cosmetics', 'variety_store', 'wine', 'outdoor',
'copyshop', 'sports', 'deli', 'tobacco', 'art', 'tea', 'coffee', 'tyres', 'pastry', 'chocolate', 'music', 'medical_supply', 'dairy') THEN shop
'copyshop', 'sports', 'deli', 'tobacco', 'art', 'tea', 'coffee', 'tyres', 'pastry', 'chocolate', 'music', 'medical_supply', 'dairy', 'video_games') THEN shop
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to have a line break here too.

@kocio-pl
Copy link
Collaborator

Do you think you can make the final changes soon?

@Karthoo
Copy link
Contributor Author

Karthoo commented Apr 15, 2018

Should be fine now

@kocio-pl kocio-pl merged commit 2101a01 into gravitystorm:master Apr 16, 2018
@kocio-pl
Copy link
Collaborator

Great, thanks a lot!

@Karthoo Karthoo deleted the video_games branch April 16, 2018 17:54
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.

2 participants