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

Centering 38 maki icons #565

Merged
merged 4 commits into from
May 30, 2022
Merged

Centering 38 maki icons #565

merged 4 commits into from
May 30, 2022

Conversation

Udobnyj
Copy link
Contributor

@Udobnyj Udobnyj commented May 27, 2022

Based on this task, I adjusted 46 of our icons to work better with the background.

I didn't change the vector. I just moved them inside the artboards. But six of them are not along the pixel grid but 0.5px: shelter, bowling-alley, basketball, ferry-JP, amusement-park, and religious-muslim.

MAIN STUDIO TEST LINK

Concreete 0.5px icons links
I didn't find shelter, bowling-alley, and ferry-JP, so I replaced the other icons.

shelter

bowling-alley

ferry-JP

basketball

amusement-park

religious-muslim

@Udobnyj Udobnyj requested review from tristen and willymaps as code owners May 27, 2022 09:33
@Udobnyj
Copy link
Contributor Author

Udobnyj commented May 30, 2022

I was thinking about 0.5px moved icons.
I think I have an idea of how to fix it. I can do it in this task. Along with sizing, I can adjust them to fit the pixel grid

@tristen
Copy link
Member

tristen commented May 30, 2022

@Udobnyj this is a little tedious, but could you share a map example with these icons using a background? Also perhaps a map example showing the before with backgrounds? Just to get a better sense of how centering these addresses visual inconsistency?

@tristen
Copy link
Member

tristen commented May 30, 2022

But six of them are not along the pixel grid but 0.5px: shelter, bowling-alley, basketball, ferry-JP, amusement-park, and religious-muslim.

Hmm @Udobnyj we should avoid offsetting these to 0.5px just for centering and consider redrawing these instead.

Edit Oh! I missed your comment here #565 (comment)

Screen Shot 2022-05-30 at 10 46 51 AM

@Udobnyj
Copy link
Contributor Author

Udobnyj commented May 30, 2022

I decided to revert ten icons (6 of those that I moved by 0.5 pixels and four more). As I wrote above, I will correct them in this resizing task. Therefore, this push now has 38 changed icons (I forgot about charging-station.svg and added it).

Screen Shot 2022-05-30 at 19 48 18

LINK to the combining map

Below are links to specific ones. But it's too much of them to test it on the map. I think a better way will be to check others in my Figma file

May-30-2022 20-19-50

Shops
Screen Shot 2022-05-30 at 20 00 34

Fuel
Screen Shot 2022-05-30 at 19 58 45

Playground
Screen Shot 2022-05-30 at 19 57 29

Restaurant-pizza
Screen Shot 2022-05-30 at 19 59 37

Mountain
Screen Shot 2022-05-30 at 20 04 32

@Udobnyj Udobnyj changed the title Centering 46 maki icons Centering 38 maki icons May 30, 2022
Copy link
Member

@tristen tristen left a comment

Choose a reason for hiding this comment

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

🏅

@Udobnyj Udobnyj merged commit a44bea3 into main May 30, 2022
@Udobnyj Udobnyj deleted the centered-icons branch July 3, 2022 14:28
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