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

Support web as a first class citzen #367

Open
brunolemos opened this issue Dec 26, 2016 · 18 comments
Open

Support web as a first class citzen #367

brunolemos opened this issue Dec 26, 2016 · 18 comments

Comments

@brunolemos
Copy link
Contributor

brunolemos commented Dec 26, 2016

react-native-web is great to share code between mobile and web, but currently react-native-vector-icons does not support it.

I see that there are some instructions in the docs, that's not enough because the real solution is to be able to use the same code without any change:

import Icon from 'react-native-vector-icons/ABC';
...
    <Icon name="xyz" />
...

I also just found the PR #133 which unfortunately didn't get merged.
I hope we can continue the discussion to support web platform out of the box.

Maybe also distributed multiple font formats like WOFF.

@oblador
Copy link
Owner

oblador commented Dec 27, 2016

FWIW, neither iOS nor Android works just out of the box. Is the problem that you think integrating fonts are too cumbersome or are the docs incorrect?

@brunolemos
Copy link
Contributor Author

Currently a ton of people uses create-react-app in their web project, which supports react-native-web by default. That means they don't need to do anything to be able to import a View component (or Text, Image, ...) and render it in the browser.

The docs solution does not expose the Icon component. That's basically the issue here.
That means every person that uses react-native-vector-icons needs to replicate the Icon component behaviour to be able to share the same code between platforms.

@oblador
Copy link
Owner

oblador commented Jan 19, 2017

Not sure I follow here, what do you mean by not exposing the Icon component?

@brunolemos
Copy link
Contributor Author

brunolemos commented Feb 25, 2017

Sorry, I was not clear in my previous comments.
I'm able to use react-native-vector-icons in the web successfully by following the docs.

But that requires me to:

  1. Change webpack config (eject from create-react-app)
  2. Create a style tag to inject the fonts I was using

The intention of this issue is to remove the above requirements.

Possible solutions:

  1. Remove the need to edit webpack by creating a ES5 bundle (see For web! #415. ps: create-react-app projects already support ttf files, so ES5 would be enough to fix the invalid token import console error)
  2. Injecting style could be in the core of the lib. If I import react-native-vector-icons/XXX, inject XXX for me. (I think Added support web-platform (Icon & Icon.Button) #133 did that)

That's it 🙂

@oblador
Copy link
Owner

oblador commented Jul 30, 2017

First solution should be implemented now, I'm happy to receive further PRs to make this even easier but at the same time I don't want to make lockins to setup (webpack etc) in the source files. One thing that would be quite useful is to also distribute other versions of the font files (woff etc).

@darrencruse
Copy link

darrencruse commented Dec 29, 2017

@oblador Thanks for this library and fwiw I was able to follow the README and get react-native-vector-icons working in my (first!) react-native-web project so that's great.

Like the OP I did think it's unfortunate the library needed me to add that "initialization" code to add those style tags though. Separate from the uses of the Icon tags themselves. e.g. What if I was pulling in a third party component that happen to use react-native-vector-icons and I didn't even know it.

So anyway re your comment: "I'm happy to receive further PRs to make this even easier..."

I was looking at PR #133 seeing if I followed what the issue was with it to see if maybe I could help out...

I'm not sure I follow what the issue was though isn't that PR attempting to do what your README says but in a decentralized/automatic way that doesn't depend on that "separate" initialization to create the style tags?

Was the issue that it would create multiple style tags for the same font because that code is called for every use of an Icon tag regardless of what font? Was that the issue?

(i.e. if I added a fix for that would the PR be accepted?)

@necolas
Copy link

necolas commented Feb 19, 2018

Unfortunately, single-export, font-based icons are not great for web apps vs using SVG (http://nicolasgallagher.com/making-svg-icon-libraries-for-react-apps/). You'd probably need to alter the API to something like the one in that post for it to support incremental loading and alternative implementations like SVG

@dantman
Copy link
Contributor

dantman commented Feb 19, 2018

We don't need separate icon components to use SVG components like <g><path d='...'></g>. We could download the SVG versions of all the font icon libraries, putting them in their own icon name to JSX map, and say web support is limited to the libraries that offer that. Then in .web.js versions of things we could use SVG instead of fonts.

@necolas
Copy link

necolas commented Feb 19, 2018

putting them in their own icon name to JSX map

That still causes all icons to be downloaded, whether or not you use them. Not good for web apps

@dantman
Copy link
Contributor

dantman commented Feb 19, 2018

All icons for one icon library. Same limitation as icon fonts.

If you want individual icon loading we could go with strings and innerHTML instead. Then we'd just need some separate way for the application to globally tell Icon the answer to "Where can I find this icon's src to download it?".

@necolas
Copy link

necolas commented Feb 19, 2018

If you read the article I linked to, that's how you avoid these problems

@dantman
Copy link
Contributor

dantman commented Feb 19, 2018

@necolas I read the article you linked. It does not implement incremental loading in the way rnvi would require. All it does is use a createIconComponent function to create icons required separate files. This is not how rnvi works and isn't a valid way to expect incremental loading to be handled.

Incremental loading for rnvi would have to be a HTTP handler that would download the SVG markup using xhr/fetch when an <Icon name='foo' /> is mounted.

@necolas
Copy link

necolas commented Feb 19, 2018

You can't incrementally load an icon font, and my point is that the API of this library is inherently problematic (as your xhr suggestion highlights)

@tafelito
Copy link

tafelito commented Jun 5, 2018

@necolas in the article you linked you mention that you can use the same approach to use other type of packages for other use cases like React Native. What are you using for that, react-native-svg or any other solution?

@necolas
Copy link

necolas commented Jun 5, 2018

Yes react-native-svg should work as not many SVG features are necessary for the icon use case.

@tafelito
Copy link

tafelito commented Jun 5, 2018

In that case, you'll have to import the icons from a different namespace for each platform, right? @acme/react-icons for react @acme/react-native-icons for react native, wouldn't that break the all purpose of using react-native-web when you want to share code between the two?

@necolas
Copy link

necolas commented Jun 5, 2018

Use file name extensions for platform-specific implementations – IconName.{native,web}.js – as documented in RN and RNW. Then the bundler can pick the right file for the platform without any difference to how you import the icon.

@tafelito
Copy link

tafelito commented Jun 5, 2018

ok I see, I was thinking on creating different packages that's why. So the react and rn components has to be on the same npm package (@acne/react-icons) and use a different extensions, is that what you meant?

And I still would need a way to convert the optimized svg content to a react-native-svg format, but anyway, thanks for the answers!

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

6 participants