Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

feat: add SVG icons set #544

Merged
merged 8 commits into from
Dec 3, 2018
Merged

feat: add SVG icons set #544

merged 8 commits into from
Dec 3, 2018

Conversation

kuzhelov
Copy link
Contributor

Export initial set of icons requested here: #441 (comment)

Non-working icons were fixed.

@@ -1,11 +1,3 @@
import { FontIconSpec } from '../../../../types'

const fontAwesome = (iconCode: string): FontIconSpec => ({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

those are not used

@mnajdova
Copy link
Contributor

Couple of thoughts, for individual icons:
like - the regular and outlined icons are the same (both outlined)
redo - was not able to find it (maybe has different name)
translate - was not able to find it (maybe has different name)
immersive reader - was not able to find it (maybe has different name)
add participants - we have just add participant (should there be two different icons)
alert (triangle with exclamation mark) - outline same as regular
exclamation mark for important messages - was not able to find it (maybe has different name)
add to calendar - we have just calendar
member joined (control message in conversation) - was not able to find it (maybe has different name)
member left (control message in conversation) - regular same as outlined

Other icons:
Call end - regular and outline same (both filled)
Call control present new - regular and outline same (both outlined)
Call control stop resenting new - regular and outline same (both outlined)
Mics - regular and outline same (both outlined)
Mics off - regular and outline same (both outlined)

@kuzhelov not sure if this is the scope of this PR, but let's try to fix at least the new icons added.

@mnajdova mnajdova assigned mnajdova and kuzhelov and unassigned mnajdova Nov 30, 2018
@alinais alinais added this to kuzhelov in Core Team Nov 30, 2018
@kuzhelov
Copy link
Contributor Author

kuzhelov commented Nov 30, 2018

@mnajdova

Thanks a lot for thorough review, here is response for those where problem still exists:

  • error - here we have two icons where one is rendered as outlined in both cases, another as filled:
    • always outlined: callalert
    • always filled: error
    • thinking about removing this from current PR before it will be clarified how we are going to handle this (or, maybe even use another icon?)
  • same story for like and liked

Missing ¯\(ツ)/¯ ones:

  • redo - wasn't able to find either, has introduced retry instead :)
  • immersive reader
  • add calendar

Same icons for regular and outlined

This problem origins from Preprocessed Icons, applied to:

  • call
  • call end
  • call control present new
  • call control stop resenting new
  • leave
  • mic
  • mic off
  • redbang

Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Approving this changes, but let's create an issue or track somewhere the icons that needs fixing.. @codepretty please take a look into the list of icons in the comments of this PR.

@kuzhelov kuzhelov merged commit 97e6f7e into master Dec 3, 2018
@kuzhelov kuzhelov deleted the feat/add-icon-set branch December 3, 2018 15:37
@codepretty
Copy link
Collaborator

codepretty commented Dec 3, 2018

For processed icons, I just ported all the icons included in the Teams app today. A number of these icons are incorrect/outdated/duplicates, but we just haven't had time to go through the current app and update everything. If you have questions about which icons to use, I'd recommend first looking at https://teams.microsoft.com/_?ring=ring0#/toolkit and then reaching out to myself or @notandrew. However, for the ones listed below...

  • error - here we have two icons where one is rendered as outlined in both cases, another as filled:

I'll sync with Design to see what the correct error icon to use is.

  • same story for like and liked

I think these should be combined as the outline and filled states of one icon, respectively.

Missing ¯\(ツ)/¯ ones:

redo - wasn't able to find either, has introduced retry instead :)

The retry icon is outdated. The correct icon to use is refresh which has both a filled and outline state.
image

immersive reader

This is named icons-read-aloud (or readaloud icon in processed icons)
image

add calendar

I'm not sure what add calendar means? are you referring to the meetingnew icon?
image

Same icons for regular and outlined

  • call
  • redbang

I see a filled and outline state for these icons. redbang is hard to tell because it's subtle, but it does exist.

  • call end
  • call control present new
  • call control stop resenting new
  • leave
  • mic
  • mic off

I'll sync with Design for these icons.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Core Team
  
kuzhelov
Development

Successfully merging this pull request may close these issues.

None yet

4 participants