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

feat(icons): Updated contact and globe icons #767

Merged
merged 4 commits into from
Jul 12, 2022
Merged

Conversation

filipemarins
Copy link
Contributor

Proposed changes (including videos or screenshots)

Issue(s)

Further comments

Update contact and globe icons

dougfabris
dougfabris previously approved these changes Jul 11, 2022
Copy link
Collaborator

@tassoevan tassoevan left a comment

Choose a reason for hiding this comment

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

I know this is a stretch from the goal of this pull request, but I can't deal with the fact that contact violates our standard, stated at the README:

The filenames define the icon names, so they should stick with a basic rule: an icon name must describe the icon shape, not its utility.

In this scenario, the icon should be renamed as address-book and an alias for it must be placed as a symlink at src/aliases/contact.svg.

@tassoevan tassoevan changed the title chore: update contact and globe icons feat(icons): Updated contact and globe icons Jul 12, 2022
@filipemarins
Copy link
Contributor Author

I know this is a stretch from the goal of this pull request, but I can't deal with the fact that contact violates our standard, stated at the README:

The filenames define the icon names, so they should stick with a basic rule: an icon name must describe the icon shape, not its utility.

In this scenario, the icon should be renamed as address-book and an alias for it must be placed as a symlink at src/aliases/contact.svg.

Done <3

@tassoevan tassoevan merged commit 57eecb5 into develop Jul 12, 2022
@tassoevan tassoevan deleted the update/icons branch July 12, 2022 18:56
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.

3 participants