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.

Sign up for GitHub

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

Jump to bottom

Add support for Duotone Icons #59

Merged
merged 1 commit into from
May 13, 2020
Merged

Add support for Duotone Icons #59

merged 1 commit into from
May 13, 2020

Conversation

schonfeld
Copy link
Contributor

Addresses #45 and #24, and adds support for Duotone Icons.

This adds the [optional] secondaryColor and secondaryOpacity props, which will fill secondary Duotone layers with the given color and opacity. If no values are explicitly given, as per FA's docs, this will default to using the primary color at 40% opacity.

robmadole
robmadole requested changes May 13, 2020
View reviewed changes
Copy link
Member

@robmadole robmadole left a comment

Choose a reason for hiding this comment

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

Absolutely fantastic! Just one typo and we'll get this merged.

README.md Outdated
### Duotone

```javascript
<FontAwesomeIcon icon="coffee" color="blue" secondaryColor="red" secondaryOpacoty={ 0.4 } />
Copy link
Member

Choose a reason for hiding this comment

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

Quick typo "Opacoty"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ended up fixing this & doing a git rebase fixup... Wasn't worth littering the commit log with yet another commit.

@schonfeld
Copy link
Contributor Author

schonfeld commented May 13, 2020 •
edited
Loading

Oops. Also, is the readme ToC auto-generated? I don't think I've added an entry for the Duotone section under Features...

Edit: just added it now, along with typo fix

@schonfeld schonfeld force-pushed the feat.duotone-support branch from 5bc69d1 to 7f70f73 Compare May 13, 2020 17:04
@robmadole
Copy link
Member

Looks great. I don't think the TOC is auto-generated but some of our other projects do this. We just don't have it setup here yet.

Thanks for this @schonfeld . Great work.

@robmadole robmadole merged commit adb1186 into FortAwesome:master May 13, 2020
@robmadole
Copy link
Member

Hmm, we got some test failures. I'll take a look at these soon and get a release cut.

https://github.com/FortAwesome/react-native-fontawesome/runs/671675598?check_suite_focus=true

@schonfeld
Copy link
Contributor Author

How strange. It's still passing fine on my local?

@schonfeld schonfeld deleted the feat.duotone-support branch May 13, 2020 18:10
@robmadole
Copy link
Member

We have a matrix of versions we are testing on. So I think something goofy is going on there. I'll dig into it as soon as I can. If you wanna beat me to it you are welcome to but I don't want to put this CI burden on you.

@schonfeld
Copy link
Contributor Author

Sure enough, rn-svg changed how it uses the fill prop. I'll start a new PR to fix those tests.

This was referenced May 14, 2020
Merged
Merged
@likern likern mentioned this pull request Nov 1, 2021
Closed
3 tasks
@github-actions github-actions bot mentioned this pull request Nov 6, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robmadole robmadole robmadole approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@schonfeld @robmadole