search

e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 34 forks source link

Simplify the code paths in list.js related to mode and purpose confirmation #334

Closed shankari closed 5 years ago

shankari commented 5 years ago

The mode and purpose confirmation code is currently pretty convoluted, because @sunil07t wrote it assuming that all data would be local, so a lot of processing is handled in the then block. We now pull the data from both the cache and the server as part of the timeline load https://github.com/e-mission/e-mission-phone/pull/528 so we can use $scope variables and dramatically simplify the code.

@atton16 was reluctant to change the existing code as part of https://github.com/e-mission/e-mission-phone/pull/528 because it was so convoluted. But convoluted code that people are afraid to touch is a Bad Thing.

So we open this new issue to try and untangle what the code is intended to do, what it actually does and remove getLocalModeForTrip and getLocalPurposeForTrip for once and for all.

sunil07t commented 5 years ago

I worked on mode and purpose feature about a year and a half ago so I will need to review but if I can help in any way please let me know.

shankari commented 5 years ago

@sunil07t recall that your original mode and purpose feature only worked with local data. We now have a way to retrieve it in a unified way from both the local and remote datastores (using the UnifiedDataLoader), but that means that we really need to retrieve it at one time at the beginning because we don't want to make a bunch of remote calls.

@atton16 has already implemented this in https://github.com/e-mission/e-mission-phone/pull/528 But there's still a bunch of getLocalMode and getLocalPurpose in the code. We should remove them and unify the code. So if you can give us an outline of what you were trying to do and how best to use the $scope variable instead, that would be great.

shankari commented 5 years ago

@asiripanich now that you have had a chance to play around with these changes for a bit, is it true that you can only set the mode and purpose once? It would be good to get a sense of what exactly needs to be changed to make this be user-friendly so that we can use that as a motivator for the cleanup.

asiripanich commented 5 years ago

@shankari, the mode and purpose of a trip are always modifiable by the user and get sent to remote. I think changing the logic of when UnifiedDataLoader needs to retrieve data from remote can significantly make user experience more fluid.

shankari commented 5 years ago

@asiripanich but are they modifiable multiple times? e.g. if the user confirms the mode to be tram but then wants to change it to metro, is that possible? from the code, it looked like not, but I haven't actually tried testing it.

So UnifiedDataLoader should only load data from remote on the first load of data for a day. @atton16 already implemented that in https://github.com/e-mission/e-mission-phone/pull/528

My argument here is that we don't actually even need to read from the local database. When the user sets a mode and purpose, we can:

As long as the user is on the same page, the data is in memory. When the user goes to a new page, the existing UnifiedDataLoader call will read the data from the local database if it has not been pushed, and from the server if it has been pushed.

Performant and easy to understand :)

shankari commented 5 years ago

@PatGendre, during our talk today, you said that you had some comments on the UX from the perspective of an end-user. Can you please add them here or open a new issue? I think it will be super useful to @asiripanich and @atton16 as they try to build an intuitive UX that can be used without a lot of explanation.

PatGendre commented 5 years ago

Hi Shankari, yes, here are some of the comments of 2 people testing the app recently :

PatGendre commented 5 years ago
shankari commented 5 years ago

@PatGendre thank you so much for the feedback! Some comments inline.

the app mixes two types of usage : tracking/sharing and "selfcare". The goal of the app should be clearer. The DIARY screen serves the first usage, whereas the DASHBOARD serves the second usage.

I think that @asiripanich and @atton16 are currently only interested in the tracking aspects.

For the sharing usage, the app could also include actual trip sharing and comparison,

True, although you would want to be careful wrt privacy. would the trip be shared forever? Would there be a time limit?...

whereas for the tracking app the DASHBOARD should be more like a leaderboard with the total indicators (trips number, etc.) but maybe not the calories/co2 emissions.

The general answer for a whole bunch of the suggestions below is "sure, go ahead". The whole point of building a platform is that I don't have to decide what works best for the UI. leaderboard (e.g. competition) certainly works for some people but turns others off. Each deployer of the platform can decide what they feel is the most incentivizing for their particular area, target population etc. The app has one implementation of the UI to give you a sense of what is possible, but really you can and should customize it however you want.

For the record, an undergrad group at Berkeley did a study on comparing emotion (e.g. polar bear w/leaderboard) versus information (e.g. suggestions for change) and found that emotion was more engaging but less effective in actually changing behavior (https://www2.eecs.berkeley.edu/Pubs/TechRpts/2019/EECS-2019-2.html). The UIs from their study are still available in the urap-2017-emotion and urap-2017-information branches, although the server is no longer running since the study is done.

in the DIARY, it would be more convenient to hide the date selector and enable the user to scroll infinitely in all the history, because it is frustrating to ask to select a date where there is no trace (or the date selector should only display the dates with actual data)

This is the one thing that is hard to do with the current architecture because the processed data is designed to be read on a day by day basis. I've been thinking for a while about changing that, specially since the unprocessed data is ready using a time range anyway, but it has not been high enough priority for me to change.

the answer to everything below this point is - the UX is configurable, feel free to change if you think it is better

why put the menu (diary/dashboard/profile...) at the bottom? Usually it is at the top?

It is at the bottom in iOS. Since our UI is crossplatform, we had to pick one, and we picked iOS-style. Feel free to move it to the top in your version of the app if you like.

As for UI design, would it be possible to use Google's material design?

The UI is completely customizable by the deployers. If you want to use material design, feel free to do so, and maybe contribute the redesigned UI to a branch so that others can build from it too!

when displaying the detail of trip, the map should be larger (full screen)

Again, there are different possible opinions on this. @atton16 @asiripanich what has your user feedback on this been?

another thing maybe for the DIARY : when the pipeline hasn't yet been run on the data, the map appears as grey and the trip is tagged "draft" but I am not sure it will clear for the user ; maybe the map should not be displayed in that case, and instead of draft it could be written sthg like "the data will be processed in the next 24 hours" (if the pipeline is run by a daily cron); maybe the raw data map could be displayed only if the user clicks on the trip for more details. It can be inspiring to look at the https://www.modalyzer.com/ app to see how they do it, for instance.

Not sure why we should not display the map, we do have the location points, they are just not segmented. Is the user suggesting that raw data map be hidden only for draft trips or for all trips? If for all trips, the moves UX had a straight line along the left with big blobs for the places and small blobs for the stops. You could definitely try to replicate that if you wanted. @atton16 @asiripanich what have your pilot users told you about this?

PatGendre commented 5 years ago

You're welcome, thanks for your insightful reply. Most of the remarks come from an active OpenStreetMap mapper, so his views may be a little biased towards a community of sharing sharing lots of data, and interested in indicators such as in a leaderboard. Also, we tested only on Android, therefore the bias towards material design. The full screen map request has been made by 3 users so may be worthwhile (depends may be on age and good view ;-) If all goes well, we shall have more user feedback in 2 or 3 April when we test again and we will decide then if we go for rewriting part of the UI.

shankari commented 5 years ago

Simplified the code paths in https://github.com/e-mission/e-mission-docs/issues/353

shankari commented 5 years ago

Closing this for now, although we should probably move the fit-and-finish issues reported here into another bug.

shankari commented 5 years ago

@st-patrick here's another issue with many UI fit-and-finish issues reported by @PatGendre If you want to tackle any subset of them, please pull them out into a separate issue so that we can get consensus on what to implement.

I am not the best person to be making UI design decisions, hopefully the wisdom of the crowd will help here