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

Add platform specific handling of font files #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

noinfo
Copy link

@noinfo noinfo commented Jun 7, 2018

Makes webfonts work in iOS and Android besides having to use different names in 'CSS' style.

@@ -23,9 +23,10 @@ class Icon extends Component {
}
}

const fontFamily = Platform.OS === 'ios' ? 'FontAwesome' : 'fontawesome-webfont';
Copy link
Member

Choose a reason for hiding this comment

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

can't we have the same FontAwesome name in both ios and android?

Copy link
Author

Choose a reason for hiding this comment

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

That does not seem to work since iOS uses the Font Name and Android uses the filenames. The pull requests amends this so that you can simply copy the files from the FontAwesome4.7.0 Zip. Even renaming does not work for Android as font names should be all lowercase.

I don't know if that is a recent change, but otherwise it won't work for my react native 0.55.4 projects.

Copy link
Contributor

@jgcmarins jgcmarins left a comment

Choose a reason for hiding this comment

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

makes sense

@sibelius
Copy link
Member

sibelius commented Jun 7, 2018

Can we make the user pass this as a prop instead? Having the default as the old value?

@noinfo
Copy link
Author

noinfo commented Jun 7, 2018 via email

@sibelius
Copy link
Member

sibelius commented Aug 1, 2019

check if this helps #61

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