-
Notifications
You must be signed in to change notification settings - Fork 55
Conversation
@@ -1,11 +1,3 @@ | |||
import { FontIconSpec } from '../../../../types' | |||
|
|||
const fontAwesome = (iconCode: string): FontIconSpec => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those are not used
Couple of thoughts, for individual icons: Other icons: @kuzhelov not sure if this is the scope of this PR, but let's try to fix at least the new icons added. |
Thanks a lot for thorough review, here is response for those where problem still exists:
Missing ¯\(ツ)/¯ ones:
Same icons for
|
There was a problem hiding this 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.
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...
I'll sync with Design to see what the correct error icon to use is.
I think these should be combined as the outline and filled states of one icon, respectively. Missing ¯\(ツ)/¯ ones:
The
This is named
I'm not sure what add calendar means? are you referring to the Same icons for
|
Export initial set of icons requested here: #441 (comment)
Non-working icons were fixed.