-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix #5182 Switch From Mapbox to MapLibre #5184
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
Conversation
2a0b823
to
cf86367
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks great!
I will try to test tomorrow.
Meanwhile any idea why the unit tests testOnMapReady and testShowDetail fail? |
I am getting this crash:
|
Deleting the app's data solved the crash. |
I can consistently crash by switching from dark mode to light mode in the app's settings menu and coming back to the map. Can you reproduce? |
@nicolas-raoul I have identified the root cause for issue "Could not find layer Outdoor". Pushing a new patch with these changes |
cf86367
to
588c256
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ayan-10 I'm able to reproduce the issue. Still trying to figure out what could be the root cause since I'm using Mapbox's predefined style for "Dark" just like "Outdoors" and "Streets". MapLibre offers us the option to use Mapbox Configuration so I continued with it to maintain consistency. I checked the URL being called for the Dark Style and it is "mapbox://styles/mapbox/light-v10" which is consistent with that being called in the case of mapbox-sdk. |
Great, no more crash in Contributions now. However, in addition to the visual issues reported by Ayan, I observe a crash in Explore: tap any picture, then tap the pen icon at the right of the latitude/longitude:
If possible, would you mind using normal push instead of force-push, unless really necessary? Thanks! :-) |
@nicolas-raoul Could you please check if the issue still persists? Meanwhile, I'm trying to fix the dark mode issue. |
The crash is not occurring for me anymore. |
No crash anymore indeed, thanks! |
@Ayan-10 I Just pushed some changes for the "About" and "Settings" screens on dark mode. Please let me know how to get to the 3rd screen with achievements and leaderboard. I can apply a similar fix and test. |
To see achievements, you must be logged-in, tap the app's hamburger menu, then tap the cup icon with your usename at the top of the menu. I will test this branch from now, thanks! |
UI that appears bright in dark mode:
|
I guess you have found out that the app has two flavors, beta and prod, matching https://commons.wikimedia.beta.wmflabs.org and https://commons.wikimedia.org. |
|
a6f4ad6
to
bdb2355
Compare
Thanks! I created a different issue for (2) and (5). For (6) you can use the "Fake GPS" app or similar to put yourself in a place that has many Wikidata items, for instance right on the Eiffel Tower. The banner should then appear within a minute. |
…tFeature() issue
…ode issue on two screens
…ode issue on additional screens
…ode issue on notification screen
6a28088
to
874f05a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as intended, in both light and dark mode.
Code looks great, maybe some Strings could be made into constants, but no blocker.
Thanks a lot for implementing this very important change!
* feedback dialog: fix black font in dark mode * LocationPickerActivity: fix light map in dark mode * Fix #5182 Switch From Mapbox to MapLibre (#5184) * Fix #5182 Switch From Mapbox to MapLibre * Fix #5182 Switch From Mapbox to MapLibre - Resolved requestFeature() issue * Fix #5182 Switch From Mapbox to MapLibre - Resolved dark mode issue on two screens * Fix #5182 Switch From Mapbox to MapLibre - Resolved dark mode issue on additional screens * Fix #5182 Switch From Mapbox to MapLibre - Resolved dark mode issue on notification screen * Fix #5182 Switch From Mapbox to MapLibre - Test errors * fix issue #5015 - custom image selector not identifying photo location (#5190) Co-authored-by: Siva <doodsiva@gmail.com> * feedback dialog: fix black font in dark mode * LocationPickerActivity: fix light map in dark mode * LocationPicker: use predefined style based on device theme * LocationPickerActivityTest: add additional target exception in catch block * LocationPickerConstants: remove extra newline introduced --------- Co-authored-by: Kartikay Kaushik <93285364+kartikaykaushik14@users.noreply.github.com> Co-authored-by: Siva Subramaniam <112970189+siva-subramaniam-v@users.noreply.github.com> Co-authored-by: Siva <doodsiva@gmail.com>
Description (required)
Fixes #5182
What changes did you make and why?
Migrated from Mapbox to MapLibre
Tests performed (required)
Tested {build variant, e.g. ProdDebug} on {name of device or emulator} with API level {API level}.
Screenshots (for UI changes only)
Need help? See https://support.google.com/android/answer/9075928
Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.