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: Updated icons #188

Merged
merged 13 commits into from
Jun 4, 2020
Merged

feat: Updated icons #188

merged 13 commits into from
Jun 4, 2020

Conversation

tassoevan
Copy link
Collaborator

@tassoevan tassoevan commented Apr 1, 2020

Updates our icon list. Additionally, now we:

  • keep a glyphsMapping.json file that guarantees each icon will be uniquely used by the same unicode character across future versions;
  • fix broken symbolic links for aliases;
  • prefer naming icons according to what they look instead of what feature they represent.

@tassoevan tassoevan force-pushed the feat/new-icons branch 2 times, most recently from 8fb7715 to 4dcc1f1 Compare April 1, 2020 14:50
@tassoevan tassoevan changed the title feat: New icons feat: Updated icons Apr 1, 2020
packages/icons/src/neutral/home.svg Outdated Show resolved Hide resolved
packages/icons/src/neutral/home.svg Outdated Show resolved Hide resolved
Copy link
Member

@gabriellsh gabriellsh left a comment

Choose a reason for hiding this comment

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

Some of the icons cited are used today in the main codebase with these names, but I think all icon names should follow a pattern in order to facilitate usage. Taking that in mind:

edit should be pencil
add-reaction should be add-emoji.
change should be arrow-change or arrow-loop, or atleast start with arrow- (I think arrow-left-right would be most suitable)
import should be arrow-down-box
multiline should be arrow-return
mobile should be smartphone or maybe mobile-device
new-chat should be pencil-box
post should be document
queue could be box-stacked
share should be arrow-up-box
transfer should be arrow-right-user
team should be user-stacked
upload should be arrow-up-cloud or maybe upload-cloud
sign-out should be arrow-right-box

These ones have duplicates:
attachment - file-generic - clip
back - arrow-back - reply-directly
cancel - circle-cross
checkmark-circled - circle-check
circled-arrow-down - circle-arrow-down - download
discover - globe
edit-rounded - new-chat
th-list - sheet - files-sheet
audio - files-audio
add-user - user-plus
jump - jump-to-message - arrow-jump
map-pin - pin-map
files-video - video
files-zip - zip
send-active - send-filled
shield - shield-alt
sort-amount-down - sort
reload - refresh

These do not have a direct name-to-symbol relation, but I didn't find a better name for it
canned-response
live
livechat
new-window
podcast
thread

I triple checked to see if I didn't miss any, but there might be a couple that got overseen.

@tassoevan
Copy link
Collaborator Author

tassoevan commented Jun 3, 2020

@gabriellsh

  • I've renamed add-reaction to emoji-plus, and others as well.
  • I'll apply most of the suggestions, but mobile exists as a mirror to desktop; should it be renamed too?
  • The duplicates exist because they're temporary aliases to older icon names that we need to deprecate on RC first.

Addendum: check https://open.rocket.chat/public/icons.html for a list of the icons to be replaced.

@gabriellsh
Copy link
Member

It's okay for me. I just mentioned mobile because the word itself is an adjective, but Desktop isn't. Maybe it could be mobile-phone, but I think there's no problem in having an odd one out.

@tassoevan tassoevan requested a review from a team June 3, 2020 19:34
@tassoevan tassoevan marked this pull request as ready for review June 3, 2020 19:35
@ggazzo ggazzo merged commit 184e8ed into develop Jun 4, 2020
@ggazzo ggazzo deleted the feat/new-icons branch June 4, 2020 00:58
gabriellsh added a commit that referenced this pull request Jun 4, 2020
…into new/playground

* 'develop' of github.com:RocketChat/Rocket.Chat.Fuselage:
  feat: MP3 encoder (#235)
  fix: Missing legacy icons (#238)
  perf: Keyed children (#231)
  feat: Updated icons (#188)
  perf: Generated classNames from props (#230)
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