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

Incorporate Label UI changes from ceo_ebike_project_stage into master #794

Merged
merged 38 commits into from
Aug 20, 2021

Conversation

GabrielKS
Copy link
Contributor

@GabrielKS GabrielKS commented Aug 20, 2021

To maintain fine-grained control of what was added, this was implemented as follows:

  1. Browse the master vs. ceo_ebike_project_stage comparison (permalink: 6f0235d...a98d8b7) to figure out what commits should be brought over
  2. Conclude that everything starting on July 20, 2021 should be brought over except that 9a56787 (and thus 86cd8f3) must be handled specially because trip_confirm_options.json does not exist in master
  3. Run the following command to cherry-pick everything in the comparison since July 20, 2021 except those two commits:
git cherry-pick -m 1 -X theirs --allow-empty 5dc1da1 12ac1d2 12c4f3e dee1429 3dcd803 64f7b70 0908943 82884fa 88a772c 595f4d5 7fc0d37 d7fc607 01d7023 bb6c538 4e352e5 d3a918d 22f5772 159980a fa8bce9 f28742d 4d4b0b6 215c051 fb525d3 cdfc945 7e1a015 356cb30 a4b5ad2 b97d9d7 74182ee 4b64e58 1a6a617 d7e8cca a0b7194 9986ebf 7520753 a98d8b7
  1. Let the command run, running git commit --allow-empty and git cherry-pick --continue whenever prompted to capture empty merge commits
  2. Make a new commit bringing the changes in 9a56787 into the sample trip_confirm_options.sample.json file
  3. Test, remember that 12ac1d2 was CanBikeCO-specific (maybe because of the three labels instead of two?); revert it
  4. Test again, seemingly successfully:

Successful cherry-pick

GabrielKS and others added 30 commits August 19, 2021 16:03
 + As requested by Shankari, the Label screen now loads the most recent
   trips at the bottom and one scrolls up to load more (like a chat app)
 - The implementation is a bit hacky because the ion-infinite-scroll
   element cannot implement this behavior for us in this old version of
   Ionic
 - Does not yet reverse the direction of the trip start/end time markers

Tested on both iOS and Android, but would appreciate it if someone else
tested it as well before it gets merged.
- Add the ClientStats module to infinite scroll
- Add a new kind of client stat
- Mark the stat when the button is pressed

Testing done:
- Hacked the code to ensure that verifiability is always set to "can-verify"
- Verified the trip
- "End trip and force sync"
- Confirmed that the value was pushed
```
2021-08-04 15:37:25,004:DEBUG:123145498861568:Updated result for user = 113aef67-400e-4e21-a29f-d04e50fc42ea, key = stats/client_nav_event, write_ts = 1628116460.869 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('610b16a53906cf6a2964f11b'), 'ok': 1.0, 'updatedExisting': False}
2021-08-04 15:37:25,005:DEBUG:123145498861568:Updated result for user = 113aef67-400e-4e21-a29f-d04e50fc42ea, key = stats/client_nav_event, write_ts = 1628116560.485 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('610b16a53906cf6a2964f11d'), 'ok': 1.0, 'updatedExisting': False}
2021-08-04 15:37:25,007:DEBUG:123145498861568:Updated result for user = 113aef67-400e-4e21-a29f-d04e50fc42ea, key = stats/client_time, write_ts = 1628116588.745 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('610b16a53906cf6a2964f11f'), 'ok': 1.0, 'updatedExisting': False}
```
- Confirmed that the value was in the usercache at the end

```
edb.get_usercache_db().find({"metadata.key": "stats/client_nav_event"}).distinct("data.name")
['app_launched',
 'checked_diary',
 'checked_inf_scroll',
 'expanded_trip',
 'notification_open',
 'opened_app',
 'sync_launched',
 'verify_trip']
```

+ two robustness fixes where we check for the inferred labels and the
expectation before using them. These should not occur but it is good to avoid
crashing if they do!
Track when the verify button is pressed + robustness fixes
So people can access `to_label` as the first thing.
Requested by one of the 4CORE admins

Testing done:
- launched the app
- label screen was shown first
Reverse Label screen scrolling direction
 + Track when the user switches between filter tabs
 + Track existing green and yellow labels present when trips are verified
 + Track existing green and yellow labels present when labels are selected

Tested by verifying that the desired messages appear in the backend database
 + Log existing information before we change it!
**This is a test**
It can address some of the feedback about figuring out which trip is being
displayed.

Testing done:
- Displayed in the emulator
- Experimented with zooming
The auto-load on scroll feature, so common to messaging and chat apps is
actually fairly complicated to implement correctly in the case in which
we display a filtered list. In particular:
- the additional scrolling required to maintain "place" in the list triggers
  multiple "scroll" calls even when the user is not doing anything, which leads
  to multiple calls to the server.
- if new entries are not added to the list after the server call, then the
  condition that triggered the server call (top < threshold) persists, leading
  to multiple calls again.

See e-mission/e-mission-docs#662

This is an inherent limitation of using auto-load on scroll for a
filtered list and also happens with the standard ionic component,
e-mission/e-mission-docs#662 (comment)

So instead of butting my head against the wall getting auto-load to work, I
switched to manual load, similar to the diary screen.
e-mission#769

As a bonus, this also helps with e-mission/e-mission-docs#659
It indicates how many of the trips are displayed in **this filter**, should
help to clarify questions around "missing trips" and allow users to explore
other tabs as necessary.

Testing done:
- loaded data for a user with a single trip
- labeled that trip so the user had no trips
- loaded data for a user with multiple trips

In all cases, used the button to scroll all the way to the oldest trip.
Did not encounter any issues.
This fixes one of the minor fit-and-finish issues
"If you go to "All Trips", you are at the top, showing the oldest trips, and have to scroll all the way to the bottom"
from e-mission/e-mission-docs#662
…to_label

Switch the infinite scroll from auto-load on scroll to a button
So users can figure out whether the trips are delayed or just in the other
filters.

Testing done:
- Loaded data, confirmed that the start and the end were displayed
Because as the long day wears on, it is increasingly likely that we would have
processed some trips but not others.

Testing done:
Checked that the time was updated
…to_label

Display the start date **and end date** for the processed trips
to give users a chance to correct the trip.

The implementation is fairly straightforward.
- as a trip is modified, set its `waiting for modification` flag to true
- ensure that trips that are waiting for modification are not filtered out
- create a timeout to set the flag to false and recompute
- cancel any existing timeouts to avoid recomputations

Testing done:
- Set all three labels for a trip
- Saw the logs:

Open first popover, create new timeout promise, nothing to cancel

```
DEBUG:in openPopover, setting draftInput = {"start_ts":1628366684.0019567,"end_ts":1628367280.652}
DEBUG:in storeInput, after setting input.value = walk, draftInput = {"start_ts":1628366684.0019567,"end_ts":1628367280.652,"label":"walk"}
DEBUG:trip starting at 2021-08-07T13:04:44.001957-07:00: creating new timeout
DEBUG:trip starting at 2021-08-07T13:04:44.001957-07:00: cancelling existing timeout undefined
```

Open second popover, create new timeout promise, cancel previous

```
DEBUG:in openPopover, setting draftInput = {"start_ts":1628366684.0019567,"end_ts":1628367280.652}
DEBUG:in storeInput, after setting input.value = home, draftInput = {"start_ts":1628366684.0019567,"end_ts":1628367280.652,"label":"home"}
ionic.bundle.js:3063 [Violation] 'touchend' handler took 150ms
DEBUG:trip starting at 2021-08-07T13:04:44.001957-07:00: creating new timeout
DEBUG:trip starting at 2021-08-07T13:04:44.001957-07:00: cancelling existing timeout [object Object]
ionic.bundle.js:5301 [Intervention] Ignored attempt to cancel a touchstart event with cancelable=false, for example because scrolling is in progress and cannot be interrupted.
self.touchStart @ ionic.bundle.js:5301
```

Open third popover, create new timeout promise, cancel previous

```
DEBUG:in openPopover, setting draftInput = {"start_ts":1628366684.0019567,"end_ts":1628367280.652}
DEBUG:in storeInput, after setting input.value = bike, draftInput = {"start_ts":1628366684.0019567,"end_ts":1628367280.652,"label":"bike"}
DEBUG:trip starting at 2021-08-07T13:04:44.001957-07:00: creating new timeout
DEBUG:trip starting at 2021-08-07T13:04:44.001957-07:00: cancelling existing timeout [object Object]
```

After a significant time, recompute

```
DevTools failed to load SourceMap: Could not load content for http://10.0.2.2:3000/socket.io/socket.io.js.map: Connection error: net::ERR_CONNECTION_TIMED_OUT
DEBUG:trip starting at 2021-08-07T13:04:44.001957-07:00: executing recompute
```

- Set two labels for a trip and then the third

Set first two labels

```
DEBUG:in openPopover, setting draftInput = {"start_ts":1628294651.152,"end_ts":1628297929.551}
DEBUG:in storeInput, after setting input.value = walk, draftInput = {"start_ts":1628294651.152,"end_ts":1628297929.551,"label":"walk"}
DEBUG:trip starting at 2021-08-06T17:04:11.152000-07:00: creating new timeout
DEBUG:trip starting at 2021-08-06T17:04:11.152000-07:00: cancelling existing timeout undefined
DEBUG:in openPopover, setting draftInput = {"start_ts":1628294651.152,"end_ts":1628297929.551}
DEBUG:in storeInput, after setting input.value = work, draftInput = {"start_ts":1628294651.152,"end_ts":1628297929.551,"label":"work"}
DEBUG:trip starting at 2021-08-06T17:04:11.152000-07:00: creating new timeout
DEBUG:trip starting at 2021-08-06T17:04:11.152000-07:00: cancelling existing timeout [object Object]
```

Recompute is run, but the trip is not filtered because only two labels are filled out

```
DEBUG:trip starting at 2021-08-06T17:04:11.152000-07:00: executing recompute
```

Third label is filled out

```
DEBUG:in openPopover, setting draftInput = {"start_ts":1628294651.152,"end_ts":1628297929.551}
DEBUG:in storeInput, after setting input.value = no_travel, draftInput = {"start_ts":1628294651.152,"end_ts":1628297929.551,"label":"no_travel"}
DEBUG:trip starting at 2021-08-06T17:04:11.152000-07:00: creating new timeout
DEBUG:trip starting at 2021-08-06T17:04:11.152000-07:00: cancelling existing timeout undefined
```

Final recompute is called and trip is filtered

```
DEBUG:trip starting at 2021-08-06T17:04:11.152000-07:00: executing recompute
```
…to_label

Delay the deletion of entries from the To Label screen
…to_label

Change the delete delay to 30 sec + add day of the week for clarity
 + Make room for walkthrough button on Label navbar
 + Add walkthrough tour targets and content
 + That plus a few final tweaks to the walkthrough itself
GabrielKS and others added 8 commits August 19, 2021 16:04
 + Go back to using col-XX for width
 + Use Promise instead of $q
Temporarily disable auto loading of walkthrough
 + Replays e-mission@9a56787
   (which is included in e-mission@86cd8f3)
 - Note that there remain older differences between the CanBikeCO trip_confirm_options.json and this sample
@shankari shankari merged commit 4dc03af into e-mission:master Aug 20, 2021
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

Successfully merging this pull request may close these issues.

2 participants