phlask / phlask-map

Code behind the Phlask Web Map
https://beta.phlask.me
34 stars 36 forks source link

Panning after opening modal pans map to last location #527

Closed tomporvaz closed 1 week ago

tomporvaz commented 2 months ago

Describe the bug In desktop mode (and presumably mobile too) the map will pan to the last location visited specifically after opening the modal and not closing it.

To Reproduce Steps to reproduce the behavior:

  1. Choose a location (e.g. Franklin Square), click the pin opening the details modal.
  2. Without closing the modal, manually pan the map to another location (e.g. Franklin D. Roosevelt Park).
  3. Click the new location (e.g. Franklin D. Roosevelt Park).
  4. See error, the map has panned back to the first location (e.g. Franklin Square).

Expected behavior I expect the map to pan to the new location, e.g. Franklin D. Roosevelt Park.

tomporvaz commented 2 months ago

Also, when modal is opened and then closed, and then near me button is clicked, no pan. Also, if you open a modal, and then click near me, and then exit modal, the map will pan back to last location.

ravicodelabs commented 2 months ago

Hey. Just started by trying to reproduce the stated issues. There are a few notable points. Here are the findings:

Note also that at the moment, I believe current location would be the default current location (around city hall I believe) when location services are off.

ravicodelabs commented 1 month ago

Hi. I'm leaving some of my testing notes here, which organizes the various cases, in case someone else finds it helpful in debugging.

Issue and Environment | Location On | Location Off | Issue Details -- | -- | -- | -- Issue A Desktop | Issue not occurring in Arc browser. | Issue not occurring for me in Arc, Chrome, or Safari. | Open a location modal, pan to somewhere else (keeping the mentioned modal open), click a new location pin (so that a new modal opens), and the map will incorrectly pan back to the first location. Issue A Mobile | Not applicable to mobile - can't pan the map while a modal is open. | Not applicable to mobile - can't pan the map while a modal is open. | Open a location modal, pan to somewhere else (keeping the mentioned modal open), click a new location pin (so that a new modal opens), and the map will incorrectly pan back to the first location. Issue B Desktop | Reproduced issue in Desktop. | Reproduced issue in Desktop. | Near Me not panning to current location after opening/closing a modal. Issue B Mobile | Reproduced issue in Mobile. | Reproduced issue in Mobile. | Near Me not panning to current location after opening/closing a modal. Issue C Desktop | Issue also present when location turned on. Also, current location is not showing actual current location, and is instead using the default "La Colombe" default location (e.g. the one used when location is turned off), which also may not be correct. | Issue seems present, but might be more nuanced. In particular, when location access is not given to the browser, it seems to go back to the last location as previously stated. However, when location access is given to the browser, it is going back to the current location I believe. | If you open a modal, and then click near me, and then exit the modal, the map will pan back to last location Issue C Mobile | Not applicable - the Near Me button is hidden when modal is open, I think. | Not applicable - the Near Me button is hidden when modal is open, I think. | If you open a modal, and then click near me, and then exit the modal, the map will pan back to last location
ravicodelabs commented 1 month ago

Proposed fix for the issues here is provided in PR #539.

gcardonag commented 1 month ago

These are awesome notes, thanks for putting them together @ravicodelabs ! I'm looking through the scenarios and PR 👀

As for the note about the default location being city hall when users do not share location, that is accurate. We set that in order to have a starting position in that scenario (since we otherwise don't have a place to set as the initial map center)

ravicodelabs commented 1 month ago

Sounds good @gcardonag, and thanks for clarifying the default center was an intentional bit, makes sense!

gcardonag commented 1 week ago

Closing this as it has been addressed, thanks again @ravicodelabs !