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

Merge from master to pull in fixes #612

Open
wants to merge 548 commits into
base: urap-2017-emotion
Choose a base branch
from

Conversation

shankari
Copy link
Contributor

@shankari shankari commented Nov 1, 2019

No description provided.

shankari and others added 18 commits August 6, 2019 07:23
Fix today button by upgrading to most recent version of the datepicker
Updated carbon footprint values for LIGHT_RAIL and TRAM
The conflict must have been caused by a previous change to comment out the
`check-sensed-data` button in the fork. After it was restored in this change,
they are identical, so I just retained the existing functionality.
…e speed is positive and returning 0 if it is the case to not break the calorie sum with a wrong return type
…nual_lib/leaflet/uglify-js-3.6.6

Bump uglify-js from 2.5.0 to 3.6.6 in /www/manual_lib/leaflet
ericafenyo and others added 11 commits January 13, 2020 12:55
…to continue using the legacy Apache HTTP client.
Convert the translation scripts to be executable
add note about cocoapods version without problems
add the required manifest field for apps that target API 28 and want …
The related manifest change was already checked in thanks to @ericafenyo
(#616)
Add android schema so that we can use `android:` tags in the xml
+ update ionic deploy to the `remove_swift_class` version
This fixes the last performance issue by allowing us to load the data one batch
at a time. This cuts down on both loading time and processing time significantly.

Highlights:
- shift the trip loading to work with one batch at a time
- this involved significant restructuring of the service code, which used to
  read both the trips and the pending labels in parallel. It would be
  unnecessary overhead to load the labels every time, specially because they
  won't change between subsequent pagination-based loads.
- So the new structure is:
    - we load the pending labels first. We read the timeline `processed_ts` at
      the same time, in order to read only the labels after that, and we return
      that as well.
    - we set that as the beginning of the timeline, under the argument that
      probably the user will want to see their most recent trips first, and
      read a weeks-worth of data at a time.
    - as we get to the bottom of the list, the infinite scroll code will call
      back to read more data. We will NOT re-load labels, only trips for the
      past week and so on.
    - Finally, we get to the end of the timeline and will not get any more trips.
      We stop then.
    - Note that when updating the currently loaded end timestamp after a batch
      load, we offset it by one second. If we don't do this, then the `$gte` on
      the server side ensures that the last trip is repeated.

- Minor fix to make the filter match the leaderboard
    `.reduce((acc, val) => acc && val, true);` makes it so that a trip is only
        unlabelled if all labels are missing. While exploring the data, we
        found that almost all users had only filled in the mode and purpose and
        not the replaced mode. That is actually OK - we primarily need the
        replaced mode for eBike trips. So I change the filter to match the
        leaderboard and not the other way around.

- Minor fit and finish fixes
    - Change the "No trips found" label to say what to do
        - remove the filter if that the reason
        - scroll for more if we are not at the end
    This required some rearranging of elements to make sure that the full
        message was not seen.

    - Change the ordering of start and end slugs so that we would have an
      unbroken timeline instead of one which jumps around from end to previous
      start.

      Consider trips from 9:20 -> 10:10 and 10:40 -> 11:30

      before the fix, they would have displayed as below. Note that time increases
      from 10:40 to 11:30 and decreases from 11:30 to 9:20 for a jerky experience.

        ```
        10:40
        11:30

        9:20
        10:10
        ```

      after the fix, they would have displayed as below. Note that time flows smoothly
      decreasing down the timeline.

        ```
        11:30
        10:40

        10:10
        9:20
        ```
Based on feedback from J that times along are insufficient.

Changes:
- Call the place filling code from the controller rather than the service
- After finishing, apply the changes using `$scope.$apply`
- Invoke the call on every incoming batch call
- Change all the templates to work with the new location (in the trip object,
  not in geojson)
- This ran into issues with rate limiting on nominatim, so
    - imported bottleneck
    - set up a queue of the placeLimiting calls
    - use the queue to schedule the calls
- Make the list items bigger since we need more room for the new data

Bunch of other minor fixes:
- Remove the unnecessary `$scope.$apply` from the batch read
- Switch a bunch of log statements from console.log to Logger.log
- Switch the filling of additional information from map to forEach
    to avoid unnecessary duplication of memory
Without this change, if there is more than a one week gap in the middle of a
timeline, the infinite scroll stops. This is because we are not retrieving a
certain number of trips in each batch, but we assume that there are no more
trips if our batch gets zero entries.

So, we use new server-side changes to retrieve both the last element and the
first element on the server. We only end the scroll when the first trip that we
see is the first trip that the pipeline processed.

+ new wrapper to call the new server method
+ minor changes to return values
+ some additional control to update the endTs in the zero-batch-but-no-trip-end
case. We just bump up the currentEndTs by a week.
Since the infinite scroll component calls it anyway.
+ some minor logging changes
With the previous structure, it would sometimes be unclickable, and we would
need to click on the address instead. Moving it to the same row as the address
but as an additional column makes it more likely to be clickable.
Now that we rely on the infinite scroll to load more entries, we need to ensure
that the displayList is also zeroed out on setup. Otherwise it looks like there
are plenty of entries, and the infinite scroll won't kick in.

So if you log out and login, then the trips don't get replaced.

This is definitely a corner case, since most users are not going to login to
somebody else's account. But if people want to refresh, they need to be able to
see the newly refreshed value not whatever they had before.
Otherwise, it is completely hidden.
We may want to offer the option to resize the map div in the future
And to display only selected ones in the UI

To add new filters, follow the template in `www/js/diary/infinite_scroll_filters.js`
To select a subset of filters, choose the values in `www/js/diary/infinite_scroll_list.js`

```
$scope.filterInputs = [
  InfScrollFilters.UNLABELED, InfScrollFilters.INVALID_EBIKE
];
```
For reasons I don't fully understand, the diary screen spacing was broken after
the labeling screen changes.

- worked on master
- didn't work on original `add_labeling_screen` branch
- didn't work on this branch

I double checked the HTML file on master and here: identical
Double checked the generated HTML for a single item on master and here: identical

Finally, gave up and created a row/col structure around it that ensures that it
is displayed correctly
Instead of repeating the list in both the HTML and the javascript

Changes:
- Include label, choose text and width into the input details
- In both controllers, extract those into `userInputDetails` arrays
- In both HTML files, use ng-repeat to display the defined user inputs
  programatically without DRY
+ add additional stats to track when people use it
- set the width
- ensure that we only have the unlabeled
- put it into the class in the HTML
[feat] Create a new infinite scroll screen that makes it easier to label trips and to see a full history
* Improve the matching of user inputs to cleaned trips

While matching user inputs on the server, found that user input matching was
broken for some cleaned trips on the phone.
e-mission/e-mission-docs#476 (comment)

Expanded the user input end check to fix.
e-mission/e-mission-docs#476 (comment)

Will merge into master after additional testing on the branch.

+ bump up the allowed delta to 15 minutes since the time threshold default for
the distance filter is 10 minutes.

* Fix regression in enhanced trip matching

The enhanced trip matching (75129db) caused a
regression in which a spurious trip (not a trip) that occurred after the real
trip fit the criteria for a match.

And since it was confirmed after the real trip, as you would expect while going
down the trip list, it was matched preferentially.
e-mission/e-mission-docs#476 (comment)

Fixed by checking the degree over overlap and rejecting too short matches

* Fix the infinite scroll user input matching

to be consistent with the refactor in
21ab3fa
and
6bd731a
The manual refresh was cool when we started, but it had a lot of problems after
we added the popups. Using the popup on the first entry would frequently
trigger the refresh, which would close the popup. And launching the popup again
would trigger the refresh again, ad-infinitum.

Instead, we use a manual refresh which is less cool but works well.
Minor things that have annoyed me forever
So it works on older versions of android and iOS
Not sure how necessary this backward-compat is, but the test phones are set up
to not upgrade, so I ran into this with them.
We should decide when to move beyond es5 to es2017 at least :)
es5 = 2009
es2017 = 2017

without this fix, we get a syntax error and white screen on older android
(force no update for webview) or iOS 12
Switch to es5-compatible version of bottleneck
- add a new button to the profile screen
- pull out the consent text into a separate file
- `ng-include` the consent text into the existing consent file
- create a new popover template and `ng-include` the consent text there as well
- convert the template load in intro.html back to a standard file for now

Next step: enable i18n

This will require us to move the `geti18nFile` method out of intro.js. I tried
a shortcut with `$rootScope`, but it doesn't work because `intro.js` is not
loaded if it is not the first initialization.

We might want to split up at least some parts of intro anyway as we work on the
onboarding and the status screen, since we will also want to access the status
screen from the onboarding and from the profile later.
This is a follow-on to 173c120
to add i18n support

Concretely:
- add a new service for i18n utils
- move the code to get the i18n filenames to the utils
- improve the implementation of the function
    - generalize the arguments to take in the default location so it is not hardcoded to intro
    - switch to `$http` instead of the weird copied-over file code
    - Print a substring of the result instead of everything
- change the intro to use it
- change the profile screen to use it
- change the trip JSON loading options to use it
- change the profile template to use the filename from the scope
- change the intro to use the filename from the scope
    - since it has some text about clicking buttons as well
    - so we can now hardcode the location of the consent-text file
Add a privacy link to the profile as well
And replace it with a note that it is linked from the profile page
Follow on to #744
So the text is not hidden on larger phones. See screenshots for a visual example.
The new location is close to the tracking on/off button, so it makes sense conceptually as well
Move the privacy policy link up
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.

8 participants