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

Fix reselect poi #127

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix reselect poi #127

wants to merge 4 commits into from

Conversation

joxad
Copy link

@joxad joxad commented May 27, 2024

No description provided.

joxad added 3 commits May 27, 2024 16:09
fix(map): Test fix (when scrolling down the bottom sheet, it was impossible to re select the boulder selected previously)
@wang-li
Copy link
Collaborator

wang-li commented May 27, 2024

@joxad Can you add a short description of the problem in the PR description, as well as a two screen recordings demonstrating the issue and the fixed behavior please? 🙏

@joxad
Copy link
Author

joxad commented May 28, 2024

Version Prod with the bug https://photos.app.goo.gl/tcxDyKGD5XB64yY77

@joxad
Copy link
Author

joxad commented May 28, 2024

Version dev with the fix

https://photos.app.goo.gl/bfzf6cFfyY9rD5TeA

@JocelynDavid
Copy link

Bug is :

When you select a poi, swipe down the bottom sheet, and then reselect the poi, the bottom sheet doesn't expand properly.
It works if you drag the map first, but not reselect the poi.

@@ -118,6 +118,7 @@ class MapFragment : Fragment(), BoolderMapListener {
when (newState) {
STATE_EXPANDED -> mapViewModel.onProblemTopoVisibilityChanged(isVisible = true)
STATE_HIDDEN -> {
mapViewModel.onTopoUnselected()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like a call to mapView.unselectProblem() is missing, as the boulder problem stays selected when dismissing the topo bottom sheet from a back press or dragging it down (the unselectProblem() function's scope can be changed from private to public)

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.

3 participants