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

Resolves #5445 highlighting nearest place on clicking home nearby banner #5453

Merged
merged 5 commits into from
Jan 19, 2024

Conversation

ShashwatKedia
Copy link
Contributor

Description (required)

Fixes #5445

What changes did you make and why?
Added method to change marker icon if place is nearest, when user clicks on home nearby banner and also highlight the bottom sheet details of that particular place. Also fixed the home nearby banner to keep displaying even after clicks on it and comes back to it later (which wasn't happening earlier)

Tests performed (required)

Tested prodDebug on OnePlus Nord CE 2 Lite with API level 31

Screenshots (for UI changes only)

Video showing functionality of app in dark and light mode:
https://drive.google.com/file/d/1ZmxEvQ05mT82A5uz1D4eEI2DmtSRpWCf/view?usp=share_link

@ShashwatKedia
Copy link
Contributor Author

@nicolas-raoul I added functionality for highlighting nearest place only when user clicks on banner, as I felt that, highlighting it always would make the home banner click redundant and would defeat it's purpose. If you feel otherwise, then I can modify it.

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.

Unit test failing:

fr.free.nrw.commons.contributions.ContributionsFragmentUnitTests > testOnResumeCaseNullCaseIf FAILED

@ShashwatKedia
Copy link
Contributor Author

@nicolas-raoul my apologies, I missed checking NPE while calling function in onResume

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.

Works great, thanks!
I changed the indentation a bit, see my commits above.
Merging now.

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.

Highliting nearest location on nearby map, when user clicks on home banner
2 participants