entria / react-native-fontawesome

React Native Font Awesome Icons
MIT License
176 stars 34 forks source link

6.0.1 Icons with matching names do not show regular font #54

Closed bsbechtel closed 5 years ago

bsbechtel commented 5 years ago

When upgrading to 6.0.1, if I try to use icons that have the same name between the regular and solid fonts, the icon always defaults to the solid font. Ex:

<FontAwesome type={IconTypes.FAR}>{Icons.circle}</FontAwesome>

and

<FontAwesome type={IconTypes.FAS}>{Icons.circle}</FontAwesome>

both show solid circles, even with the correct fonts installed in iOS (I have not tested Android). This is making some of the Regular icons inaccessible using this package.

bsbechtel commented 5 years ago

Any updates on this? I looked into the package codebase to see if I could find a solution, but I didn't have much luck identifying what might be the root issue.

jgcmarins commented 5 years ago

do you have any failing test or reproducible repo?

bsbechtel commented 5 years ago

I can put one together....but really, all you need to do is try setting the type on any icon that uses the same unicode characters for multiple fonts.

leandrosimoes commented 5 years ago

I just tested in Android and it's working fine as you can see at the images bellow, but unfortunately I can't test on IOS:

FAR SCREENSHOT and FAS SCREENSHOT

bsbechtel commented 5 years ago

@leandrosimoes glad to hear....still working on putting together a reproducible demo for ios. I'll link to it here as soon as I can. Thanks

bsbechtel commented 5 years ago

Here is a repo with a demo of iOS behavior: https://github.com/bsbechtel/FADemo

bsbechtel commented 5 years ago

Just curious if anyone has had a chance to investigate this on iOS? Is there anything I can do to assist?

leandrosimoes commented 5 years ago

@bsbechtel Unfortunately I haven't a IPhone or even a MAC to try to check and fix it for you, sorry about that! Anyone could please help us on this?

KalebMatthews commented 5 years ago

Not to be a bother but why was this issue closed. It exists. The fact that you cannot help with it does not mean that someone else can not. Please re-open this issue so someone who does have access to develop on iOS can resolve this issue or point a contributor to the code change that needs to happen to resolve it.

If this was closed for legitimate reasons please comment on the resolution so it does not appear to be ignored.

Thanks.

KalebMatthews commented 5 years ago

I did a little digging into this issue and I found that the correct fontFamily is being passed into the Text component inside this package. I believe the issue resides in the fact that Solid and Regular are in the same font family (FontAwesome5Free). Because of that I believe it can not determine properly which one to use. That is just a theory but wanted to give y'all something to go on.

EDIT: @jgcmarins I just found this link that will give some insight into whats going on. I am unable to find a solution as of yet.

jgcmarins commented 5 years ago

@KalebMatthews maybe we can take a look into a reproducible repo

KalebMatthews commented 5 years ago

@jgcmarins Sorry should have put a reply here when I saw it. Crazy day yesterday. To reproduce this its super simple. Just do the following:

bsbechtel commented 5 years ago

@KalebMatthews @jgcmarins I linked a reproducible repo above. Please let me know if you can't access it.

bsbechtel commented 5 years ago

Just curious if this was ever investigated any further. Thanks!

leandrosimoes commented 5 years ago

I just open a PR that separate the Icons into tree constants (RegularIcons, SolidIconst and BrandIcons) and uses the properly font families. I think that this will fix this issue. https://github.com/entria/react-native-fontawesome/pull/61

bsbechtel commented 5 years ago

That's great. Many thanks! Once it's merged, I can test it and close this.

leandrosimoes commented 5 years ago

A new version was released, please, check if in the new version this problem was fixed and feel free to reopen this issue if wasn't.

PS: REMEBER TO SEE THE NEW DOCUMENTATION BECAUSE THERE ARE SOME BREAKING CHANGES