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

refactor(icons): invalid names #262

Merged
merged 2 commits into from
Nov 27, 2018
Merged

refactor(icons): invalid names #262

merged 2 commits into from
Nov 27, 2018

Conversation

emmenko
Copy link
Member

@emmenko emmenko commented Nov 26, 2018

Fixes #259
Closes #260

Renamed icons listed here

image

<?xml version="1.0" encoding="UTF-8"?>
<svg width="24px" height="24px" viewBox="0 0 24 24" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
<!-- Generator: sketchtool 45.2 (43514) - http://www.bohemiancoding.com/sketch -->
<title>arrow-left</title>
Copy link
Member Author

Choose a reason for hiding this comment

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

PS: the names here are still "wrong". Should we update them as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Uff. Good one. No opinion 😐

Copy link
Member Author

Choose a reason for hiding this comment

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

I went ahead and fixed the names: f38e012

I also noticed some inconsistency in the SVG structures. I already checked and notified the design team about it.

<g id="Icons" stroke="none" stroke-width="1" fill-rule="evenodd">
<g id="MC-icon-set" transform="translate(-24.000000, -936.000000)" fill="#000000">
<g id="Directions" transform="translate(24.000000, 888.000000)">
<g id="Arrow-Left" transform="translate(0.000000, 48.000000)">
Copy link
Member Author

Choose a reason for hiding this comment

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

Same here with the id

Copy link
Contributor

@tdeekens tdeekens left a comment

Choose a reason for hiding this comment

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

👌🏿🤩. Sorry for the confusion and derailed discussion before we all contributed to.

Copy link
Contributor

@dferber90 dferber90 left a comment

Choose a reason for hiding this comment

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

👍

@emmenko emmenko force-pushed the nm-fix-rename-icons branch from f38e012 to be5f2e2 Compare November 27, 2018 09:44
@emmenko emmenko merged commit 0e8ee88 into master Nov 27, 2018
@emmenko emmenko deleted the nm-fix-rename-icons branch November 27, 2018 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants