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

Color prop does not render correct icon color #10

Open
jordantomax opened this issue Mar 13, 2021 · 15 comments
Open

Color prop does not render correct icon color #10

jordantomax opened this issue Mar 13, 2021 · 15 comments

Comments

@jordantomax
Copy link

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
Copy link
Contributor

MoOx commented Mar 13, 2021

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

@jordantomax
Copy link
Author

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

@MoOx
Copy link
Contributor

MoOx commented Mar 14, 2021

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

@birkir
Copy link
Owner

birkir commented Mar 16, 2021

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

facebook/react-native#26702

@jordantomax
Copy link
Author

@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
Copy link
Owner

birkir commented Mar 18, 2021

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

@jordantomax
Copy link
Author

@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
Copy link
Contributor

MoOx commented Mar 21, 2021

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

@jordantomax
Copy link
Author

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

@MoOx
Copy link
Contributor

MoOx commented Mar 22, 2021

It's not planned as explained here... #2 (comment)
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
Copy link
Author

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
Copy link
Contributor

MoOx commented Mar 23, 2021

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
Copy link
Author

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
Copy link

@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
Copy link
Owner

birkir commented Jun 23, 2021

Fixed now.

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

No branches or pull requests

4 participants