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

[Bug]: Attempt to read from null error in Explore -> Map fragment #5457

Closed
kanahia1 opened this issue Jan 19, 2024 · 21 comments · Fixed by #5541
Closed

[Bug]: Attempt to read from null error in Explore -> Map fragment #5457

kanahia1 opened this issue Jan 19, 2024 · 21 comments · Fixed by #5541
Assignees
Labels

Comments

@kanahia1
Copy link
Contributor

Summary

This issue is caused when there are no elements in mediaList Maybe the issue caused when there are no media (images) available at specific coordinate.

Steps to reproduce

  1. Open Explore -> Map Fragment in App
  2. Go to Bihta, Patna, India or Coordinates = 25.5303740, 84.8597150 in map

Expected behaviour

Place's images should appear on the map.

Actual behaviour

Toast message Error fetching nearby places. Attempt to read from null...

Device name

Samsung S21

Android version

Android 14

Commons app version

4.2.1-debug-HEAD~9be8df3

Device logs

java.lang.NullPointerException: Attempt to read from null array at fr.free.nrw.commons.explore.map.ExploreMapPresenter.getLatLngBounds(ExploreMapPresenter.java:223) at fr.free.nrw.commons.explore.map.ExploreMapPresenter.updateMapMarkers(ExploreMapPresenter.java:193) at fr.free.nrw.commons.explore.map.ExploreMapFragment.updateMapMarkers(ExploreMapFragment.java:433) at fr.free.nrw.commons.explore.map.ExploreMapFragment.lambda$populatePlaces$5$fr-free-nrw-commons-explore-map-ExploreMapFragment(ExploreMapFragment.java:413)

Screen-shots

WhatsApp.Video.2024-01-19.at.12.32.38_ef292d87.mp4

Would you like to work on the issue?

Yes

@ShashwatKedia
Copy link
Contributor

@kanahia1 this might happen because you are using the betaDebug variant, as was the case here #5255 (comment), pointed out by @nicolas-raoul here: #5255 (comment). Does the issue persist in prodDebug variant as well?

@kanahia1 kanahia1 changed the title [Bug]: Attempt to real from null error in Explore -> Map fragment [Bug]: Attempt to read from null error in Explore -> Map fragment Jan 19, 2024
@kanahia1
Copy link
Contributor Author

Thanks @ShashwatKedia it worked! on production server. But we will need to handle it since there can be a place on earth where are no images in that case there will be this error. So, we can show some dialog at that place to make user experience better.

@nicolas-raoul
Copy link
Member

Indeed, I just tried with prodDebug in Siberia and get the same error:
Screenshot_20240120-071808.png

@rohit9625
Copy link
Contributor

Using dialogues can be interrupting for users. Instead, we should use Snackbars with proper error messages. Concisely, this is what I mean :
WhatsApp Image 2024-01-21 at 17 55 05_0bcf6fa8

Although, we can move from Toasts to Snackbars. What do you say?
@nicolas-raoul @kanahia1

@nicolas-raoul
Copy link
Member

Yes snackbar is good.

I think we should try to convey the real problem to the user, though.

Here, the real problem is that there are no Wikidata items in that area, right?

@rohit9625
Copy link
Contributor

No problem, I am adding a new string in the resource. This will be displayed to the user.
Can you provide a well-formatted message to be displayed to the user?

@nicolas-raoul
Copy link
Member

How about No Wikidata items in this area? :-)

@rohit9625
Copy link
Contributor

rohit9625 commented Jan 21, 2024

Well, this seems to be a good response we can provide. I am placing it in the resources.
Although, I have a question, i.e., I created a method to display the snackbar in the same file where showToasts() method is defined.
Do I need to create any UI test for snackbar?

@rohit9625
Copy link
Contributor

After making changes, these are the results:

Light Mode
WhatsApp Image 2024-01-21 at 19 49 21_32bd796a

Dark Mode
WhatsApp Image 2024-01-21 at 19 49 21_8a749ca6

However, there was a resource already string like this:
<string name="no_nearby_places_around">No nearby places around</string>
But the text provided by you seems much better.

@nicolas-raoul
Copy link
Member

Ah, no_nearby_places_around is great, thanks for finding it! 🙂

@nicolas-raoul
Copy link
Member

Ah wait, I was totally forgetting the context since yesterday, I am so sorry! 😱

Please use "No pictures in this area".

Thanks! 🙂

@rohit9625
Copy link
Contributor

Done 👍
WhatsApp Image 2024-01-22 at 20 49 19_c86f5d85

Please express your thoughts about the colors and action button for the **Snackbars** because I am considering replacing all the toasts with a beautiful **Snackbar**.
What do you think?

@kanahia1
Copy link
Contributor Author

Hey @rohit9625,
I think we cannot replace all the Toasts with Snackbars since Toasts usually are used to display text related with system or some background services since Toasts are shown even if Activity is finished. So replacing all the Toasts with SnackBars can cause disturbance in User Experience.


References
https://m2.material.io/components/snackbars#usage
https://stackoverflow.com/questions/34432339/android-snackbar-vs-toast-usage-and-difference

@rohit9625
Copy link
Contributor

Oh, my bad!😅
Yeah, you are right Mr. @kanahia1. We can't replace all the toasts but some toasts should be replaced I guess.

@nicolas-raoul
Copy link
Member

@rohit9625 Your screenshot looks great! 🙂

@kanahia1
Copy link
Contributor Author

Hey @rohit9625, I have added TODO for snackbar in #5475, you can add code for snackbar and share a pull request. 🙌

@rohit9625
Copy link
Contributor

Okay I'll do it :)

@rohit9625
Copy link
Contributor

rohit9625 commented Jan 28, 2024

Showing response message will work after solving this issue #5487
Have a look @kanahia1

@kanahia1
Copy link
Contributor Author

Sure @rohit9625 I will look into #5487

@rohit9625
Copy link
Contributor

Done Mr. @kanahia1 👍

@rohit9625
Copy link
Contributor

Hello @nicolas-raoul
Can you please look at the PR that solves this issue?
I don't know why the previous PR closed. I am creating a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment