refactor(fonts): Remove useless fonts in android (#485) #512
Conversation
|
@dyesseyumba Thanks for your PR. Don't worry. I think I have found a way to solve the back problem. I will tidy my codes and send a PR tonight. |
There was a problem hiding this comment.
Hi @dyesseyumba and thank you for tackling this one.
As you said it: running react-native link will embed all TTFs anyways.
Therefore, we need to change our yarn run link script defined in package.json (line 17) to not simply run react-native link, but to name the libraries to be linked instead.
Here are the libraries that needs linking as of now:
- react-native-code-push
- react-native-config
- react-native-cookies
- react-native-device-info
- react-native-i18n
- react-native-photo-view
- react-native-safari-view
- react-native-search-bar
So I guess the new link script will look like this:
"link": "react-native link react-native-code-push && react-native link react-native-config && ....."
Once this is done, calling yarn run link won't try to link react-native-vector-icons anymore, and everything should work as intended (as long as someone doesn't directly use react-native link of course)
Could you change our link script and see how it goes? (this should also fix the same issue for #475)
Thanks a million!
| // Name of the font files you want to copy | ||
| iconFontNames: [ | ||
| 'FontAwesome.ttf', | ||
| 'MaterialIcons.ttf', |
There was a problem hiding this comment.
I don't think we're using MaterialIcons
|
@chinesedfan look like our comments collided :D |
…point#485) BREAKING CHANGE: Update link script in Package.json
|
I applied the changes. In scripts, the "link" line has more than 300 characters but it works. |
|
@dyesseyumba Seems like @chinesedfan found a way better idea than mine, check #513 |
chinesedfan
left a comment
chinesedfan
left a comment
There was a problem hiding this comment.
@dyesseyumba Thanks for your work! Please hold for a while until we reach an agreement.
package.json
Outdated
| "clean": "rm -rf node_modules", | ||
| "clean:android": "cd android && ./gradlew clean && cd -", | ||
| "link": "react-native link", | ||
| "link": "react-native link react-native-code-push && react-native link react-native-config && react-native link react-native-cookies && react-native link react-native-device-info && react-native link react-native-i18n && react-native link react-native-photo-view && react-native link react-native-safari-view && react-native link react-native-search-bar", |
| // Name of the font files you want to copy | ||
| iconFontNames: [ | ||
| 'FontAwesome.ttf', | ||
| 'Octicons.ttf'] |
|
@dyesseyumba #513 has been merged. So you can just revert the second commit. Would you mind to resolve those conflict in contributors files? Then I think we can merge this one and close #402. |
…point#485) BREAKING CHANGE: Update link script in Package.json
…oid (gitpoint#485)" This reverts commit 282f475.
Merge remote-tracking branch 'origin/refactor_fonts' into refactor_fonts
|
@chinesedfan All conflicts has been resolved. |
|
@dyesseyumba Good. But |
|
I'm sorry @dyesseyumba, but I don't know what went wrong with your PR and the way you merged commits. |
|
Yes @machour, is the only needed commit. To resolve the last conflict, I rebased the master branch in my local forked branch. |
|
Thank you so much @dyesseyumba, that made the PR readable again. |
|
Works perfectly, after building Android we get only the 3 mentioned TTF bundled: 🍺 ~/git-point/android (master)*$ find . -name '*.ttf'
./app/build/intermediates/assets/debug/fonts/FontAwesome.ttf
./app/build/intermediates/assets/debug/fonts/MaterialIcons.ttf
./app/build/intermediates/assets/debug/fonts/Menlo.ttf
./app/build/intermediates/assets/debug/fonts/Nunito-Bold.ttf
./app/build/intermediates/assets/debug/fonts/Nunito-Italic.ttf
./app/build/intermediates/assets/debug/fonts/Nunito-Light.ttf
./app/build/intermediates/assets/debug/fonts/Nunito-Regular.ttf
./app/build/intermediates/assets/debug/fonts/Nunito-SemiBold.ttf
./app/build/intermediates/assets/debug/fonts/Octicons.ttf
./app/src/main/assets/fonts/Menlo.ttf
./app/src/main/assets/fonts/Nunito-Bold.ttf
./app/src/main/assets/fonts/Nunito-Italic.ttf
./app/src/main/assets/fonts/Nunito-Light.ttf
./app/src/main/assets/fonts/Nunito-Regular.ttf
./app/src/main/assets/fonts/Nunito-SemiBold.ttf🎉 |
3 participants
According to the documentation of react-native-vector-icons docs only FontAwesome, MaterialIcons and Octicons will be installed and used.
But if we launch the command
react-native link, all react-native-vector-icons fonts will be installed even in ios. I think it is caused by the rnpm configuration in react-native-vector-icons library.