This repository has been archived by the owner on Apr 28, 2018. It is now read-only.
After moving with the app loaded, places are sorted by absolute location rather than travel times #627
Labels
Milestone
Comments
Current state:
I think there are three options:
I have the code written for 3, but I'm concerned about unexpected edge cases and it'll take some more time to test (which takes a while, given I have to walk around). After speaking with Maria, she mentioned I should focus on #558, then focus on this bug. As such, I'll land the bug fix and then maybe get to testing this. |
mcomella
added a commit
to mcomella/prox
that referenced
this issue
Mar 11, 2017
…erver. This ensures these linked properties update together. This also fixes a bug where we didn't update the mapping when we updated the displayedPlaces while re-sorted the displayedPlaces list.
mcomella
added a commit
to mcomella/prox
that referenced
this issue
Mar 11, 2017
mcomella
added a commit
to mcomella/prox
that referenced
this issue
Mar 11, 2017
mcomella
added a commit
that referenced
this issue
Mar 11, 2017
mcomella
added a commit
to mcomella/prox
that referenced
this issue
Mar 11, 2017
mcomella
added a commit
to mcomella/prox
that referenced
this issue
Mar 11, 2017
We previously sorted by distance from location, for an unknown reason. The old implementation had a bug: it never updated the placeKeyToDisplayedPlacesIndexMap but updated the displayedPlaces list so the existing behavior is unknown. NB: I didn't test this yet.
I landed the intermediate fix (untested IRL) and am working on the big fix in https://github.com/mcomella/prox/tree/i627-sort (note: I think it only needs to be tested IRL). |
I'm not going to land this now – alternatively, we should consider deliberately designing this experience rather than landing hacky fixes. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
I think this is a mistake from a UX perspective.
Maybe be hard b/c of async nature of travel times.
The text was updated successfully, but these errors were encountered: