Open mcomella opened 7 years ago
Briefly looked into this.
Looking at the current code, I think we only fetch places when the app returns to the foreground (PlaceCarouselViewController.willEnterForeground
). I assume we just completely refresh the UI to index 0 in this case.
If we wanted to update the places as we walk, the PlaceCarouselViewController has the LocationMonitorDelegate
which should get the location update calls (we may need to reconfigure the radius deltas for which this method is called). It calls updatePlaces
, for which shouldFetchPlaces
disables fetching a new set of places so we'll need to decide when we should set it to false. Then we need to determine how to update the UI.
@antlam FYI that I don't think we update the UI at all as we walk, unless the app is closed and reopened. See below.
However, it looks like we may resort the places every 100m as we walk. If I understand this correctly, the currently displayed card and the cards on either side will not be updated when we re-sort, but any subsequent swipes will use the newly ordering.
That being said, the behavior is complicated and would not be intuitive for users. One example: The cards are resorted while the user is looking at a card. They swipe left, left, and swipe right, right to get back to the original card. However, this may not be the original card because this card is of the new ordering.
I can explain further if necessary (though it'll take more work so I'm leaving this explanation as is for now).
(FYI @antlam)
Implementation details:
The LocationManager receives significant location updates ever 100m (as set by Firebase) at which point didUpdateLocations -> PCViewController.updatePlaces -> sortPlaces (if !shouldFetchPlaces
). I think the UI is updated in the following fashion: the current, previous, and next VC stay the same. However, every time we get a new VC (i.e. swipe L or R), the newly retrieved VC is using the new sorting, based on plus or minus 1 of the index of the card we swiped to in the new ordering.
Also note that anything that happens with place updating does not affect the map view, which makes a copy of the places list before it displays the places.
NB: Strangely enough, when we re-sort, we do so by absolute distance from location, not travel times. This seems undesired – @antlam, what do you think?
I don't think we update the UI at all as we walk, unless the app is closed and reopened.
This is actually the behaviour we set out for in Kona right? I don't think this is actually a huge problem right now. But I'd definitely be up for revisiting/ prioritizing this work after Chicago if we learn otherwise.
They swipe left, left, and swipe right, right to get back to the original card. However, this may not be the original card because this card is of the new ordering.
I agree. But I don't think I can come up with a simple, not-hairy solution in time.
That being said, I don't think we've seen users think of our "stack" of Places Cards as something that's very "linear". I think the important thing here is to drive home the message that "these are some places around you" - which we could have more success doing via other methods such as onboarding/first run, hints, etc.
Though I would like to revisit this for after Chicago because of the complexity here. It would take UX some time but we could create a flow chart to document all these scenarios again for everyone. We should also re-prioritize this afterwards with Product if it's a huge problem.
when we re-sort, we do so by absolute distance from location, not travel times. This seems undesired – @antlam, what do you think?
I agree. We should always default to sorting by travel times :)
@thebnich Is going to get @mariapopova0729 a build to verify the expected behavior, after walking:
I verified the behavior of Prox local build while walking around: Whether the user backgrounds the app or has the app on screen while walking around, the cards are refreshed almost instantaneously based on the new location. If the cards DO not get refreshed (which I observed several times), then scrolling through one or two cards results in refreshing the set. It seems like a timing issue and I couldn't reliably expect that the cards will be refreshed without actions on my part.
or has the app on screen while walking around, the cards are refreshed almost instantaneously
This sounds different from what we were describing earlier today. Are you saying that if you're actively using the app and looking through cards, the app refresh will lose the card you're on and start you from the beginning?
That is correct, the behavior I saw on device while walking around was that cards do get updated and I lose the card I was on.
OK, I assume we'll want to fix that since that would be a pretty jarring testing experience.
If we fix this, we may also want to fix:
NB: Strangely enough, when we re-sort, we do so by absolute distance from location, not travel times.
To summarize the behavior: If the app is not killed and the user hasn't interacted with it (swiping to another card, switch to map view), then the user should see the same card (and it shouldn't change). However, if new cards are available, the next card the user swipes to should be "new" and relevant data.
In the future, we could also add some kind of "get newest content" button (or otherwise indicate to the user that new cards are available with some action to refresh).
I fixed a bug in the existing behavior in #639.
Verify with antlam it is okay. If not, fix it!