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 generic code (primarily stats) from tripaware/ceo_ebike to master #679

Closed
shankari opened this issue Oct 14, 2021 · 15 comments
Closed

Comments

@shankari
Copy link
Contributor

Let's keep track of them here without mucking up the current merge.
e-mission/e-mission-phone@7836019

Make the km vs. miles configurable
e-mission/e-mission-phone@22d38af

Adjust card size (currently cutoff after merge from master)

                <div id="diary-card" ng-class="tripgj.listCardClass" ng-style="{height: '355px'}">

e.g. https://www.geeksforgeeks.org/how-to-dynamically-get-the-content-height-of-a-div-using-angularjs/

@shankari
Copy link
Contributor Author

I have implemented the first two, but the third one seems much harder to work with.
It is somewhat non-trivial to figure out how to hook into the loading callbacks to figure out the final item height.
But more seriously, it seems pretty challenging to resize list items on the fly even when we actually have the final size.

There are two hardcoded height entries - we set the item-height for the collection-repeat, and we set the height for the cards in the items.

        <div collection-repeat="trip in data.displayTrips" item-height="'160px'">
                <div id="diary-card" onload="fixSize()" ng-class="trip.listCardClass" ng-style="{height: '150px'}">

I can change the card heights using jquery, but without changing the item-height, it just makes the text be invisible.

$(".list-card").css("height","300px")
Before change After resize
Screenshot_1634313877 Screenshot_1634313901

@shankari
Copy link
Contributor Author

The item height was introduced in e-mission/e-mission-phone@e110f3f#diff-e694b32469d49b6682b8a207844dab1a6950a61c487d58216b109e9a0c1ccc85R34 and like most code by @krisma, has a hardcoded value and no documentation.

We didn't have it before, so I am not even sure the item-height is required.

@shankari
Copy link
Contributor Author

Removed <item-height>; we have some gap between entries, probably due to the start and end tags.
However, changing the card size still doesn't work.

Before resize After resize
Screenshot_1634314625 Screenshot_1634314637

@shankari
Copy link
Contributor Author

Looking at this further, this is because the collection-repeat has a transformation appended to it. Without fixing the transformation, the items will just overlap each other.

<div collection-repeat="trip in data.displayTrips" style="transform: translate3d(0px, 3570px, 0px); height: 211px; width: 393px;">
            <div class="start-time-tag-inf-scroll ng-binding">9:20 AM</div>
            <div style="padding-left: 19%;">

			 <ion-item id="diary-item" style="background-color: transparent;" class="list-item item">
                <div id="diary-card" ng-class="trip.listCardClass" ng-style="{height: '150px'}" class="list card list-card bg-light list-card-lg" style="height: 300px;">
        </div>

@shankari
Copy link
Contributor Author

shankari commented Oct 15, 2021

The transform is due to the use of collection-repeat
https://ionicframework.com/docs/v1/api/directive/collectionRepeat/

Using standard angular ng-repeat works fine wrt resizing, but ends up with all the start and end tags at the every top and the very bottom. This is probably due to the lack of transformation in ng-repeat.

Before After
Screenshot_1634316229 Screenshot_1634316256

@shankari
Copy link
Contributor Author

Going back to collection repeat, and then changing the actual transform values did work, but that seems a pretty complicated calculation to make, and one which will basically reproduce the code in ionic. And we are not even going to stay with ionic over the long term.

So here's my proposal to fix that:

  • Bump up the default size for CanBikeCO
  • if it works:
    • make the size be configurable via a scope variable
    • provide a button to change it manually

Let's see if the hack works

@shankari
Copy link
Contributor Author

The hack works. Yay!!
It is pretty manual, and the card doesn't get appear to be updated on the first resize, but if you press it enough times, it works 😄

Screen Recording 2021-10-15 at 11 08 25 AM mov

@shankari
Copy link
Contributor Author

shankari commented Oct 15, 2021

One more issue: report from a user on a Motorola G-Power that they cannot see the map.
They can see the map in the diary, but not in the label screen.

@shankari
Copy link
Contributor Author

The map in the diary screen has a height (derived from the item height, as we just saw).
The map in the label screen has

height="isAndroid? '50%': '600px'" width="100%"

We explicitly added the fixed value on iOS because the map is completely hidden otherwise.
e-mission/e-mission-phone@089797e

I wonder if motorola has a custom webview that behaves more like iOS. Let's just remove the android hack and see if it fixes the problem.

shankari added a commit to shankari/e-mission-phone that referenced this issue Oct 15, 2021
This fixes issues in which the labels are partially hidden on some phones, and
is a reasonable workaround for the inability to test with multiple phones.
We could even consider allowing users to save their perfect height in a local
setting later so they don't have to keep changing the settings every time.

I tried to do this automatically, but failed with the current UI
e-mission/e-mission-docs#679 (comment)
@shankari
Copy link
Contributor Author

shankari commented Oct 16, 2021

While implementing the fix to fill in additional fields (e.g. user_input) for unprocessed trips, it looked like we were reading unprocessed trips for subsequent days as well. Need to investigate further.

@shankari shankari reopened this Oct 16, 2021
@shankari
Copy link
Contributor Author

Loaded diary trips for 4th august 2016.

Seemed to process all trips - 4th, 5th, 6th, etc.

Processing trip starting at 2016-08-04T10:03:51.235000-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T10:41:32.136385-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T13:10:38.739684-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T13:40:36.959000-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T13:46:26.561801-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T14:06:52.592000-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T14:18:35.840464-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T14:39:38.288795-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T16:34:45.744782-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-05T04:55:09-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-05T07:26:36-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-05T17:23:23-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-05T17:51:41-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-09T18:04:30-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-10T09:54:31-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-10T11:07:23-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-10T17:43:53-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-10T18:11:17-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-10T19:28:28-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-10T19:32:48-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-11T09:20:41-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-11T11:23:09-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-11T11:45:20-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-11T14:08:04-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2021-10-13T21:20:04-07:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}

@shankari
Copy link
Contributor Author

Ah the error is because

DEBUG:about to query for unprocessed trips from Thu Aug 04 2016 19:38:38 GMT-0700 -> Sat Oct 16 2021 23:59:59 GMT-0700

On going to the next page...

DEBUG:about to query for unprocessed trips from Fri Aug 05 2016 17:25:52 GMT-0700 -> Sat Oct 16 2021 23:59:59 GMT-0700

It looks like we are reading all trips until the end of today, not that day. This is likely to be particularly problematic in the case where the user has a significant history and goes way back in it, and may be at least a partial contributor to "diary loading is slow".

Just to confirm, this code has been unchanged since the unprocessed trip functionality was first implemented.
e-mission/e-mission-phone@d97b474

@shankari
Copy link
Contributor Author

shankari commented Oct 16, 2021

Ah, although this particular code was unchanged, the restructuring in e-mission/e-mission-phone@f3ea875 was a bit sloppy, and meant that for

    var addUnprocessedTrips = function(processedTripList, day, completeStatus) {

day and completeStatus were both undefined.

Since complete status was undefined, we read unprocessed trips, and since day was undefined, moment(day) became moment(undefined) which is today.

shankari added a commit to shankari/e-mission-phone that referenced this issue Oct 16, 2021
The retrieval code was refactored for greater performance
e-mission@f3ea875

However, this introduced some minor regressions.
e-mission/e-mission-docs#679 (comment)

which were caused due to incorrect parameter passing and resulting "undefined" values.
e-mission/e-mission-docs#679 (comment)

Testing done: while reading data for 4th August
----

we detect that the pipeline has finished running

```
INFO:Selected date is :1470294000000
getting pipeline complete timestamp
DEBUG:complete_ts = 1470951950.271(Thu Aug 11 2016 14:45:50 GMT-0700) end of current day = 1470380399(Thu Aug 04 2016 23:59:59 GMT-0700) retVal = true
Returning 1470951950.271,true
```

we only read entries after `1470951950.271 = 2016-08-11T21:45:50.271000+00:00`

```
getRawEntries: about to get pushGetJSON for the timestamp
About to return message {"key_list":["manual/mode_confirm"],"start_time":1470951950.271,"end_time":1634403476,"key_time":"metadata.write_ts"}
getRawEntries: about to get pushGetJSON for the timestamp
About to return message {"key_list":["manual/purpose_confirm"],"start_time":1470951950.271,"end_time":1634403476,"key_time":"metadata.write_ts"}
```

we got a fair number of unprocessed inputs, mainly because we only filled in the inputs way after the pipeline

```
About to show 'Reading from server'
DEBUG:About to dedup localResult = 0remoteResult = 52
DEBUG:Deduped list = 52
DEBUG:About to dedup localResult = 0remoteResult = 55
DEBUG:Deduped list = 55
unprocessed manual inputs  52 MODE, 55 PURPOSE
```

And we only processed trips from the 4th.

```
DEBUG:while reading data for 1470294000000 from server, got nTrips = 9
About to hide 'Reading from server'
Finished hiding ionicLoading, returning list of size 9
Reading trips from server finished successfully with length 9 completeStatus = true
trip count = 9, calling processTripsForDay
About to show 'Processing trips'
0:2016-08-04T10:03:51.235000-10:00, 1880.7650001049042
1:2016-08-04T10:41:32.136385-10:00, 8056.584614753723
2:2016-08-04T13:10:38.739684-10:00, 605.260315656662
3:2016-08-04T13:40:36.959000-10:00, 135.75
4:2016-08-04T13:46:26.561801-10:00, 745.4381992816925
5:2016-08-04T14:06:52.592000-10:00, 280.5680000782013
6:2016-08-04T14:18:35.840464-10:00, 936.7305357456207
7:2016-08-04T14:39:38.288795-10:00, 5956.420205116272
8:2016-08-04T16:34:45.744782-10:00, 232.60321807861328

Processing trip starting at 2016-08-04T13:10:38.739684-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T13:40:36.959000-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T13:46:26.561801-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T14:06:52.592000-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T14:18:35.840464-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T14:39:38.288795-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
Processing trip starting at 2016-08-04T16:34:45.744782-10:00 {data: {…}, start_place: {…}, style: ƒ, onEachFeature: ƒ, pointToLayer: ƒ, …}
About to hide 'Processing trips'
```
@shankari
Copy link
Contributor Author

First round of fixes:
e-mission/e-mission-phone#800

Second round of fixes:
e-mission/e-mission-phone#801

shankari added a commit to e-mission/e-mission-phone that referenced this issue Oct 16, 2021
This fixes issues in which the labels are partially hidden on some phones, and
is a reasonable workaround for the inability to test with multiple phones.
We could even consider allowing users to save their perfect height in a local
setting later so they don't have to keep changing the settings every time.

I tried to do this automatically, but failed with the current UI
e-mission/e-mission-docs#679 (comment)
shankari added a commit to e-mission/e-mission-phone that referenced this issue Oct 16, 2021
To see if it fixes
e-mission/e-mission-docs#679 (comment)

It is already a fixed height on iOS because otherwise it doesn't work
089797e
@shankari
Copy link
Contributor Author

The new resize icon

Screen Shot 2021-10-18 at 2 12 58 PM

on some phones, causes overlaps with the existing tabs.

Screen Shot 2021-10-18 at 2 12 08 PM

And there are cases in which there were overlaps even before, which this will only exacerbate.

Screen Shot 2021-10-18 at 2 11 17 PM

We should really remove the clutter on the top bar, and change the filters to be icons, or a dropdown.

shankari added a commit to shankari/e-mission-phone that referenced this issue Oct 18, 2021
The top bar on the "infinite scroll" screen is already cluttered, and sometimes
there are overlaps. Adding one more button introduces overlaps even where we
didn't have them before.

We should really replace the button-based labels at the top with icons, or
convert to a drop-down.

e-mission/e-mission-docs#679 (comment)
shankari added a commit to e-mission/e-mission-phone that referenced this issue Oct 18, 2021
The top bar on the "infinite scroll" screen is already cluttered, and sometimes
there are overlaps. Adding one more button introduces overlaps even where we
didn't have them before.

We should really replace the button-based labels at the top with icons, or
convert to a drop-down.

e-mission/e-mission-docs#679 (comment)
shankari added a commit to e-mission/e-mission-phone that referenced this issue Oct 21, 2021
The top bar on the "infinite scroll" screen is already cluttered, and sometimes
there are overlaps. Adding one more button introduces overlaps even where we
didn't have them before.

We should really replace the button-based labels at the top with icons, or
convert to a drop-down.

e-mission/e-mission-docs#679 (comment)
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

1 participant