FontAwesome 5 #776

hampustagerud · 2018-07-17T01:57:12Z

I have implemented a way to use FontAwesome 5, both Free and Pro versions (see instructions included in FONTAWESOME5.md). This should resolve #613, close #695, close #713 and close #771. I have tested regular icons on both Android and iOS. ~~Button and similar extra classes currently only works properly on Android (defaults to Regular on iOS) but should be easily fixed with some more time.~~ People just seemed really interested in this feature so I wanted to make it available as soon as possible 😃

Also, there is no breaking changes for those using FA4. There is a separate glyphmap and new classes using it 👍🏻

I have now automated the upgrading and building, added it to the directory and IconExplorer (it's not a 100% functional due to how FontAwesome5 is built and the directories are built but most icons are visible). It should also be fully functional with Buttons and TabBarItems (I have only tested Button though) on both Android and iOS, be sure to read the manual on how to access these since it is not really the same as with the standard fonts.

+ GlyphMap for FontAwesome 5.1.0 Pro + Font files for FontAwesome 5.1.0 Free + Implementation enabling FontAwesome5 icons Handles the copying and renaming of FontAwesome5 Pro font files Removed debug code

Now a default import from a separate file, instead of creating it using FA5iconSet(bool).

Fixes to pass tests

oblador · 2018-07-17T17:28:07Z

lib/create-icon-set.js

@@ -38,11 +43,13 @@ export default function createIconSet(glyphMap, fontFamily, fontFile) {
      color: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
      children: PropTypes.node,
      style: PropTypes.any, // eslint-disable-line react/forbid-prop-types
+      extraStyle: PropTypes.any, // eslint-disable-line react/forbid-prop-types


What is this extraStyle used for? Can't see it referenced elsewhere.

This was part of an earlier solution which I seemed to have missed when cleaning up the code, on it!

oblador · 2018-07-17T17:32:36Z

scripts/fontawesome5.sh

+
+echo "Creating glyphmaps"
+
+mkdir -p temp


Can you do the following like the other build scripts instead?

TEMP_DIR=`mktemp -d -t rnvi` mkdir $TEMP_DIR

oblador · 2018-07-17T17:35:29Z

scripts/fontawesome5.sh

+
+echo "Installing FontAwesome5"
+
+npm install @fortawesome/fontawesome-free --no-shrinkwrap


Think we can do npm pack instead to avoid polluting the node_modules folder

npm pack is probably better yea...

oblador · 2018-07-17T17:39:07Z

scripts/fa5upgrade.sh

+echo "Copying font files"
+
+cp ./node_modules/@fortawesome/fontawesome-pro/webfonts/fa-brands-400.ttf Fonts/FontAwesome5_Brands.ttf
+cp ./node_modules/@fortawesome/fontawesome-pro/webfonts/fa-light-300.ttf Fonts/FontAwesome5_Light.ttf


It's not ideal to install these to the node_modules folder IMHO as it is usually not checked in to git and thus wouldn't work out of the box if you're more than one dev. Would it be possible to either use the fonts from the @fortawesome/fontawesome-pro package directly or output them to the main project's directory?

Not sure, the API requires direct access to the files but maybe the FontAwesome5Pro module could point backwards? I'll look into it!

oblador · 2018-07-17T17:39:50Z

package.json

    "build-feather": "./scripts/feather.sh",
    "build-foundation": "./scripts/foundation.sh",
    "build-ionicons": "./scripts/ionicons.sh",
    "build-materialicons": "./scripts/materialicons.sh",
    "build-materialcommunityicons": "./scripts/materialcommunityicons.sh",
    "build-octicons": "./scripts/octicons.sh",
    "build-zocial": "./scripts/zocial.sh",
-    "build-simplelineicons": "./scripts/simplelineicons.sh"
+    "build-simplelineicons": "./scripts/simplelineicons.sh",
+    "fa5upgrade": "./scripts/fa5upgrade.sh"


This should probably exported as a bin instead of as a script.

I'm not too familiar with this aspect of npm so I took a shot. Would adding it to bin make it runnable from the app folder (instead of navigating into the library folder for upgrading)? My original though was that you could just run something like npm run fa5upgrade from the top folder which would be easier (and cleaner).

Yep, exactly :-)

oblador · 2018-07-17T17:57:09Z

FontAwesome5.js

+    };
+
+    static defaultProps = {
+      size: DEFAULT_ICON_SIZE,


Do we have to duplicate the defaults here? Should be enough if they are applied later.

oblador · 2018-07-17T17:59:06Z

FontAwesome5.js

+    static Light = LightSet;
+    static Regular = RegularSet;
+    static Solid = SolidSet;
+


Have you thought about implementing getImageSource or would one go directly for the specific sets?

I could probably fix this when I am implementing the Button class!

oblador · 2018-07-17T18:00:52Z

FontAwesome5.js

+}
+
+const iconSet = FA5iconSet(false);
+export default iconSet;


The other icon sets also export Button, TabBarItem, TabBarItemIOS, ToolbarAndroid. Would it be possible to use these specialized components with FA5?

For now it is possible to use through the iconSets in the FontAwesome5 class but I think implementing it should be fairly easy.

oblador · 2018-07-17T18:01:42Z

FontAwesome5Pro.js

+ * Usage: <FontAwesome5Pro name="icon-name" size={20} color="#4F8EF7" />
+ */
+
+import {FA5iconSet} from './FontAwesome5';


Not sure if the linter caught it but, should be spaces around the curly braces like this: `{ FA5iconSet }

No, I didn't get a warning but I'll fix it 👍🏻

oblador · 2018-07-17T18:02:11Z

RNVectorIcons.podspec

@@ -12,7 +12,7 @@ Pod::Spec.new do |s|
  s.platform       = :ios, "7.0"
  s.source         = { :git => "https://github.com/oblador/react-native-vector-icons.git", :tag => "v#{s.version}" }
  s.source_files   = 'RNVectorIconsManager/**/*.{h,m}'
-  s.resources      = "Fonts/*.ttf"
+  s.resources      = "Fonts/*.(o|t)tf"


Is this change needed? Seems like FA5 uses .ttf too?

This is true, my first design was using the desktop version of the font (which is .otf) but I later moved to the npm hosted version and just forgot about this but I can change it.

oblador · 2018-07-17T18:04:38Z

So awesome, thanks a bunch! This is a big PR with some new API considerations, so added some comments relating to this.

oblador · 2018-07-17T19:54:56Z

Btw I’m originally from and on my way to Malmö right now haha.

hampustagerud · 2018-07-17T20:19:28Z

I had to make a few changes to the API in order to be able to select style but they don't break anything so I felt it was worth it. Also, I am mainly a C/C++ (at least native) developer so this whole npm ecosystem is quite new to me and sometimes it feels like it´s messing with me on purpose which is why it may not be the cleanest solutions possible. I´ll look into the things you commented on and try to fix it (and maybe learn something 🙃).

Haha it´s a small world (or at least country)! I feel sorry for you that you had to leave the best part of Sweden but I hope you feel welcome when you return 😉 Work or pleasure?

+ FontAwesome 5.1.1 + Temporary folder and npm pack when building + Separate glyphmaps

hampustagerud · 2018-07-18T11:48:10Z

I have updated the code quite a lot now:

+ iconSets are now generated via a factory
+ iconSets support all features that "classic" iconSets do (Button, TabBarItem etc.) and in the same way they do.
+ getImageSource() works
+ Fixed the IconExplorer project
+ Building uses a temporary folder and npm pack
+ Upgrading occurs locally to the project (not in the library folder) and requires no additional steps after the script is finished. It can be run using just fa5-upgrade from the project folder, much cleaner and easier!
+ Docs update

- Library doesn't have to be manually implemented on iOS, it is done automatically the first time an iconSet is generated

I think that is all the changes, the code looks and works better at least 🙃

oblador · 2018-07-19T20:07:20Z

bin/fa5-upgrade.sh

+echo "Downloading FontAwesome5 Pro"
+
+npm pack @fortawesome/fontawesome-pro
+tar -xzf fortawesome-fontawesome-pro*


AFAIK npm pack will output the file name, so you can use that instead of globing.

oblador · 2018-07-19T20:09:11Z

bin/fa5-upgrade.sh

+echo "Setting up npm config"
+
+npm config set "@fortawesome:registry" https://npm.fontawesome.com/
+npm config set "//npm.fontawesome.com/:_authToken" $fa5_token


I'm assuming that this might already have been done before. Maybe we should check whether the setting already exists and not ask for token in that case.

I'm not sure how to implement this, I can read the first variable and only set it to a new value if it is undefined. However, the second one is secret and I can't read anything from checking npm config get "//npm.fontawesome.com/:_authToken". Is there a way to extract the value? The documentation doesn't seem to mention anything about it or the error thrown ("--- sekretz ---").

oblador · 2018-07-19T20:11:39Z

bin/fa5-upgrade.sh

+
+echo "Modifying package.json"
+
+add-font-assets


This assumes $PATH variable to include ./node_modules/.bin correct? Maybe better to be explicit here.

oblador · 2018-07-19T20:13:38Z

bin/fa5-upgrade.sh

+
+echo "Linking project"
+
+react-native link


This is potentially dangerous, can we be explicit with only linking the current project? Didn't try but was thinking like this:

react-native link `basename "$PWD"`

I ran with react-native link react-native-vector-icons which I intended first but I guess I forgot it later.

oblador · 2018-07-19T20:21:25Z

lib/create-icon-set.js

@@ -43,6 +61,7 @@ export default function createIconSet(glyphMap, fontFamily, fontFile) {
    static defaultProps = {
      size: DEFAULT_ICON_SIZE,
      allowFontScaling: false,
+      extraStyle: {},


Think you forgot this one.

oblador · 2018-07-19T20:22:54Z

package.json

@@ -4,6 +4,8 @@
  "description": "Customizable Icons for React Native with support for NavBar/TabBar/ToolbarAndroid, image source and full styling.",
  "main": "dist/index.js",
  "bin": {
+    "add-font-assets": "./bin/add-font-assets.js",
+    "fa5-upgrade": "./bin/fa5-upgrade.sh",


Does it make sense to prefix this somehow? I know generate-icon doesn't, but that doesn't mean it's correct hehe.

I don't know, looking through some .bin folders it doesn't seem conventional to have some common prefix.

oblador · 2018-07-19T20:24:48Z

FONTAWESOME5.md

+Use this to select which style the generated image should have:
+
+```javascript
+import FontAwesome5 from 'react-native-vector-icons/FontAwesome5';


Should say import FontAwesome5, { FA5Style } ...

oblador · 2018-07-19T20:26:59Z

FONTAWESOME5.md

+You need your FontAwesome npm token which can be obtained by logging into your
+account and then access the ```Services``` tab.
+
+Run ```fa5upgrade``` and enter the token when asked to in order to


npm run fa5-upgrade or ./node_modules/.bin/fa5-upgrade, yeah?

npm run fa5-upgrade results in npm ERR! missing script: fa5-upgrade for me. ./node_modules/.bin/fa5-upgrade works but so does fa5-upgrade?

Maybe that only works with yarn, let’s change to full bin path (current name is wrong now anyway). Not everybody has set up their path to include node_modules.

oblador · 2018-07-19T20:28:30Z

package.json

@@ -4,6 +4,8 @@
  "description": "Customizable Icons for React Native with support for NavBar/TabBar/ToolbarAndroid, image source and full styling.",
  "main": "dist/index.js",
  "bin": {
+    "add-font-assets": "./bin/add-font-assets.js",


Do we need to expose this script to the user? If so maybe document it.

Not really, it is only useful when running fa5-upgrade.

oblador · 2018-07-19T20:29:08Z

fonts.gradle

@@ -5,7 +5,7 @@
 def config = project.hasProperty("vectoricons") ? project.vectoricons : [];

 def iconFontsDir = config.iconFontsDir ?: "../../node_modules/react-native-vector-icons/Fonts";
-def iconFontNames = config.iconFontNames ?: [ "*.ttf" ];
+def iconFontNames = config.iconFontNames ?: [ "*.ttf", "*.otf" ];


Revert this one too to be consistent across platforms.

oblador · 2018-07-19T20:29:51Z

bin/fa5-upgrade.sh

+tar -xzf fortawesome-fontawesome-pro*
+mv package pro
+
+popd


Learned a new command today, thanks!

oblador · 2018-07-19T20:31:59Z

bin/add-font-assets.js

+}
+
+for (let i = 0; i < pack.rnpm.assets.length; i += 1)
+  if (pack.rnpm.assets[i] === './assets/fonts') process.exit();


Feels like includes() or an old school pack.rnpm.assets.indexOf('./assets/fonts') !== -1 would be prettier somehow

Total blackout here, of course that's what it should be!

oblador · 2018-07-19T20:33:13Z

bin/add-font-assets.js

+
+const json = fs.readFileSync('package.json', 'utf8');
+
+const pack = JSON.parse(json);


Not sure if it's better but I would do a require('package.json') to save me some typing.

I tried it at first because it does look a lot cleaner but then node can't seem to find the file, it only works when I use the fs module. Maybe there is something wrong with my env?

Think fs is relative to pwd and require to the file. Try require(path.resolve(’package.json’))

Yes, require is relative to the file... I forgot about that!

oblador · 2018-07-19T20:35:29Z

bin/add-font-assets.js

+
+pack.rnpm.assets.push('./assets/fonts');
+
+fs.writeFileSync('package.json', JSON.stringify(pack, null, 2), 'utf8');


Very small detail, but most prefer files ending with a new line and many editors will add it by just saving. To avoid diffs, could you add one?

I do too, I don't know if it's git or my editor (currently JetBrains) but I have to have two blank lines at the end to make one blank line in the saved file.

It seems like yarn test complains on the blank line though?

I meant in the generated package.json

Ah, got it!

oblador · 2018-07-19T20:38:20Z

So awesome man! PR of the year for sure 😄 Really close now, added a few comments, mostly minor things. Thank you for your patience! 🙇

hampustagerud · 2018-07-19T23:38:17Z

Should be a little closer to the finish line now 😊

oblador · 2018-07-20T07:20:39Z

@hampustagerud I merged this perhaps a bit prematurely. Was checking out the IconExplorer project and most of the icons are just question marks on iOS:

beausmith · 2018-07-20T07:45:00Z

Ha! Random… I'm eagerly watching this PR, and noticed the Sweden references… just flew into CPH yesterday (from SF), and now chilling in Åhus with my in-laws for the next month. ❤️

oblador · 2018-07-20T08:03:11Z

Also @hampustagerud, would you mind sharing your fontawsome token so I can play around with it a bit? DM me on https://twitter.com/trastknast

hampustagerud · 2018-07-20T10:12:17Z

I can send it soon!

The bug in the icon list is that the icons default to regular which does not even contain all free icons. I have changed to code in IconList.js to always set the solid prop to true and then all icon renders properly, also this shouldn't interfere with the other icon sets.

Should I create a PR or just commit it?

oblador · 2018-07-20T10:14:38Z

Please make a PR! 👍

hampustagerud · 2018-07-20T10:18:47Z

Will do that!

Also, realised that the brands icons are not showing since they are a different font but in the same glyphmap... Maybe the best thing would be to have a custom IconList for FA5 which could show all free versions of the icons? Some would probably be question marks still but there should be a some icons in every ListItem (either Brand, Regular or Solid).

oblador · 2018-07-20T10:36:22Z

Hmm, would be cool if we could tell by the glyph which font to use so you didn’t have to do the brand prop. Think I actually saw that somewhere that they have different ranges.

hampustagerud · 2018-07-20T11:04:12Z

That would be interesting for sure, then there could be some automatic fallback as well.

hampustagerud force-pushed the fontawesome5 branch 2 times, most recently from 82dcf01 to 3449bc1 Compare July 17, 2018 14:48

hampustagerud added 6 commits July 17, 2018 17:05

FontAwesome5 implementation

5340877

+ GlyphMap for FontAwesome 5.1.0 Pro + Font files for FontAwesome 5.1.0 Free + Implementation enabling FontAwesome5 icons Handles the copying and renaming of FontAwesome5 Pro font files Removed debug code

Update README.md

8e789bf

Easier way to use FontAwesome5 Pro

13e644a

Now a default import from a separate file, instead of creating it using FA5iconSet(bool).

FontAwesome 5 instructions

c682035

Fixes to pass tests

Automatic build and upgrading, also added to directory and IconExplorer

a698fe1

Buttons and TabBarItems should be working on iOS too now

5d2dae1

hampustagerud force-pushed the fontawesome5 branch from 71a23fb to 5d2dae1 Compare July 17, 2018 15:06

hampustagerud mentioned this pull request Jul 17, 2018

FontAwesome v5 #613

Closed

beausmith mentioned this pull request Jul 17, 2018

Config with correct brands package. Update readme with install steps. shyaniv7/react-native-fontawesome-pro#18

Merged

oblador requested changes Jul 17, 2018

View reviewed changes

hampustagerud added 10 commits July 18, 2018 06:13

Updated build flow

b144027

+ FontAwesome 5.1.1 + Temporary folder and npm pack when building + Separate glyphmaps

Upgrading FontAwesome is automatic and occurs locally

5c1e203

Create FontAwesome 5 iconSet with factory

0f9b2a5

Setup FontAwesome 5 automatically

f1ef199

Bug fixes with FontAwesome 5 loading

8a8a1d1

getImageSource() implementation for FontAwesome 5

569473f

Export all types in FontAwesome5 iconSets

47cece2

Changed name on FA5Type to FA5Style

6fd8697

Update docs to reflect changes to FontAwesome 5

50942b8

Some final iOS fixes for FontAwesome 5 PR

d952755

oblador reviewed Jul 19, 2018

View reviewed changes

hampustagerud added 7 commits July 20, 2018 00:18

Updated scripts

f6d6365

Name changes and minor fixes

6bd1e49

ensureNativeModuleAvailable() in its own file

8c3189e

Removed add-font-assets from user binaries

7eb5d31

Edited package.json ends with blank line now

4d1503a

Cleaner code

068dc30

Docs update

d8763ee

oblador approved these changes Jul 20, 2018

View reviewed changes

oblador merged commit 45e8617 into oblador:master Jul 20, 2018

oblador mentioned this pull request Jul 20, 2018

Update to FontAwesome glyphset #703

Closed


		echo "Installing FontAwesome5"

		npm install @fortawesome/fontawesome-free --no-shrinkwrap


		const json = fs.readFileSync('package.json', 'utf8');

		const pack = JSON.parse(json);


		pack.rnpm.assets.push('./assets/fonts');

		fs.writeFileSync('package.json', JSON.stringify(pack, null, 2), 'utf8');

FontAwesome 5 #776

FontAwesome 5 #776

Conversation

hampustagerud commented Jul 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oblador commented Jul 17, 2018

oblador commented Jul 17, 2018

hampustagerud commented Jul 17, 2018

hampustagerud commented Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oblador Jul 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hampustagerud Jul 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oblador commented Jul 19, 2018

hampustagerud commented Jul 19, 2018

oblador commented Jul 20, 2018

beausmith commented Jul 20, 2018

oblador commented Jul 20, 2018

hampustagerud commented Jul 20, 2018

oblador commented Jul 20, 2018

hampustagerud commented Jul 20, 2018

oblador commented Jul 20, 2018

hampustagerud commented Jul 20, 2018

hampustagerud commented Jul 17, 2018 •

edited

Loading

hampustagerud commented Jul 18, 2018 •

edited

Loading

oblador Jul 19, 2018 •

edited

Loading

hampustagerud Jul 19, 2018 •

edited

Loading