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 new icons #441

Merged
merged 6 commits into from
Sep 14, 2018
Merged

Add new icons #441

merged 6 commits into from
Sep 14, 2018

Conversation

samanpwbb
Copy link
Contributor

@samanpwbb samanpwbb commented Aug 31, 2018

@natslaughter for review

add car-rental, car-repair, casino, charging-station, furniture, globe, hardware, jewelry, optician, paint, shoe, watch, watermill, windmill

car-rental-11
car-rental-15
furniture-11
furniture-15
globe-11
globe-15
hardware-11
hardware-15
jewelry-11
jewelry-15
optician-11
optician-15
paint-11
paint-15
shoe-11
shoe-15
watermill-11
watermill-15
windmill-11
windmill-15
car-repair-11
car-repair-15
casino-11
casino-15
charging-station-11
charging-station-15
watch-11
watch-15


closes #423, closes #422, closes #421, closes #420, closes #419, closes #418, closes #417, closes #410, closes #409, closes #408, closes #404, closes #376

add car-rental, car-repair, casino, charging-station, furniture, globe, hardware, jewelry, optician, paint, shoe, watch, watermill, windmill
@samanpwbb samanpwbb requested a review from natslaughter August 31, 2018 22:44
@natslaughter
Copy link
Contributor

@samanpwbb these are looking superb.
I'll post feedback by icon as I go through geometry reviews and placing the icons on a map.

First up, watermill

screen shot 2018-09-06 at 12 02 56 pm

The 15 pixel icon looks great, which will be the only size we see on the Mapbox core-styles. But the 11-pixel icon does seem to be a bit blurry around the wheel of the mill:

screen shot 2018-09-06 at 12 06 25 pm

It looks like if the wheel were to shift slightly to align with the grid, there would be a little more negative space.

screen shot 2018-09-06 at 12 17 14 pm

screen shot 2018-09-06 at 12 17 28 pm

screen shot 2018-09-06 at 12 17 49 pm

@natslaughter
Copy link
Contributor

car-repair 15 pixel looks good:
screen shot 2018-09-06 at 11 46 43 am

11 pixel seems to be a little tight, and although the image below on the right isn't pixel aligned and is a rough sketch, I think it begins to show a possibility for more space between the car and wrench.

screen shot 2018-09-06 at 1 01 33 pm

@natslaughter
Copy link
Contributor

charging-station 15-pixel looks good

screen shot 2018-09-06 at 1 40 37 pm

11-pixel might be able to house a little more negative space for the lightning bolt, as seen below in the two sketches to the right (left is the original icon). As it is currently, it's a bit difficult to read.

screen shot 2018-09-06 at 1 29 44 pm

@natslaughter
Copy link
Contributor

optician 15-pixel looks good
screen shot 2018-09-06 at 1 54 03 pm

I guess the 11-pixel could be better pixel aligned, unless you think the blurry grey-ness causes a desired effect.

screen shot 2018-09-06 at 1 54 55 pm

@natslaughter
Copy link
Contributor

I'm finding the boot to be a bit 'clunky' and I wonder if decreasing the opening width would help? Also, did you try using a shoe symbol instead of a boot?

screen shot 2018-09-06 at 2 24 39 pm

@samanpwbb
Copy link
Contributor Author

@natslaughter thanks for the review 🙏 will have updates for ya here tomorrow

@samanpwbb
Copy link
Contributor Author

Also, did you try using a shoe symbol instead of a boot?

Yeah, I went with a boot because i had a hard time getting lace detail to look good and fit in with the rest of the icon set. Boots have stronger silhouettes so they don't need to rely on negative space as much. On the other hand, a shoe might be a better, more general symbol. You think I should give shoe another shot @natslaughter?

@samanpwbb
Copy link
Contributor Author

@natslaughter updated all the icons based on feedback. Over to you again.

@natslaughter
Copy link
Contributor

@samanpwbb these look tremendous. Thank you for your contribution. Please merge and close at your leisure. ❤️

screen shot 2018-09-14 at 10 08 48 am

screen shot 2018-09-14 at 10 08 28 am

screen shot 2018-09-14 at 10 10 19 am

screen shot 2018-09-14 at 10 09 20 am

screen shot 2018-09-14 at 10 11 49 am

screen shot 2018-09-14 at 10 11 38 am

@samanpwbb samanpwbb merged commit 22fcc0e into master Sep 14, 2018
@samanpwbb samanpwbb deleted the samanpwbb/streets-v8 branch September 14, 2018 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants