-
Notifications
You must be signed in to change notification settings - Fork 54
feat(docs): remove Font Awesome icons from examples #1764
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1764 +/- ##
==========================================
+ Coverage 69.76% 69.77% +<.01%
==========================================
Files 868 870 +2
Lines 7484 7486 +2
Branches 2176 2200 +24
==========================================
+ Hits 5221 5223 +2
Misses 2255 2255
Partials 8 8
Continue to review full report at Codecov.
|
Quick clarification, the Teams theme shouldn't use FontAwesome icons. However, other themes may use FontAwesome icons. It is also OK for the doc site itself to use FontAwesome icons. I think only the actual examples files in Suggestions and reasoning:
|
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.
See above
@levithomason I have updated this PR. Only the example code snippets are using Teams icons instead of Font Awesome icons. The usages of missing icons from Teams theme were replaced by other meaningful icons from Teams, so that we don't need to ask for new icons from the design team. |
docs/src/examples/components/Popup/Variations/PopupExampleOffset.shorthand.tsx
Show resolved
Hide resolved
docs/src/examples/components/Popup/Variations/PopupExamplePosition.shorthand.tsx
Outdated
Show resolved
Hide resolved
docs/src/examples/components/Popup/Variations/PopupExampleOffset.shorthand.tsx
Outdated
Show resolved
Hide resolved
@@ -153,6 +155,8 @@ export default { | |||
add, | |||
'arrow-up': arrowUp, | |||
'arrow-down': arrowDown, | |||
'arrow-left': arrowLeft, |
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.
Let only design add icons. Use arrow-up
and rotate it as needed.
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.
The two icons arrow-left
, arrow-right
are simply copied from ProcessedIcons
folder. I did not design them.
@@ -152,8 +152,8 @@ export default () => ( | |||
<CodeSnippet | |||
value={` | |||
<> | |||
<Button icon='envelope' /> | |||
<Button icon='envelope' aria-label='Send message' /> | |||
<Button icon='email' /> |
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.
As @levithomason mentioned - this is general doc, I would stick with FA here.
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.
Are you sure? If user copies these code examples into the codesandbox
template, the icons won't render.
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.
I am not sure, but let's go with what you have now.
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.
Make sure you accept screen changes before merging.
This pull request is automatically deployed with Now. Latest deployment for this branch: https://stardust-react-git-feat-no-font-awesome-in-docs.stardust-ui.now.sh |
Overview
There are examples in docs that use Font Awesome icons. If such an example snippet is run on e.g. Codesandbox, the icons are not rendered at all, due to missing Font Awesome. The Font Awesome should not be used in docs at all. Relevant issue #1762
Changes
Font Awesome theme is no longer merged withTeams
theme inapp.tsx
of docsGitHub icon has been added toTeams
themeTeams
theme (they are taken fromProcessedIcons
folder)TODOWe need the following icons, that are currently missing from the
Teams
theme (they are also not in processedIcons):Copy, Copy outlineSpinnerAlign rightMinusClockThumbs down (aka Dislike)FolderFolder openUser