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

fix(android): refactor font copy / support android gradle plugin 4.1+ #1307

Merged
merged 1 commit into from
Feb 23, 2021

Conversation

mikehardy
Copy link
Contributor

Hi there! Thanks so much for this module, @oblador, I've been using it quite a while.

I noticed in #1265 that Android Gradle Plugin 4.1 broke the font copy gradle task, similar to the way react-native bundle copy had to be altered to support that version

I had time to look at their bundle copy and updated the font copy here to more closely match their script's copy task: https://github.com/facebook/react-native/blob/c76070412fccdaa82bf4af3c94809d04ae71777e/react.gradle#L282-L317

Previously I was not able to see my react-native-vector-icons in the APK in release mode with android gradle plugin 4.1 but with these changes I can now see them. It should be backwards compatible as well and my build uses flavors and ABI splits so those appear fine to me in testing as well.

Changes:

  • react-native JS bundle doesn't copy per-release target
  • react-native JS bundle adds the copy task as dependent very carefully now!
  • I added some metadata for the task, a "group" and "description"
  • Fixed formatting, 4-space indents was predominant, I went with that everywhere

this is based heavily off the react-native JS bundle copy task
this was developed in collaboration with @gulshan183

Fixes #1265
Obsoletes #1271

I am currently using this as a patch
react-native-vector-icons+8.0.0.patch.txt (remove the .txt extension and drop it in patches directory) with patch-package in case anyone else wants to try it out or needs it now

@mikehardy
Copy link
Contributor Author

@gulshan183 - in particular since you and I both tried this once already - I'd love it if you could take a look at this. Seems to work for me but there is nothing like a good review. Thanks!

@mikehardy
Copy link
Contributor Author

@oblador / @gulshan183 any thoughts on this one :-) ? I'm usually not one to ping but this PR is in production for us now as a patch and is working fine. Would love to see it upstream-ed or if you have anything to change I'd be happy to re-shape it to fix. Thanks!

fonts.gradle Outdated Show resolved Hide resolved
fonts.gradle Outdated Show resolved Hide resolved
- react-native JS bundle doesn't copy per-release target
- react-native JS bundle adds the copy task as dependent very carefully now!
- I added some metadata for the task, a "group" and "description"
- Fixed formatting, 4-space indents was predominant, I went with that everywhere

this is based heavily off the react-native JS bundle copy task
this was developed in collaboration with @gulshan183

Fixes oblador#1265
Copy link
Owner

@oblador oblador left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mikehardy
Copy link
Contributor Author

Fantastic! @oblador please @ me if anything related pops up in a new issue or whatever, I'll do my best to repair if I inadvertently broke something but will likely not notice unless directly pinged.

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

Successfully merging this pull request may close these issues.

Fonts not copied correctly with Android Gradle Plugin 4.1.0
2 participants