[regression] fix to display the date correctly fails

[shankari]

#819 (comment)

Error: Maximum call stack size exceeded.
equals@ionic://localhost/_app_file_/Users/kshankar/Library/Developer/CoreSimulator/Devices/181596B5-9D24-40BD-9D93-C6D93038ACF6/data/Containers/Data/Application/7AC9801C-42C1-4189-A8DE-A37FC5AEE306/Library/NoCloud/phonegapdevapp/www/lib/ionic/js/ionic.bundle.js:14431:22

or

Error: Maximum call stack size exceeded.
equals@ionic://localhost/_app_file_/Users/kshankar/Library/Developer/CoreSimulator/Devices/181596B5-9D24-40BD-9D93-C6D93038ACF6/data/Containers/Data/Application/7AC9801C-42C1-4189-A8DE-A37FC5AEE306/Library/NoCloud/phonegapdevapp/www/lib/ionic/js/ionic.bundle.js:14427:16
equals@ionic://localhost/_app_file_/Users/kshankar/Library/Developer/CoreSimulator/Devices/181596B5-9D24-40BD-9D93-C6D93038ACF6/data/Containers/Data/Application/7AC9801C-42C1-4189-A8DE-A37FC5AEE306/Library/NoCloud/phonegapdevapp/www/lib/ionic/js/ionic.bundle.js:14447:20

where the related line is

14421 function equals(o1, o2) {
14422   if (o1 === o2) return true;
...
14426   if (t1 == t2 && t1 == 'object') {
14427     if (isArray(o1)) {
14428       if (!isArray(o2)) return false;
14429       if ((length = o1.length) == o2.length) {
14430         for (key = 0; key < length; key++) {
14431           if (!equals(o1[key], o2[key])) return false;
14432         }
14433         return true;
14434       }
...
14445       for (key in o1) {
14446         if (key.charAt(0) === '$' || isFunction(o1[key])) continue;
14447         if (!equals(o1[key], o2[key])) return false;
14448         keySet[key] = true;
14449       }
..
}

[shankari]

git revert 465f917a4b02a8d0c5a50eca16867052a2c96a55 (e-mission/e-mission-phone#897) fixes it. But then we don't have the changes to fix the date. Let's spend some time seeing if we can fix it.

Through experimenting with removing code, I can confirm that

            trip.nextTrip = ctList[tIndex+1];
            trip.prevTrip = {display_date: trip.display_date};

works and

            trip.nextTrip = ctList[tIndex+1];
            if (tIndex == 0) {
              trip.prevTrip = {display_date: trip.display_date};
            } else {
              trip.prevTrip = ctList[tIndex-1];
            }

does not

Not sure why setting the previous trip fails

[shankari]

For now, I have this as the following, which doesn't crash and shows the date above each trip.

            trip.nextTrip = ctList[tIndex+1];
            trip.prevTrip = {display_date: trip.display_date};

@sebastianbarry this is your highest priority issue now.

[shankari]

[shankari]

Testing done: previously broken load works

[sebastianbarry]

[Yesterday 10:33 AM] Shankari, K.
A simple workaround might be to scrap the date separator altogether and just display the date and time for each trip

[sebastianbarry]

A solution may be to get rid of the date label header entirely, since this was a new design added by Tyler in the Unified Label/Diary screens PR he made

We have 3 options to solve this:

1. Fix the date header right now as is and keep it. This does not account for many trips on one day because you will have to scroll to the top trip to remember what day you're on. There is also a place for the Date picker button
2. Keep the date label persistent at the top, as a Div, and then change the date in the div at the top once you scroll to a new date. You can always see the date and there is a place for the Date picker button See http://man.hubwiz.com/docset/Ionic.docset/Contents/Resources/Documents/ionicframework.com/docs/api/service/$ionicScrollDelegate/index.html for keeping track of where you are on the scroll
3. Get rid of the date label entirely and write the date over each trip box Easiest, but no place for the date picker button

[shankari]

Yes the ionicScrollDelegate is at https://ionicframework.com/docs/v1/api/service/$ionicScrollDelegate/
we know what the scroll position is, but I am not sure how easy it is to map it to a list item

• Maybe getting the height of the scroll and dividing by the number of items will let you find the range of positions for each item which you can then match
• or maybe you do need to have separators as well, and you can find the next separator and get the date from it or something.

The list also has an ionicListDelegate which doesn't seem to have what we want either.
https://ionicframework.com/docs/v1/api/service/$ionicListDelegate/

[shankari]

I would also suggest looking at the scroll element and the <ion-item elements in the DOM as you scroll and see how the elements get updated and whether we can use some of those fields

[shankari]

It looks like the items do have the offsets stored in them. If this corresponds to the scroll position, we might be done

[sebastianbarry]

For now, I am choosing to go with option 3: Get rid of the date label entirely and write the date over each trip box Easiest, but no place for the date picker button

I addressed this in e-mission/e-mission-phone#904

Sidenote: I am still having troubles with my pipeline running on my test data, which was taking me a while to look into, so my testing is ONLY conclusive for 1 DAYS WORTH OF DATA, I did not test with multiple days worth of data

My thoughts:

• I got stuck trying to implement option 2: Keep the date label persistent at the top, as a Div, and then change the date in the div at the top once you scroll to a new date
• First, I could not find a way to have persistent floating text as a small button on the screen (as seen in WhatsApp messaging, when you scroll between days of messages: a small text bubble pops up over the scrolling content to tell you what day you're on) I found that ion has an ion-fab but this didn't seem to be what I am looking for exactly since its just a circular bubble button
• Second, I was able to get ionicScrollDelegate to display the scroll height in pixels, but I was having trouble finding the content that you posted @shankari which had the height offset stored in the HTML. I wasn't able to identify where in the HTML this is stored, or how we would getElementByID this to store the height

Next steps:

• Get a floating text bubble to stay persistent which displays the trip's date
• Implement some sort of horizontal line between trip items with different dates (I think it should have the display_date on the horizontal line, as how it has been before. But if we can't find a way to make prevTrip work or some way of correctly displaying the date over the HR horizontal line, then we should just make it a flat horizontal line)

[shankari]

@sebastianbarry I don't know that we need to have a floating text bubble. Why can't it just be a button in the header?
The functionality we want is that the date is visible consistently in the header. Not sure why floating/non-floating should make a difference.

ion-fab is in the most recent version of ionic. You need to look at v1. Just searching for ionic is not enough.

wrt

but I was having trouble finding the content that you posted @shankari which had the height offset stored in the HTML.

It is 3 lines above the line highlighted in your screenshot.

<div collection-repeat=... style=transform(0px, 300px,...

I put a screenshot showing it so not sure why that would be confusing...

[shankari]

Sidenote: I am still having troubles with my pipeline running on my test data, which was taking me a while to look into, so my testing is ONLY conclusive for 1 DAYS WORTH OF DATA, I did not test with multiple days worth of data

Please finish testing ASAP. I do not want to merge untested code, and I don't want to be responsible for testing everything

[sebastianbarry]

Sidenote: I am still having troubles with my pipeline running on my test data, which was taking me a while to look into, so my testing is ONLY conclusive for 1 DAYS WORTH OF DATA, I did not test with multiple days worth of data

Please finish testing ASAP. I do not want to merge untested code, and I don't want to be responsible for testing everything

With the help of @shankari I was able to figure out this KeyError issue on my e-mission-server, and finish the testing with multiple days:

It displays well, testing is completed for this change

[sebastianbarry]

@sebastianbarry I don't know that we need to have a floating text bubble. Why can't it just be a button in the header? The functionality we want is that the date is visible consistently in the header. Not sure why floating/non-floating should make a difference.

ion-fab is in the most recent version of ionic. You need to look at v1. Just searching for ionic is not enough.

wrt

but I was having trouble finding the content that you posted @shankari which had the height offset stored in the HTML.

It is 3 lines above the line highlighted in your screenshot.

<div collection-repeat=... style=transform(0px, 300px,...

I put a screenshot showing it so not sure why that would be confusing...

1. I see that ion-fab is not for Ionic v1, so this is not an option for sure. Thank you for the clarification
2. I also see where these height offsets are stored now, thank you for seeing that. While I know where the offsets are, I'm still unsure how to get these values in the JavaScript:

<div collection-repeat="trip in data.displayTrips" style="transform: translate3d(0px, 1560px, 0px); height: 391px; width: 414px;">
<div collection-repeat="trip in data.displayTrips" style="transform: translate3d(0px, 1950px, 0px); height: 391px; width: 414px;">

for example, has attribute style="transform: translate3d(0px, 1560px, 0px), with 1560px being the value we need to pass into $ionicScrollDelegate.scrollTo() function. However, how do I get the 1560px in the Javascript? I know there is such thing as getElementByID and getElementByClass but there is nothing unique between these <div collection-repeat=...

[sebastianbarry]

@shankari Here is the cherry-picked PR for these changes for the master branch: e-mission/e-mission-phone#906

[sebastianbarry]

I am currently working on:

Next steps:

• Get a floating text bubble to stay persistent which displays the trip's date
• Implement some sort of horizontal line between trip items with different dates (I think it should have the display_date on the horizontal line, as how it has been before. But if we can't find a way to make prevTrip work or some way of correctly displaying the date over the HR horizontal line, then we should just make it a flat horizontal line)

I am able to restructure the header just a bit so that we can account for the to-label, etc. filter buttons, along with displaying the currently scrolled display_date as the header title:

However, I am getting stuck on dynamically knowing what the scroll position is. I have it right now, so that whenever you click the "refresh" button, THAT is when it updates the scroll position.
I also have another issue, where when you are scrolling on the page, the loaded content CHANGES DYNAMICALLY. See this video:

Screen.Recording.2022-11-01.at.11.11.14.AM.mov

A solution idea I have for this, is that we can find out where/when this HTML element dynamic updating is happening, we can input our getScrollPosition() function inside of this update, and also compare our scrollPosition to the list to find out which element is at that position, and then display that element's display_date in the header and update the header.

---

@shankari do you know if this is a viable solution? Am I on the right track? Could you explain why or how the HTML page is dynamically changing as I scroll on the page?

[shankari]

@sebastianbarry it is dynamically changing because we use collection-repeat, which allows us to display large lists by creating a small set of directives and automatically swapping out what is displayed in them as we scroll.

I don't think there is a hook into the collection-repeat
But even without collection-repeat, the list of visible entries would keep changing.
You need to listen to the scroll callbacks

[shankari]

I am not sure how to do this with ionic v1, though, I found some random issues complaining about the scrolling
and the test repo has this
https://github.com/mrtomrichy/Ionic-iOS8-Scroll-Issue/blob/master/www/js/app.js

$ionicConfigProvider.scrolling.jsScrolling(false);

I think we have to go to the source code.

[shankari]

Looking at the v1 source code for scroll,
https://github.com/ionic-team/ionic-v1/search?q=scroll

I can find the unit test for the scroll controller, which looks like it triggers on scroll
https://github.com/ionic-team/ionic-v1/blob/eefba1331bc9206767c651912db71e9ed34850d5/test/unit/angular/controller/scrollController.unit.js#L115

[shankari]

and searching for the corresponding on('scroll') finds
https://github.com/ionic-team/ionic-v1/blob/eefba1331bc9206767c651912db71e9ed34850d5/js/angular/directive/scroll.js#L33

* @param {expression=} on-scroll Called whenever the user scrolls.
* @param {expression=} on-scroll-complete Called whenever the scrolling paging is completed.

Having said that, if this takes more than the end of today to experiment with, I would drop this approach and just keep the date in the trip item. The date selector at the top can be static and set to today.

[sebastianbarry]

I found this guide online, to get the scroll updating dynamically:

Screen.Recording.2022-11-02.at.10.52.00.AM.mov

I was able to get the page to respond to scrolling using the native JavaScript onscroll="myFunction()" HTML element with a <script> myFunction() { ... } </script> appended to the bottom of the page:

Screen.Recording.2022-11-02.at.10.59.32.AM.mov

The only problem with doing it this way, is that I can't call an angular $scope.function like this, so I looked for an Angular 1 version of the onscroll element. I found this page:

I tried putting this into the code but it is giving console errors Error: Unknown provider: $elementProvider <- $element

I looked up this error but while it is the same error message, the thread is about a slightly different subject: https://stackoverflow.com/questions/12494825/error-unknown-provider-elementprovider-element

@shankari do you know if I'm on the right path? I see that I strayed a bit from the ionic on-click you posted. What does the $element do exactly? I see that we use the element in other places in our code:

[shankari]

@sebastianbarry unfortunately, you didn't add a link to the stackoverflow so I had to search for it again 😄
and the last comment indicates that listening to a scroll event doesn't work outside a directive.
https://stackoverflow.com/a/48064098/4040267

But note that we are not using standard angularjs; we are using ionic and ionic already has a directive for the scroll controller. Again, we are now at the point where you can no longer rely on stackoverflow for answers, it is easiest to look at the source code.

The linked source code
https://github.com/ionic-team/ionic-v1/blob/eefba1331bc9206767c651912db71e9ed34850d5/js/angular/directive/scroll.js#L33
defines a custom directive with a bunch of parameters

You know (or should know from prior directives that you wrote) how to pass in functions to directives (e.g. how to pass in a function to the ng-click parameter of the button directive).

So have you tried the basic and obvious

<ion-content class="diary-entry" on-scroll="on-scroll-function-in-controller">

Note also the documentation for ion-content, which says the same thing
https://ionicframework.com/docs/v1/api/directive/ionContent/

[sebastianbarry]

The on-scroll works! After some changing and testing, this is what I was able to come up with for the dynamic date display while scrolling:

Screen.Recording.2022-11-03.at.12.28.57.PM.mov

The way I implemented this is a bit interesting, and may not be the best way of doing it. When the list of displayTrips (which is the direct list of trips that are displayed on the screen. This is calculated differently for each filter to-label``all unlabeled``all trips) is calculated, I also generate a list called tripsPosition which is a dictionary with the height and display_date for each trip in the displayTrips list. Then, every time the ion-content is scrolled, the function updateDisplayDateis called and it figures out based off of our current vertical scroll position, which trip we would be looking at based off of tripsPosition list, and then displays that display_date

$scope.updateScrollDisplayDate = function() {
    $scope.scrollPosition = $ionicScrollDelegate.getScrollPosition();
    let scrollpos = Math.floor($scope.scrollPosition.top);
    if(scrollpos < 0) {scrollpos = 0;}
    let heightOffset = 391;
    let ind = Math.floor(scrollpos/heightOffset);
    $scope.scrollDisplayDate = $scope.tripsPosition[ind].display_date;
  }

$scope.recomputeDisplayTrips = function() {
...
if ($scope.data.displayTrips[0]) {
      let heightOffset = 391;
      let height = 0;
      $scope.data.displayTrips.forEach((t) => {
        let display_date = t.display_date
        let thisTripPosition = { display_date, height }
        height += heightOffset;
        $scope.tripsPosition.push(thisTripPosition)
      });    
    }
}

issues

1. On iOS, when you have let go of the screen after scrolling, the scrolling continues for a little while longer before stopping. This whole time, the on-scroll funciton keeps getting recalled, until the scrolling stops, and then the last instance of the on-scroll function is the one that executes all the way through and completes. This results in a small delay between the time that you have let go of the scroll, and the time that the display-date title gets updated
2. In order to fit everything in the <ion-header-bar> with the addition of the new display-date title <div> in the header, I tried to condense all the to-label``all unlabeled and all trips buttons into one HTML <select> element like shown below, however when you select an option the ng-click attribute never gets activated. I need to find some sort of Angular select that will allow me to execute an Angular function when a select option is selected. @shankari sent me this for reference: https://docs.angularjs.org/api/ng/directive/select

<select class="row buttons labelfilterlist">
          <option ng-repeat="selF in filterInputs" ng-click="select(selF)" class="{{selF.width}} button labelfilter" ng-class="{on:selF.state}" style="text-align: center;font-size: 13px;font-weight: 600; border-radius: 10px;" translate>
            {{selF.text}}
          </option>
          <option ng-click="resetSelection()" class="col-33 button labelfilter last" ng-class="{on:allTrips}" style="text-align: center;font-size: 13px;font-weight: 600; border-radius: 10px;" translate>
            {{'.show-all'}}
          </option>
        </select>

[shankari]

@sebastianbarry how do you know that the heightOffset is exactly 391? Would that be true even if you had the replaced mode button? Have you actually tested with the replaced mode as requested earlier

[sebastianbarry]

I gathered the number 391px from the height of the trip elements in the HTML:

This value is hardcoded right now, which is something I will change before pushing out this PR. My ideal solution is to gather the heightOffset from the first element in the collection's "height" value:heightOffset=document.getElementByID("collectionRepeat[0]").getAttribute("style").substring("height:", 5) or something like that

---

Regarding testing the replaced mode difference for the new UI changes, I tested it this morning and here's what I found out:

Display 1	Display 2	Display 3

Looks like the Replaced mode button does not change the height of the trip item, instead it cuts off the bottom of the button but it is still displayed and clickable. This means that technically, this solution I wrote above would still work.

However, if we want to change the way the replaced-mode displays on trip item by extending the height of that specific object, we will also need to change the way we are gathering the height of the objects for this change. Or change the way we generate the display-date header altogether, so that it dynamically gets the elements that are currently shown on the screen and displays the topmost-object's display-date instead of using the semi-adjacent {display date, height} list that I'm currently using.

[shankari]

@sebastianbarry yes, the replaced mode shows up, but it is cut off and it looks ugly.
We need to actually change the height based on the replaced mode before we can even push out the first version of the app (improved label screen, no date selection).

[shankari]

Worked around by displaying the date in the trip