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

Testing the time-use of API 32 version of the app #871

Closed
MaliheTabasi opened this issue Mar 28, 2023 · 39 comments
Closed

Testing the time-use of API 32 version of the app #871

MaliheTabasi opened this issue Mar 28, 2023 · 39 comments

Comments

@MaliheTabasi
Copy link

Hi @shankari
We have installed the app and tested the time-use feature. Please find our comment in this issue.

@MaliheTabasi
Copy link
Author

On the permission page, when the user clicks "Fix" to disable battery optimization, only the apps for which battery optimization is turned off are displayed. If the user searches for Openpath and it doesn't appear, they must change the filter to "All apps" before finding it. This process can be confusing for the user initially.

@MaliheTabasi
Copy link
Author

The "Scan program OPcode" button is not functioning on our device, which has resulted in our inability to scan the QR code. As a result, we had to manually enter the token.

@shankari
Copy link
Contributor

shankari commented Mar 28, 2023

wrt

This process can be confusing for the user initially.

Unfortunately, this is what the Android API supports.
https://developer.android.com/reference/android/os/PowerManager#isIgnoringBatteryOptimizations(java.lang.String)

Return whether the given application package name is on the device's power allowlist. Apps can be placed on the allowlist through the settings UI invoked by Settings.ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS.

Being on the power allowlist means that the system will not apply most power saving features to the app. Guardrails for extreme cases may still be applied.

And that is the intent I am launching
e-mission/e-mission-data-collection@60f6fa4#diff-b40b7828b05e6e07122b28154d8ca626e11b81be43d55aab9a353974988684c4R431

I do have text in the permission screen telling users what to do.

For more details, please see the this comment and the ones just below:
#838 (comment)

@shankari
Copy link
Contributor

shankari commented Mar 28, 2023

The "Scan program OPcode" button is not functioning on our device, which has resulted in our inability to scan the QR code. As a result, we had to manually enter the token.

Which version of the app? Which version of the OS (android/iOS)? "Scan program OPcode" works on my test Pixel 4.
What does "is not functioning" mean? There is no camera? The scanned QR code is invalid?

@MaliheTabasi
Copy link
Author

We intended to test the Places feature, but to our surprise, the activities we attempted to add did not appear at all. As a result, we are unable to test this feature while it does not work.

@MaliheTabasi
Copy link
Author

The "Scan program OPcode" button is not functioning on our device, which has resulted in our inability to scan the QR code. As a result, we had to manually enter the token.

Which version of the app? Which version of the OS (android/iOS)? "Scan program OPcode" works on my test Pixel 4. What does "is not functioning" mean? There is no camera? The scanned QR code is invalid?

In the title of the issue I mentioned API32, so it is for Android. The version of app is 1.1.7.
The button does not work at all.

@MaliheTabasi
Copy link
Author

When adding activities, it should be verified that the end time is not earlier than the start time. Currently, with the app, it is possible to add an activity that starts at 9 am and ends at 8 am on the same day, which is incorrect.

@MaliheTabasi
Copy link
Author

Please add a start time, end time, and date to each Place card, similar to what we have on Trip cards. This is necessary because each Place card needs to be associated with a specific date, as we only have one date for each activity.

As can be seen in the attached screenshot, we have a trip on March 17th and the next trip is on March 20th, we need separate Place cards for days in between. In this case we need four place cards. The first Place card should have a start time of 7:16 PM (end time of the trip), an end time of 12:00 AM, and a date of March 17th. The second Place card should have a start time of 12:01 AM, an end time of 12:00 AM, and a date of March 18th. The third Place card should have a start time of 12:01 AM, an end time of 12:00 AM, and a date of March 19th. Finally, the fourth Place card should have a start time of 12:00 AM, an end time of 10:31 AM (start time of the trip), and a date of March 20th.

Without this, it may be confusing to add activities to a single Place card spanning multiple days.

msg6201276735-663232

@MaliheTabasi
Copy link
Author

It it better to have 'Add Activity" and "add trip details" buttons to be styled in the same way.

@MaliheTabasi
Copy link
Author

It would be helpful to include a Place card before the first tracked trip that starts at 12:00 AM on the same day. This will allow users to enter activities they have done before that trip on that day. As of now, there doesn't seem to be any Place card above the first trip, as can be seen in the attached screenshot.
msg6201276735-663233

@MaliheTabasi
Copy link
Author

It appears that the comments I wrote in #840 (comment) have been addressed in the iOS app, but not in the Android app we are using.

@shankari
Copy link
Contributor

shankari commented Mar 28, 2023

@MaliheTabasi android 1.1.7 is not the most recent version. The most recent version is 1.2.0.

Screenshot 2023-03-27 at 9 22 22 PM

Please check the email titled "Upgrade to API 32; new link" sent on March 16, upgrade to 1.2.0 and let me know if you still see these issues.

@MaliheTabasi
Copy link
Author

MaliheTabasi commented Mar 28, 2023

@MaliheTabasi android 1.1.7 is not the most recent version. The most recent version is 1.2.0.

Screenshot 2023-03-27 at 9 22 22 PM

Please check the email titled "Upgrade to API 32; new link" sent on March 16, upgrade to 1.2.0 and let me know if you still see these issues.

@shankari I have used the link you shared in the email with the title "Upgrade to API 32; new link" and it redirected me to install version 1.1.7 of the app. Please recheck the link you sent on March 16.
Aside from that, only comments #871 (comment) and #871 (comment) and #871 (comment) may be related to the version of the app. Other comments are still valid, Because they are common between iOS and Android apps.

@shankari
Copy link
Contributor

shankari commented Mar 28, 2023

@MaliheTabasi you are right; I said in the email

I have been able to upload this to the Google Play console internal testing channel, but will fix some more bugs before I send out the link to the channel.

I sent out the link to the Google Play internal testing channel to the entire technical coordination list asking interested parties to send their gmail ID, similar to the Apple ID on TestFlight. This email was sent on 20th March. I also brought it up at today's meeting.

My apologies if that wasn't clear 😄 and I hope we can get you set up on the internal testing channel soon so we can test against the correct version.

@MaliheTabasi
Copy link
Author

have been able to upload this to the Google Play console internal testing

Thanks for the clarification. I will send you the gmails that need to have access to the latest version of the app.

@shankari
Copy link
Contributor

@MaliheTabasi just wanted to highlight that the label screen is not loaded day by day. instead, the data is loaded week by week. So in general, you cannot assume that the first trip is the label screen is the first trip for the user. The user can try to load additional trips by clicking the download button on the top right (please see video below).
The original first trip was from Mar 14th. After downloading more, the new first trip is from Mar 8th.

scrolling_more_places.mov

When all trips have been downloaded, the download button turns into a check mark.
We can definitely consider adding an extra place to the top when we get that point.

Screenshot 2023-03-28 at 4 45 00 PM

@MaliheTabasi
Copy link
Author

@MaliheTabasi just wanted to highlight that the label screen is not loaded day by day. instead, the data is loaded week by week. So in general, you cannot assume that the first trip is the label screen is the first trip for the user. The user can try to load additional trips by clicking the download button on the top right (please see video below). The original first trip was from Mar 14th. After downloading more, the new first trip is from Mar 8th.

When all trips have been downloaded, the download button turns into a check mark. We can definitely consider adding an extra place to the top when we get that point.

Screenshot 2023-03-28 at 4 45 00 PM

@shankari Thank you for the explanation. If the trips are displayed on a weekly basis, it would be helpful to provide some indication to the user on how to view all their trips and not just those for the current week. It may not be clear that by tapping the download button, the user can access all their trips.

Additionally, there is no issue with showing a place at the top of each week's trip list. A place card can be displayed at the top of each week's trip, starting at 12 am on the same day and ending at the start time of the first trip of the week.

@shankari
Copy link
Contributor

shankari commented Mar 29, 2023

@MaliheTabasi do you have a concrete suggestion for "it would be helpful to provide some indication to the user on how to view all their trips and not just those for the current week"?

We currently display the time range for the trips at the top and we have a button with a very clear download symbol next to it. We don't have a lot of screen space, and we don't have the ability to display hover text since this is a phone app.

Additionally, there is no issue with showing a place at the top of each week's trip list. A place card can be displayed at the top of each week's trip, starting at 12 am on the same day and ending at the start time of the first trip of the week.

Again, this screen is not a diary (either daily or weekly). It is a timeline, similar to most social media apps. You might need to have long-term data to see the full effect. I don't think that adding a fake trip in some arbitrary point of the timeline is a great idea and is not consistent with the high level user interface design.

@MaliheTabasi
Copy link
Author

MaliheTabasi commented Mar 29, 2023

@shankari Yes, I do. You can improve the UI and more clearly show that time range to the user. currently, it is like some unimportant text. Instead of a grey download symbol which I believe is not very clear, you can use a button with a label like "display all weeks" which is more user-friendly. Currently, the app shows 8/8 trips while 8 is the total trips within this week. this could be misleading because I as a user might think 8 is the total number of my trips. It is better to display this like 8/20 where 8 is the total number of trips in this week and 21 is the total number of trips and that "display all" can show all the trips.

I did not suggest adding a fake trip. We need a Place card which starts from 12AM before the first trip of the week. If you prefer you can first create it between that first trip of the week and the last trip of the previous week and then simply break it into two places. One place that is before 12AM can be shown in the previous week as the last component and another place that can be shown in the current week and starts at 12AM. This approach is totally reasonable such that between any two trips there is a place component. We did not ask you to add a place component in an arbitrary point in timeline. we wanted to have a place component between each consecutive pair of trips. All we are asking you to do is simply break that place into multiple places and everything about this place cards including their start time, end time, date, and location are clear.

I also would like to add that the whole logic behind time-use feature is to be able to have a diary either daily or weekly. A timeline can be a diary, as the trips are sorted based on their date and time.

@shankari
Copy link
Contributor

shankari commented Mar 29, 2023

@MaliheTabasi I think we should move this to an in-person discussion since I think we are arguing at cross purposes here.

  • There is no room in the UI for a button like "Display all weeks". If you can think of an icon equivalent, ideally from https://ionic.io/ionicons/v1, I am happy to change the icon to match. Again, this is not a desktop UI and we don't have a lot of room in the limited screen space.
  • there is a place component between each consecutive pair of trips now. Except for the first trip in the timeline, and I offered to add a "start place" before that first trip in the timeline.

I also would like to add that the whole logic behind time-use feature is to be able to have a diary either daily or weekly. A timeline can be a diary, as the trips are sorted based on their date and time.

Correct. But a diary implies "flipping" pages from one day to another with a clearly defined start of day and end of day. A timeline is a smooth progression of time without any "flipping". That's why "current day" or "current week" does not make sense in the context of a timeline.

@shankari
Copy link
Contributor

shankari commented Mar 29, 2023

@MaliheTabasi did you also want to make a new survey for at least the time use? Right now, I think we just have a placeholder survey.

Once you create it, @AlirezaRa94 can submit a PR to add it to https://github.com/e-mission/nrel-openpath-deploy-configs and change the URL and the labelVars at https://github.com/e-mission/nrel-openpath-deploy-configs/blob/main/configs/stage-program.nrel-op.json#L72

@AlirezaRa94, can you add the new survey xml or json to a survey-cfg directory in that repo? If you add xml, we can convert to json for now. I should really set up a process for that in the long-term.

@shankari
Copy link
Contributor

shankari commented Mar 29, 2023

@MaliheTabasi I thought about #871 (comment) some more and I am not sure that splitting the place is the best option, at least for the common case where the dwell time in the place is < 24 hours long.

Note that the common case for people is that they sleep overnight. If you split the place into two, they will have to enter the time use twice (slept from 10pm -> midnight; slept from midnight -> 7am). Since we have a continuous timeline view versus a diary, the user can actually enter a single time use for the entire span, which is clearly more intuitive.

I am open to splitting if the place spans 24 hours, but I don't think we should split at midnight for the reason above.

Again:

  • we have a timeline view
  • the timeline view is better that the diary view, with its artificial divisions at midnight, since the common case for time use is to not follow day boundaries

@MaliheTabasi
Copy link
Author

MaliheTabasi commented Mar 29, 2023

@MaliheTabasi I think we should move this to an in-person discussion since I think we are arguing at cross purposes here.

  • There is no room in the UI for a button like "Display all weeks". If you can think of an icon equivalent, ideally from https://ionic.io/ionicons/v1, I am happy to change the icon to match. Again, this is not a desktop UI and we don't have a lot of room in the limited screen space.
  • there is a place component between each consecutive pair of trips now. Except for the first trip in the timeline, and I offered to add a "start place" before that first trip in the timeline.

I also would like to add that the whole logic behind time-use feature is to be able to have a diary either daily or weekly. A timeline can be a diary, as the trips are sorted based on their date and time.

Correct. But a diary implies "flipping" pages from one day to another with a clearly defined start of day and end of day. A timeline is a smooth progression of time without any "flipping". That's why "current day" or "current week" does not make sense in the context of a timeline.

@shankari I've just given you my comment as a person who is testing the app. I left the decision-making regarding the icon or text in the button to your UI team as I am not an expert.

@MaliheTabasi
Copy link
Author

MaliheTabasi commented Mar 29, 2023

@MaliheTabasi did you also want to make a new survey for at least the time use? Right now, I think we just have a placeholder survey.

Once you create it, @AlirezaRa94 can submit a PR to add it to https://github.com/e-mission/nrel-openpath-deploy-configs and change the URL and the labelVars at https://github.com/e-mission/nrel-openpath-deploy-configs/blob/main/configs/stage-program.nrel-op.json#L72

@AlirezaRa94, can you add the new survey xml or json to a survey-cfg directory in that repo? If you add xml, we can convert to json for now. I should really set up a process for that in the long-term.

@shankari Yes, that is correct, let me talk to Taha and we will share the main survey with you after his approval.

@MaliheTabasi
Copy link
Author

MaliheTabasi commented Mar 30, 2023

@MaliheTabasi I thought about #871 (comment) some more and I am not sure that splitting the place is the best option, at least for the common case where the dwell time in the place is < 24 hours long.

Note that the common case for people is that they sleep overnight. If you split the place into two, they will have to enter the time use twice (slept from 10pm -> midnight; slept from midnight -> 7am). Since we have a continuous timeline view versus a diary, the user can actually enter a single time use for the entire span, which is clearly more intuitive.

I am open to splitting if the place spans 24 hours, but I don't think we should split at midnight for the reason above.

Again:

  • we have a timeline view
  • the timeline view is better that the diary view, with its artificial divisions at midnight, since the common case for time use is to not follow day boundaries

@shankari I understand your point. However if we display only one Place between two consecutive trips in two days, that Place card point to two different days, so how can we decide what date to display on it? If a person sleeps from 10PM on 15th March to 6AM on 16th of March, then the end time of the activity is earlier than its start time if we do not consider change in date, which does not make sense. That's why we need to take account of the change in date by splitting Place card into two. If we can come up with another solution to this problem then splitting Place only if it is longer than 24 should be enough.

@JGreenlee
Copy link

If we expect overnight activities to be a common need, the most proper solution is to explicitly ask for the end date in the survey.
Currently, we only have "Date", "Start Time", and "End Time", and the times are assumed to be from the same day.

@JGreenlee
Copy link

It is clearest to the user when 1 place corresponds to 1 blue card, rather than multiple consecutive place cards.

If we really do need to split up multi-day places, I would suggest that we divide the card up into sections, separated by a line. However, I am not sure if the project timeline allows for this anymore.

@MaliheTabasi
Copy link
Author

If we expect overnight activities to be a common need, the most proper solution is to explicitly ask for the end date in the survey. Currently, we only have "Date", "Start Time", and "End Time", and the times are assumed to be from the same day.

Thank you for sharing your thoughts. I completely agree that the proposed solution seems feasible. It's essential to ensure that the user enters a valid end date, taking into consideration that it should not be after their next trip. Moreover, some modifications to the user interface will be necessary to implement this feature.

@MaliheTabasi
Copy link
Author

It is clearest to the user when 1 place corresponds to 1 blue card, rather than multiple consecutive place cards.

If we really do need to split up multi-day places, I would suggest that we divide the card up into sections, separated by a line. However, I am not sure if the project timeline allows for this anymore.

As I had mentioned to Shankari, we are open to exploring alternative solutions to address the issue of a single place card being linked to multiple days when only one date is available for inserting activities. We understand that splitting places may not be the only solution and are willing to consider other approaches if they prove to be more effective.

@shankari
Copy link
Contributor

@JGreenlee @sebastianbarry it looks like the real request wrt "first place" is that the timeline start with a place instead of a trip (if we are displaying places).

@JGreenlee I know that the composite trip currently returns only the trip and the succeeding place. It seems like it should be fairly trivial to have it return both the preceding and the succeeding place.

We can then maybe change the display timeline to start with the preceding place of the first composite trip. It's going to be a fair amount of work, and will involve end-to-end changes, though.

@sebastianbarry
Copy link

@JGreenlee I know that the composite trip currently returns only the trip and the succeeding place. It seems like it should be fairly trivial to have it return both the preceding and the succeeding place.

I actually do like this solution; it would go something like this I gather:

  • composite_trip objects stored on the server now store both the data for nextPlace as well as previousPlace
    • this would not provide much of a performance hit, as in our testing we found that the slowest part of loading composite_trip objects was the number of locaiton data points in the trip, where in this case we would only be adding one location data point inside of the previousPlace
    • I also think that having this data point be redundant (in that: this place will be both the previousPlace for one composite_trip object and the nextPlace for the following composite_trip object) is ideal in implementation (explain later in WHY:), but the alternative would be to have the previousPlace only store the enter_ts and exit_ts and act as an ID reference to the previous composite_trip's nextPlace object (containing the location data point, etc. data). WHY: because when we load the displayTrips for example on the all-trips screen, the device may not immediately load all of the trips, so the previous trip to the top-most trip on the timeline would not have the data we need in order to display things like display_name as it won't have the location data point.
  • The Timeline.readAllCompositeTrips funciton would remain relatively unchanged as the previous place would be available to us already in the first composite_trip object
  • The $scope.readDataFromServer .then function Timeline.readAllCompositeTrips(currEnd, ONE_WEEK).then((ctList) => {... would populate the previous and next place objects for every single composite_trip, but would look for duplicates and not insert duplicate place objects inside of the allTrips list. Otherwise, there would be 2 place cards in-between every trip card
  • This solution also leads itself to the situation that @MaliheTabasi mentioned in this comment: Testing the time-use of API 32 version of the app #871 (comment)

That's why we need to take account of the change in date by splitting Place card into two.

  • If we see a duplicate place object but they fall on different days, we can just display both
    • If we display both, we may need to adjust the way that place-cards display so that they are touching, possibly like this:

Screen Shot 2023-03-30 at 2 50 17 PM

I do think however @JGreenlee is right:

However, I am not sure if the project timeline allows for this anymore.

@shankari
Copy link
Contributor

shankari commented Mar 30, 2023

@sebastianbarry the first three of your points are perfectly correct. However, the last point won't really work the way that you/Maliheh want.

If we see a duplicate place object but they fall on different days, we can just display both
If we display both, we may need to adjust the way that place-cards display so that they are touching, possibly like this:

The duplicate place object that we get from the server, will really be a duplicate; i.e. identical. There is no question of duplicate trip objects that "fall on different days"; they would not then be duplicates, by default.

Consider the following timeline: place (school) -> trip (school -> home) -> place (home, overnight) -> trip (home -> school) -> place (school)

There is only one place between the two trips, not one place per day. That is why it is a duplicate in the composite trip list

This will get split into the following composite trips (after the server change to return both places)

  • place (school) -> trip (school -> home) -> place (home, overnight, id=p2)
  • place (home, overnight, id=p2) -> trip (home -> school) -> place (school)

@JGreenlee
Copy link

JGreenlee commented Mar 30, 2023

The redundancy of including both start and end place would not hurt us, and it would not be hard to ignore the duplicates when composite trips are received and unpacked on the phone.

The only difficult and time-consuming task here would be splitting up places, but I think we can avoid this.

If surveys include an "End date", time use can be recorded overnight. This way, we do not need to split the places.
Is that correct @MaliheTabasi?

@MaliheTabasi
Copy link
Author

The redundancy of including both start and end place would not hurt us, and it would not be hard to ignore the duplicates when composite trips are received and unpacked on the phone.

The only difficult and time-consuming task here would be splitting up places, but I think we can avoid this.

If surveys include an "End date", time use can be recorded overnight. This way, we do not need to split the places. Is that correct @MaliheTabasi?

@JGreenlee Yes, that is correct, however you might need to take care of the the UI changes that are necessary and some validation processes between the start data and time and end date and time for activity insertion. For example, you might want to show both start date and start time of the Place and the end time and end date of the Place on Place card. or you might want to update UI such that each activity aside form title, start time, end time, and delete icon, has a start date and end date as well when it is displayed to users so that they can realise to which date it belongs.
Regarding the weekly display, the whole last Place card of the previous week can be displayed as the first Place of the current week if we want to avoid splitting. Please see the image attached. Given that 18th of March is the last day of previous week, this place card can be displayed both at the end of previous week and at the start of current week to avoid splitting. However it only appears once if we are displaying all trips to the user (not the weekly view).
Place

@shankari
Copy link
Contributor

shankari commented Apr 3, 2023

@MaliheTabasi can we have a quick in-person discussion to clarify what you see in the label screen?

Given that 18th of March is the last day of previous week, this place card can be displayed both at the end of previous week and at the start of current week to avoid splitting. However it only appears once if we are displaying all trips to the user (not the weekly view).

I don't think we can make progress on this until you have a better sense of how the timeline works. For example:

  • There is no "Weekly View" versus "All Trips"
  • We don't flip between weeks, so there is no "previous week" and "this week"

If you have been collecting data since we released the first trip labeling version (March 13), you should have one month worth of data on at least one of your test phones, so we should be able to show you the differences.

@MaliheTabasi
Copy link
Author

@shankari Yes, definitely.
Just wanted to give you an update regarding our expectations for the time-use feature. Taha and I had a meeting and decided that activities that starts form one day and ends in another day are not essential for our study. So, we can stop worrying about that and instead focus on fixing the errors with the current basic time-use feature.

1- Currently, user can insert an activity with an end time earlier than its start time. Please see the screenshot below where an activity started at 6:49PM and ends at 6:46PM which is not reasonable. I suggest adding a validation to the end time of activity insertion such that one cannot insert activity with end time earlier than its start time.
msg6201276735-663708

2- When there is a Place between two trips that are in two different days, I think it is not a good idea to prefill the end time of the activity with the start time of the second trip in next day (because in this case we may prefill activity such that its end time is earlier than its start time). I recommend setting it to 11:59PM, if date is the same as the first trip. The same logic can be implemented if date is the same as the second trip. In that case, the start time can be prefilled with 6AM and the end time with the start time of the second trip.

3- In the current version, when user adds activities with different dates, she cannot see the dates in the Place card and it is vague that to what date each activity belongs. Please see the screenshot below. In this screenshot these two activities are in two different dates but this is not clear from what we display to the user. Can you please add a date what we display as the activity in the place card?
msg6201276735-663707

@shankari Could you please let me know if you still think we need to chat in person? If yes I'm available on Wednesday or Thursday (Sydney time).

@shankari
Copy link
Contributor

shankari commented Apr 6, 2023

@MaliheTabasi if you have been tracking updates to the issue, you will see that we already have fixes for the issues that you originally reported (which I think are the issues that you have reported here #871 (comment)).

  1. Add start time, end time, and date to each Place card
  2. The button styles have been unified. The start and end time tags have also been unified and will show on every card.
  3. Survey timestamps are also validated as requested.

Please test against version 1.2.2.

May I also suggest filing separate issues for each pending issue so that we can check each issue off once it is done?

That way we can more easily keep track of the easy issues which are already fixed, and the more complex issues which require additional discussion/design.

@MaliheTabasi
Copy link
Author

@shankari Thanks for fixing those issues. Sure, I will do so.

@shankari
Copy link
Contributor

Closing this since we have filed separate issues for the various bugs/new features

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

No branches or pull requests

4 participants