Skip to content

Feat: Make it smoother to switch between nearby and explore maps #6164

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

andy-ife
Copy link
Contributor

This pr makes it easier for users to switch between the nearby and explore screens for quick comparisons e.g checking if a recommended location in nearby already has photos in explore. When switching from nearby to explore, nearby map camera data (zoom and center) is bundled and passed as fragment arguments, which are loaded in the explore map. And vice-versa.

Fixes #6100

Changes made:

  • res/menu: extended nearby_fragment_menu with a 'Show in Explore' option, and added a new menu explore_fragment_menu with a 'Show in Nearby' option.
  • main activity: extended the loadFragment() method to support passing bundles between fragments. This is how the map data is passed between nearby and explore.
  • fixed memory leaks caused by main activity holding references to old fragments.

Tests performed

  • Tested functionality while logged in and out.
  • Tested under unfavourable conditions - no internet, tapping quickly

Tested prodDebug, betaDebug on Tecno Spark 10c with API level 31.

smooth.nearby.explore.mp4

@nicolas-raoul
Copy link
Member

The screencast looks great!

I took the liberty to add a commit because of a conflict, feel free to modify it if you think a better resolution is possible.

I am currently testing.
When switching and panning several times in areas with a lot of items (you can use the "FakeGPS" app to simulate Vienna or Tokyo for instance), I get this crash:

java.lang.RuntimeException: android.os.TransactionTooLargeException: data parcel size 778460 bytes
Bundle stats:
  android:viewHierarchyState [size=2708]
    android:views [size=2604]
  androidx.lifecycle.BundlableSavedStateRegistry.key [size=774884]
    android:support:activity-result [size=52704]
      KEY_COMPONENT_ACTIVITY_REGISTERED_KEYS [size=49828]
      KEY_COMPONENT_ACTIVITY_REGISTERED_RCS [size=2516]
    android:support:fragments [size=721868]
      android:support:fragments [size=721796]
PersistableBundle stats:
  [null]
at android.app.servertransaction.PendingTransactionActions$StopInfo.run(PendingTransactionActions.java:146)
at android.os.Handler.handleCallback(Handler.java:991)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loopOnce(Looper.java:232)
at android.os.Looper.loop(Looper.java:317)
at android.app.ActivityThread.main(ActivityThread.java:8934)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:591)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:911)
Caused by: android.os.TransactionTooLargeException: data parcel size 778460 bytes
at android.os.BinderProxy.transactNative(Native Method)
at android.os.BinderProxy.transact(BinderProxy.java:592)
at android.app.IActivityClientController$Stub$Proxy.activityStopped(IActivityClientController.java:1504)
at android.app.ActivityClient.activityStopped(ActivityClient.java:100)
at android.app.servertransaction.PendingTransactionActions$StopInfo.run(PendingTransactionActions.java:135)
... 8 more

Not sure whether it is exists in the main branch or whether it is due to this branch. Any insight on this?

@andy-ife
Copy link
Contributor Author

Hmm... maybe a problem passing extras between fragments? I'll look into it.

@nicolas-raoul
Copy link
Member

Another crash, this time when coming back to the app after leaving it for a few hours. Unfortunately I am not sure how to reproduce it:

com.bumptech.glide.load.engine.CallbackException: Unexpected exception thrown by non-Glide code
at com.bumptech.glide.load.engine.EngineJob.callCallbackOnResourceReady(EngineJob.java:161)
at com.bumptech.glide.load.engine.EngineJob$CallResourceReady.run(EngineJob.java:428)
at android.os.Handler.handleCallback(Handler.java:991)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loopOnce(Looper.java:232)
at android.os.Looper.loop(Looper.java:317)
at android.app.ActivityThread.main(ActivityThread.java:8934)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:591)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:911)
Caused by: java.lang.IllegalStateException: Fragment ExploreMapFragment{7f60aa2} (417a48e8-1927-4d74-abc2-d1a8115998c1) not attached to a context.
at androidx.fragment.app.Fragment.requireContext(Fragment.java:900)
at androidx.fragment.app.Fragment.getResources(Fragment.java:964)
at fr.free.nrw.commons.explore.map.ExploreMapFragment.addMarkerToMap(ExploreMapFragment.java:706)
at fr.free.nrw.commons.explore.map.ExploreMapFragment.addMarkersToMap(ExploreMapFragment.java:693)
at fr.free.nrw.commons.explore.map.ExploreMapPresenter.onNearbyBaseMarkerThumbsReady(ExploreMapPresenter.java:182)
at fr.free.nrw.commons.explore.map.ExploreMapController$1.onResourceReady(ExploreMapController.java:180)
at fr.free.nrw.commons.explore.map.ExploreMapController$1.onResourceReady(ExploreMapController.java:170)
at com.bumptech.glide.request.SingleRequest.onResourceReady(SingleRequest.java:639)
at com.bumptech.glide.request.SingleRequest.onResourceReady(SingleRequest.java:578)
at com.bumptech.glide.load.engine.EngineJob.callCallbackOnResourceReady(EngineJob.java:159)

@nicolas-raoul
Copy link
Member

Sorry please ignore my previous comment, it might have been a version issue on my side.

@andy-ife
Copy link
Contributor Author

Another crash, this time when coming back to the app after leaving it for a few hours. Unfortunately I am not sure how to reproduce it:

com.bumptech.glide.load.engine.CallbackException: Unexpected exception thrown by non-Glide code
at com.bumptech.glide.load.engine.EngineJob.callCallbackOnResourceReady(EngineJob.java:161)
at com.bumptech.glide.load.engine.EngineJob$CallResourceReady.run(EngineJob.java:428)
at android.os.Handler.handleCallback(Handler.java:991)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loopOnce(Looper.java:232)
at android.os.Looper.loop(Looper.java:317)
at android.app.ActivityThread.main(ActivityThread.java:8934)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:591)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:911)
Caused by: java.lang.IllegalStateException: Fragment ExploreMapFragment{7f60aa2} (417a48e8-1927-4d74-abc2-d1a8115998c1) not attached to a context.
at androidx.fragment.app.Fragment.requireContext(Fragment.java:900)
at androidx.fragment.app.Fragment.getResources(Fragment.java:964)
at fr.free.nrw.commons.explore.map.ExploreMapFragment.addMarkerToMap(ExploreMapFragment.java:706)
at fr.free.nrw.commons.explore.map.ExploreMapFragment.addMarkersToMap(ExploreMapFragment.java:693)
at fr.free.nrw.commons.explore.map.ExploreMapPresenter.onNearbyBaseMarkerThumbsReady(ExploreMapPresenter.java:182)
at fr.free.nrw.commons.explore.map.ExploreMapController$1.onResourceReady(ExploreMapController.java:180)
at fr.free.nrw.commons.explore.map.ExploreMapController$1.onResourceReady(ExploreMapController.java:170)
at com.bumptech.glide.request.SingleRequest.onResourceReady(SingleRequest.java:639)
at com.bumptech.glide.request.SingleRequest.onResourceReady(SingleRequest.java:578)
at com.bumptech.glide.load.engine.EngineJob.callCallbackOnResourceReady(EngineJob.java:159)

No, you were right. All works well for the first few switches, but eventually, the app crashes with this error. I've mostly solved it though. Just testing for now. Concerning the first crash logs you sent (the TransactionTooLargeException), I haven't been able to recreate that specific error, but will look into it with further testing.

@andy-ife
Copy link
Contributor Author

andy-ife commented Jan 29, 2025

So these are the changes I've made:

  • Fixed the crash caused by the second error you sent. I've still been unable to recreate the first though.
  • Refactored MainActivity to pass unit tests
  • Refactored one unit test in explore: testOnCreateOptionsMenu, because now there are two possible option menus that can show in the app bar: search only or search + Show in Nearby

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

I am not seeing changes anymore, thanks!
Just a tiny ask added.

@andy-ife
Copy link
Contributor Author

Changes made 👍

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

Thanks for this great contribution!
It will make many users' lives easier.

@nicolas-raoul nicolas-raoul merged commit 7b29153 into commons-app:main Jan 30, 2025
1 check passed
Sujal-Gupta-SG pushed a commit to Sujal-Gupta-SG/apps-android-commons that referenced this pull request Feb 3, 2025
…mons-app#6164)

* Nearby: Add 'Show in Explore' 3-dots menu item

* MainActivity: Add methods to pass extras between Nearby and Explore

* MainActivity: Extend loadFragment() to support passing fragment arguments

* Nearby: Add ability to navigate to Explore fragment on 'Show in Explore' click

* Explore: Read fragment arguments for Nearby map data and update Explore map if present

* Explore: Add 'Show in Nearby' 3-dots menu item. Only visible when Map tab is selected

* Explore: On 'Show in Nearby' click, navigate to Nearby fragment, passing map data as fragment args

* Nearby: Read fragment arguments for Explore map data and update Nearby map if present

* MainActivity: Fix memory leaks when navigating between bottom nav destinations

* Explore: Fix crashes caused by unattached map fragment

* Refactor code to pass unit tests

* Explore: Format javadocs

---------

Co-authored-by: Nicolas Raoul <[email protected]>
Sujal-Gupta-SG pushed a commit to Sujal-Gupta-SG/apps-android-commons that referenced this pull request Feb 3, 2025
…mons-app#6164)

* Nearby: Add 'Show in Explore' 3-dots menu item

* MainActivity: Add methods to pass extras between Nearby and Explore

* MainActivity: Extend loadFragment() to support passing fragment arguments

* Nearby: Add ability to navigate to Explore fragment on 'Show in Explore' click

* Explore: Read fragment arguments for Nearby map data and update Explore map if present

* Explore: Add 'Show in Nearby' 3-dots menu item. Only visible when Map tab is selected

* Explore: On 'Show in Nearby' click, navigate to Nearby fragment, passing map data as fragment args

* Nearby: Read fragment arguments for Explore map data and update Nearby map if present

* MainActivity: Fix memory leaks when navigating between bottom nav destinations

* Explore: Fix crashes caused by unattached map fragment

* Refactor code to pass unit tests

* Explore: Format javadocs

---------

Co-authored-by: Nicolas Raoul <[email protected]>
nicolas-raoul added a commit that referenced this pull request Feb 7, 2025
* Rename .java to .kt

* Migrated ContributionController

* Rename .java to .kt

* Migrated ContributionDao

* Rename .java to .kt

* Migrated ContributionsContract,ContributionFragment,ContributionListAdapter,ContributionsListContract from java to Kotlin

* Rename .java to .kt

* converted/Migrated

* converted/Migrated

* Rename .java to .kt

* Migrated ContributionController

* Rename .java to .kt

* Migrated ContributionDao

* Rename .java to .kt

* Migrated ContributionsContract,ContributionFragment,ContributionListAdapter,ContributionsListContract from java to Kotlin

* Rename .java to .kt

* Show placeholder and display depiction section when no depictions are available (#6163) (#6165)

* corrected

* corrected

* Update MediaDetailFragment.kt

Spelling correction

* Migrated AboutActivity from Java to Kotlin (#6158)

* Rename Constants to Follow Kotlin Naming Conventions

>This PR refactors constant names in the project to adhere to Kotlin's UPPERCASE_SNAKE_CASE naming convention, improving code readability and maintaining consistency across the codebase.

>Renamed the following constants in LoginActivity:
>saveProgressDialog → SAVE_PROGRESS_DIALOG
>saveErrorMessage → SAVE_ERROR_MESSAGE
>saveUsername → SAVE_USERNAME
>savePassword → SAVE_PASSWORD

>Updated all references to these constants throughout the project.

* Update Project_Default.xml

* Refactor variable names to adhere to naming conventions

Renamed variables to use camel case:
-UPLOAD_COUNT_THRESHOLD → uploadCountThreshold
-REVERT_PERCENTAGE_FOR_MESSAGE → revertPercentageForMessage
-REVERT_SHARED_PREFERENCE → revertSharedPreference
-UPLOAD_SHARED_PREFERENCE → uploadSharedPreference

Renamed variables with uppercase initials to lowercase for alignment with Kotlin conventions:
-Latitude → latitude
-Longitude → longitude
-Accuracy → accuracy

Refactored the following variable names:
-NUMBER_OF_QUESTIONS → numberOfQuestions
-MULTIPLIER_TO_GET_PERCENTAGE → multiplierToGetPercentage

* Refactor Dialog View Initialization with Null-Safe Calls

This PR refactors the dialog setup code in CustomSelectorActivity to improve safety and readability by replacing explicit casts with null-safe generic calls for findViewById.

>Replaced explicit casting (as Button and as TextView) with the generic findViewById<T>() method for improved type safety.
>Added null-safety (?.) to avoid potential crashes if a view is not found in the dialog layout.

why changed:-
>Prevents runtime crashes caused by NullPointerException when a view is missing in the layout.

* Refactor Unit Test: Replace Unsafe Casting with Type-Safe Mocking for findViewById

>PR refactors the unit test for NearbyParentFragment by replacing unsafe casting in the findViewById mocking statements with type-safe

>Ensured all findViewById mocks now use a consistent, type-safe format (findViewById<View>(...)) to reduce verbosity and potential casting errors.

>Verified the functionality of prepareViewsForSheetPosition remains unchanged, ensuring no regression in test behavior.

* Update NearbyParentFragmentUnitTest.kt

* Refactor: Rename Constants to Follow CamelCase Naming Convention

>Updated all constant variable names to follow the camelCase naming convention, removing underscores in the middle or end.

>Ensured variable names remain descriptive and align with code readability best practices.

* Replace private val with const val for URL constants in QuizController

* Renaming the constant to use UPPER_SNAKE_CASE

* Renaming the constant to use UPPER_SNAKE_CASE

* Update Done

* **Refactor: Convert `minimumThresholdForSwipe` to a compile-time constant**

* Convert AboutActivity from Java to Kotlin

This PR converts the AboutActivity class from Java to Kotlin

>Testing:
>Verified all functionalities of the AboutActivity, including toolbar setup, intent launches, and dialog interactions, to ensure behavior remains consistent post-conversion.
>Successfully ran unit tests for AboutActivity to confirm the correctness of methods and logic.

* Thank you for the suggestion! Since these methods all take a single View parameter, replacing them with method references is a great way to simplify the code and improve readability. I'll updated the code accordingly. Added a TODO in the code as a reminder to refactor this in the future.

---------

Co-authored-by: Nicolas Raoul <[email protected]>

* Localisation updates from https://translatewiki.net.

* Feat: Make it smoother to switch between nearby and explore maps (#6164)

* Nearby: Add 'Show in Explore' 3-dots menu item

* MainActivity: Add methods to pass extras between Nearby and Explore

* MainActivity: Extend loadFragment() to support passing fragment arguments

* Nearby: Add ability to navigate to Explore fragment on 'Show in Explore' click

* Explore: Read fragment arguments for Nearby map data and update Explore map if present

* Explore: Add 'Show in Nearby' 3-dots menu item. Only visible when Map tab is selected

* Explore: On 'Show in Nearby' click, navigate to Nearby fragment, passing map data as fragment args

* Nearby: Read fragment arguments for Explore map data and update Nearby map if present

* MainActivity: Fix memory leaks when navigating between bottom nav destinations

* Explore: Fix crashes caused by unattached map fragment

* Refactor code to pass unit tests

* Explore: Format javadocs

---------

Co-authored-by: Nicolas Raoul <[email protected]>

* Localisation updates from https://translatewiki.net.

* enhance spammy category filter (#6167)

Signed-off-by: parneet-guraya <[email protected]>

* Localisation updates from https://translatewiki.net.

* correction

* correction

* correction

* GitHub workflow to build betaDebug (#6174)

* [Bug fix] Check if duplicate exist using both original and modified file's checksum (#6169)

* check original file's SHA too along with modified one

Signed-off-by: parneet-guraya <[email protected]>

* fix tests

Signed-off-by: parneet-guraya <[email protected]>

---------

Signed-off-by: parneet-guraya <[email protected]>

* Add multiline input for caption and description (#6173)

* allow multiple lines for description/caption

* make caption multiline too

---------

Co-authored-by: Nicolas Raoul <[email protected]>

* correction

---------

Signed-off-by: parneet-guraya <[email protected]>
Co-authored-by: Akshay Komar <[email protected]>
Co-authored-by: Nicolas Raoul <[email protected]>
Co-authored-by: translatewiki.net <[email protected]>
Co-authored-by: Ifeoluwa Andrew Omole <[email protected]>
Co-authored-by: Parneet Singh <[email protected]>
Co-authored-by: Matija Nalis <[email protected]>
nicolas-raoul added a commit that referenced this pull request Feb 26, 2025
* Rename .java to .kt

* Migrated ContributionController

* Rename .java to .kt

* Migrated ContributionDao

* Rename .java to .kt

* Migrated ContributionsContract,ContributionFragment,ContributionListAdapter,ContributionsListContract from java to Kotlin

* Rename .java to .kt

* converted/Migrated

* converted/Migrated

* Rename .java to .kt

* Migrated ContributionController

* Rename .java to .kt

* Migrated ContributionDao

* Rename .java to .kt

* Migrated ContributionsContract,ContributionFragment,ContributionListAdapter,ContributionsListContract from java to Kotlin

* Rename .java to .kt

* Show placeholder and display depiction section when no depictions are available (#6163) (#6165)

* corrected

* corrected

* Update MediaDetailFragment.kt

Spelling correction

* Migrated AboutActivity from Java to Kotlin (#6158)

* Rename Constants to Follow Kotlin Naming Conventions

>This PR refactors constant names in the project to adhere to Kotlin's UPPERCASE_SNAKE_CASE naming convention, improving code readability and maintaining consistency across the codebase.

>Renamed the following constants in LoginActivity:
>saveProgressDialog → SAVE_PROGRESS_DIALOG
>saveErrorMessage → SAVE_ERROR_MESSAGE
>saveUsername → SAVE_USERNAME
>savePassword → SAVE_PASSWORD

>Updated all references to these constants throughout the project.

* Update Project_Default.xml

* Refactor variable names to adhere to naming conventions

Renamed variables to use camel case:
-UPLOAD_COUNT_THRESHOLD → uploadCountThreshold
-REVERT_PERCENTAGE_FOR_MESSAGE → revertPercentageForMessage
-REVERT_SHARED_PREFERENCE → revertSharedPreference
-UPLOAD_SHARED_PREFERENCE → uploadSharedPreference

Renamed variables with uppercase initials to lowercase for alignment with Kotlin conventions:
-Latitude → latitude
-Longitude → longitude
-Accuracy → accuracy

Refactored the following variable names:
-NUMBER_OF_QUESTIONS → numberOfQuestions
-MULTIPLIER_TO_GET_PERCENTAGE → multiplierToGetPercentage

* Refactor Dialog View Initialization with Null-Safe Calls

This PR refactors the dialog setup code in CustomSelectorActivity to improve safety and readability by replacing explicit casts with null-safe generic calls for findViewById.

>Replaced explicit casting (as Button and as TextView) with the generic findViewById<T>() method for improved type safety.
>Added null-safety (?.) to avoid potential crashes if a view is not found in the dialog layout.

why changed:-
>Prevents runtime crashes caused by NullPointerException when a view is missing in the layout.

* Refactor Unit Test: Replace Unsafe Casting with Type-Safe Mocking for findViewById

>PR refactors the unit test for NearbyParentFragment by replacing unsafe casting in the findViewById mocking statements with type-safe

>Ensured all findViewById mocks now use a consistent, type-safe format (findViewById<View>(...)) to reduce verbosity and potential casting errors.

>Verified the functionality of prepareViewsForSheetPosition remains unchanged, ensuring no regression in test behavior.

* Update NearbyParentFragmentUnitTest.kt

* Refactor: Rename Constants to Follow CamelCase Naming Convention

>Updated all constant variable names to follow the camelCase naming convention, removing underscores in the middle or end.

>Ensured variable names remain descriptive and align with code readability best practices.

* Replace private val with const val for URL constants in QuizController

* Renaming the constant to use UPPER_SNAKE_CASE

* Renaming the constant to use UPPER_SNAKE_CASE

* Update Done

* **Refactor: Convert `minimumThresholdForSwipe` to a compile-time constant**

* Convert AboutActivity from Java to Kotlin

This PR converts the AboutActivity class from Java to Kotlin

>Testing:
>Verified all functionalities of the AboutActivity, including toolbar setup, intent launches, and dialog interactions, to ensure behavior remains consistent post-conversion.
>Successfully ran unit tests for AboutActivity to confirm the correctness of methods and logic.

* Thank you for the suggestion! Since these methods all take a single View parameter, replacing them with method references is a great way to simplify the code and improve readability. I'll updated the code accordingly. Added a TODO in the code as a reminder to refactor this in the future.

---------

Co-authored-by: Nicolas Raoul <[email protected]>

* Localisation updates from https://translatewiki.net.

* Feat: Make it smoother to switch between nearby and explore maps (#6164)

* Nearby: Add 'Show in Explore' 3-dots menu item

* MainActivity: Add methods to pass extras between Nearby and Explore

* MainActivity: Extend loadFragment() to support passing fragment arguments

* Nearby: Add ability to navigate to Explore fragment on 'Show in Explore' click

* Explore: Read fragment arguments for Nearby map data and update Explore map if present

* Explore: Add 'Show in Nearby' 3-dots menu item. Only visible when Map tab is selected

* Explore: On 'Show in Nearby' click, navigate to Nearby fragment, passing map data as fragment args

* Nearby: Read fragment arguments for Explore map data and update Nearby map if present

* MainActivity: Fix memory leaks when navigating between bottom nav destinations

* Explore: Fix crashes caused by unattached map fragment

* Refactor code to pass unit tests

* Explore: Format javadocs

---------

Co-authored-by: Nicolas Raoul <[email protected]>

* Localisation updates from https://translatewiki.net.

* enhance spammy category filter (#6167)

Signed-off-by: parneet-guraya <[email protected]>

* Localisation updates from https://translatewiki.net.

* correction

* correction

* correction

* GitHub workflow to build betaDebug (#6174)

* [Bug fix] Check if duplicate exist using both original and modified file's checksum (#6169)

* check original file's SHA too along with modified one

Signed-off-by: parneet-guraya <[email protected]>

* fix tests

Signed-off-by: parneet-guraya <[email protected]>

---------

Signed-off-by: parneet-guraya <[email protected]>

* Add multiline input for caption and description (#6173)

* allow multiple lines for description/caption

* make caption multiline too

---------

Co-authored-by: Nicolas Raoul <[email protected]>

* correction

---------

Signed-off-by: parneet-guraya <[email protected]>
Co-authored-by: Akshay Komar <[email protected]>
Co-authored-by: Nicolas Raoul <[email protected]>
Co-authored-by: translatewiki.net <[email protected]>
Co-authored-by: Ifeoluwa Andrew Omole <[email protected]>
Co-authored-by: Parneet Singh <[email protected]>
Co-authored-by: Matija Nalis <[email protected]>
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.

Make it smoother to switch between Nearby and Explore maps
2 participants