birkir / react-native-sfsymbols

SF Symbols in your iOS app
134 stars 12 forks source link

Color prop does not render correct icon color #10

Open jordantomax opened 3 years ago

jordantomax commented 3 years ago

Using version 1.0.2, I am attempting to render an icon like so:

<SFSymbol
  name='checkmark'
  weight={SFSymbolWeight.REGULAR}
  scale={SFSymbolScale.MEDIUM}
  color='black'
  style={{
    width: 32,
    height: 32
  }}
/>

When I do so, the icon disappears completely. The same thing happens when using #000000. If I use a lighter shade such as #cccccc, The color shows up, but it is very light and has a tint of red. Certain colors seem to work properly, such as white or rgba(60, 60, 67, 0.3), but most other colors produce unexpected results.

Digging into this, I see that you're using react's processColor. Looking at their tests, It seems like the colors that I entered should be valid.

I have set up a brand new react native project using react-native init, and I see the same results, so I hope this is easily reproducible.

Thanks for your help!

MoOx commented 3 years ago

Could you try removing processColor call in the JS and see if that’s better?

jordantomax commented 3 years ago

@MoOx Sure. I'll have to set the project up locally. I will try it soon.

MoOx commented 3 years ago

You can try to edit directly the file from node_modules (and if that works, use patch-package until a fix comes out)

birkir commented 3 years ago

Found the issue, looks like we have to rename the color property.

https://github.com/facebook/react-native/issues/26702

jordantomax commented 3 years ago

@birkir Thanks for looking into that. Would that bug still be a problem if I'm not defining a color in style? Are you suggesting that the color prop is never making it to the RNSFSymbol components? Thanks again.

birkir commented 3 years ago

Yes, I published a new version where I changed the internal prop name for color (iconColor), the API hasn't changed.

jordantomax commented 3 years ago

@birkir I'm getting the same error can't find variable: React as with version 1.0.1. Are you testing this locally and not seeing that error?

MoOx commented 3 years ago

It’s normal because compiled tsx omit the React import… I warned about this, my PR fixed this…

jordantomax commented 3 years ago

@MoOx Understood, thanks. I'll try again once @birkir updates to include your changes.

MoOx commented 3 years ago

It's not planned as explained here... https://github.com/birkir/react-native-sfsymbols/issues/2#issuecomment-797384119 Sorry but I am too lazy and for now I am using the branch of my fork until this is solved. I already spend our to make this module work to have some of my work rejected without notices and a module on npm that is still unusable... :(

jordantomax commented 3 years ago

There must be some kind of misunderstanding here. @birkir you understand that the code currently hosted on the NPM registry does not work? It throws errors when incorporated into a react native project (because it hasn't been properly compiled?). Are you currently using this in a project and it's working for you? Please advise.

MoOx commented 3 years ago

It should not be compiled. None of the RN modules I know are compiled before published. Metro handles compilation with Babel and can handle TypeScript for a while.

jordantomax commented 3 years ago

Oh, 🤔 . That's surprising to me too actually, I wouldn't expect metro to break with pre-compiled code. But I admittedly have never published a library for React Native. I'm looking at the node_modules that are downloaded for @react-navigation, another library that I use, and I see that they have pre-compiled to commonjs with the same peer dependencies as this library, so I'm confused as to why it's not working. Will await @birkir's thoughts.

happymaskterriblefate commented 3 years ago

@jordantomax I was able to get passed the can't find variable: React error by plopping global.React = React at the top of my App.js file.

As for the original point of this issue -- even with the renaming of color to iconColor, the results are unpredictable :/

birkir commented 3 years ago

Fixed now.