-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Android: Fixes #13854: Fix missing icons where react-native-paper IconButton is used #13869
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
base: dev
Are you sure you want to change the base?
Conversation
|
Thank you for fixing this! For future reference:
Be aware that the camera, voice recorder, and biometric unlock functionality all use |
Thank you for this info! I've now tested all 3 of these functions using a real device and they all work without issues. FYI I used an older device on Android 10, but presumably this would not make any difference. Also, given that #12782 which was a problem in Joplin 3.4 is also likely related to the expo default config, and it was causing issues for my local release builds on Windows, I think it is still worth excluding it for the Android release builds, even if this particular issue could be fixed by some library upgrades. EDIT: I tested the WIP react-native-vector-icons upgraded to v12 changes (rebased onto my branch) on the GitHub pipeline and the upgrade does indeed fix the icon issue. So if the issue affects iOS as well, then it will be addressed by this upgrade. See https://github.com/mrjo118/joplin/releases/tag/1765203601 |
This is also an issue with Joplin 13.5.1 on iOS. |
Seems like the react-native-vector-icons upgrade ought to be done in time for the Joplin 3.5 stable release then, as the missing icons do affect usability if you're not familiar with the Joplin ui, as well as looking unprofessional. I haven't checked all the places where icons are missing, but another place I spotted is on the profiles screen, the add profile button is missing the plus icon on the button. |
In the initial release of the Joplin 3.5 mobile app, there is a regression whereby all icons are missing when implemented via the react-native-paper IconButton component. This is only an issue in the release build for Android, hence why it was not noticed until now, where the first 3.5 release was recently made for the mobile app.
The issue relates to the inclusion of getExpoDefaultConfig(__dirname) in the metro configuration (originally added in React Native 0.77 upgrade in PR #12179). I made various attempts suggested by AI and suggestions online, to override potentially problematic sections of the default expo config, however none of the suggested solutions were able to resolve the issue. However, what is clear from my testing is that the release build of Android works fine without any obvious issues when completely excluding getExpoDefaultConfig(__dirname), but it is required for the dev build to work. Therefore this PR resolves the issue by excluding this config, but only when building a release build and for the Android app specifically.
The issue is very likely related to issues various users have reported on this GitHub issue expo/expo#21568. On that report, some users have reported the missing icons issue only happening on Android, although a couple of users mentioned it happening on iOS as well. I have not tested whether or not this issue occurs on iOS release builds as well, but if it is also an issue on iOS, this change could potentially be extended to cover iOS as well in a followup PR.
This fixes #13854
Testing
Screenshots:
Before:
After: