e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 32 forks source link

Regression in dashboard #875

Closed shankari closed 1 year ago

shankari commented 1 year ago

Reported by @TTAlex

Error: undefined is not an object (evaluating 'num.toFixed')
humanize@ionic://localhost/js/metrics.js:1437:31
link@ionic://localhost/js/metrics.js:1444:36
invokeLinkFn@ionic://localhost/lib/ionic/js/ionic.bundle.js:22993:15
nodeLinkFn@ionic://localhost/lib/ionic/js/ionic.bundle.js:22392:23
@ionic://localhost/lib/ionic/js/ionic.bundle.js:22732:36
processQueue@ionic://localhost/lib/ionic/js/ionic.bundle.js:29127:30
@ionic://localhost/lib/ionic/js/ionic.bundle.js:29143:39
$digest@ionic://localhost/lib/ionic/js/ionic.bundle.js:30211:36
@ionic://localhost/lib/ionic/js/ionic.bundle.js:30434:33
completeOutstandingRequest@ionic://localhost/lib/ionic/js/ionic.bundle.js:19194:15
@ionic://localhost/lib/ionic/js/ionic.bundle.js:19470:33 – "<rangedisplay range=\"caloriesData.aggrCalories\">"

And similar for carbon

shankari commented 1 year ago

The related code is

        link: function(scope) {
            // console.log("RANGE DISPLAY "+JSON.stringify(scope.range));
            var humanize = function(num) {
                if (Math.abs(num) < 1) {
                    return num.toFixed(2);
                } else {
                    return num.toFixed(0);
                }
            }

            if (Math.abs(scope.range.high - scope.range.low) < 1) {
                scope.tinyDiff = true;
            }
            scope.lowFmt = humanize(scope.range.low);
            scope.highFmt = humanize(scope.range.high);
        },
        templateUrl: "templates/metrics/range-display.html"

So clearly the scopes are set to undefined instead of whatever they were before. I bet this is also related to the dynamic config changes.

shankari commented 1 year ago

Looking only at errors, we also see this

[Error] Unhandled Promise Rejection: TypeError: this.init is not a function. (In 'this.init()', 'this.init' is undefined)
    (anonymous function) (metrics-mappings.js:394)
    promiseReactionJob

The related code is

    $ionicPlatform.ready().then(function() {
      DynamicConfig.configReady().then((newConfig) =>
        this.init()
      );
    });
shankari commented 1 year ago

ok so the reason for this failure is because this is a callback from $ionicPlatform.ready(), so within the callback, this is the window, not the service.

Screenshot 2023-03-29 at 9 59 36 AM

So there is no this.init

Screenshot 2023-03-29 at 10 00 07 AM

Let's try converting to a factory and see if that fixes things.

shankari commented 1 year ago

The dashboard now works fine with the multilabel screen, but still has some errors with the survey option.

First, we get

Error loading aggregate data, averages not availableundefined is not an object (evaluating 'currentMETs[mode]')
@ionic://localhost/js/metrics-factory.js:195:21
@ionic://localhost/js/metrics.js:780:40
@ionic://localhost/js/metrics.js:669:34
@ionic://localhost/js/metrics.js:569:37
promiseReactionJob@[native code]

followed by

Error: undefined is not an object (evaluating 'num.toFixed')
humanize@ionic://localhost/js/metrics.js:1437:31
link@ionic://localhost/js/metrics.js:1444:36
invokeLinkFn@ionic://localhost/lib/ionic/js/ionic.bundle.js:22993:15
nodeLinkFn@ionic://localhost/lib/ionic/js/ionic.bundle.js:22392:23
@ionic://localhost/lib/ionic/js/ionic.bundle.js:22732:36
processQueue@ionic://localhost/lib/ionic/js/ionic.bundle.js:29127:30
@ionic://localhost/lib/ionic/js/ionic.bundle.js:29143:39
$digest@ionic://localhost/lib/ionic/js/ionic.bundle.js:30211:36
$apply@ionic://localhost/lib/ionic/js/ionic.bundle.js:30503:31
done@ionic://localhost/lib/ionic/js/ionic.bundle.js:24824:53
completeRequest@ionic://localhost/lib/ionic/js/ionic.bundle.js:25022:15
requestLoaded@ionic://localhost/lib/ionic/js/ionic.bundle.js:24963:24 – "<rangedisplay range=\"carbonData.aggrCarbon\">"

The second is probably related to the first, since we don't have any MET or carbon mappings.

We could try to catch these exceptions and ignore them, but I am not convinced that is the correct solution, since the dashboard as currently implemented is not meaningful for the case where the trip details are using a survey. The survey can be generic, so we don't know how to pull out the modes, and how to ensure that we have energy intensity mappings for all of them.

The correct solution is to not initialize the dashboard at all.

Let's implement this generically by turning off the dashboard if the survey type is ENKETO. We will also turn off the diary screen if there is a time use component since we don't currently support time use in the diary.

Later, we can make this more general by turning screens on/off in the dynamic config. Still not sure if we want to make that be explicit or implicit based on other parts of the config (like ENKETO vs. MULTILABEL)

shankari commented 1 year ago

I am not sure how testing the dashboard and diary screens ever worked on the first test, because I found multiple minor errors in subsequent testing while looking at the logs. https://github.com/e-mission/e-mission-phone/pull/940

After I finally got it to work, though, I see that, because of the ng-if, even when the diary and dashboard screens are supposed to be shown (as in the MULTILABEL case), they are shown out of order. The order is now (label, profile, diary, dashboard), which looks wrong.

Screenshot 2023-03-30 at 8 32 24 AM
shankari commented 1 year ago

Looking at the ionic documentation, we should be able to use the hidden or disabled attributes instead of ng-if, but unfortunately, none of them actually seem to do anything.

MAIN-SCREENS: showDiary = false metrics = false setting tabSel to done
Screenshot 2023-03-30 at 8 14 14 AM

https://user-images.githubusercontent.com/2423263/228884315-0a450fea-a42e-4562-95dd-89b464b13edf.mov

shankari commented 1 year ago

Let's see if we can use jQuery to reorder the tabs. At the jquery level, the tabs are in order

E (4) = $2
0 
<ion-tab id="diary" ng-if="showDiary" icon="ion-map" href="#/root/main/diary" title="Diary"></ion-tab>
1 
<ion-tab icon="ion-checkmark-round" href="#/root/main/inf_scroll" title="Label"></ion-tab>
2 
<ion-tab id="metrics" ng-if="showMetrics" icon="ion-ios-analytics" href="#/root/main/metrics" title="Dashboard"></ion-tab>
3 
<ion-tab icon="ion-person" href="#/root/main/control" title="Profile"></ion-tab>

E Prototype
shankari commented 1 year ago

ok, so the HTML elements are not ion-tab, they are actually $('.tab-item'), which are in this order

E (4) = $6
0 
<a ng-class="{'has-badge':badge, 'tab-hidden':isHidden(), 'tab-item-active': isTabActive()}" ng-disabled="disabled()" class="tab-item tab-item-active" icon="ion-checkmark-round" title="Label">…</a>
1 
<a ng-class="{'has-badge':badge, 'tab-hidden':isHidden(), 'tab-item-active': isTabActive()}" ng-disabled="disabled()" class="tab-item" icon="ion-person" title="Profile">…</a>
2 
<a ng-class="{'has-badge':badge, 'tab-hidden':isHidden(), 'tab-item-active': isTabActive()}" ng-disabled="disabled()" class="tab-item" icon="ion-map" title="Diary">…</a>
3 
<a ng-class="{'has-badge':badge, 'tab-hidden':isHidden(), 'tab-item-active': isTabActive()}" ng-disabled="disabled()" class="tab-item" icon="ion-ios-analytics" title="Dashboard">…</a>

E Prototype
shankari commented 1 year ago

Aha! while initializing the controller, the digest cycle has not yet run, so we still have only two tabs (the other two are ng-ifed out.

[Log] JQUERY: in dynamic config load, tabs list is  – E [<a class="tab-item">, <a class="tab-item">] (2) (cordova.js, line 1413)
E [<a class="tab-item">, <a class="tab-item">]E (2)0 <a ng-class="{'has-badge':badge, 'tab-hidden':isHidden(), 'tab-item-active': isTabActive()}" ng-disabled="disabled()" class="tab-item tab-item-active" icon="ion-checkmark-round" title="Label" style>…</a>1 <a ng-class="{'has-badge':badge, 'tab-hidden':isHidden(), 'tab-item-active': isTabActive()}" ng-disabled="disabled()" class="tab-item" icon="ion-person" title="Profile">…</a>E Prototype

But in the afterEnter callback, we do see all 4 tabs. So we can move them around using jQuery.

[Log] JQUERY: after view enter, tabs list is  – E [<a class="tab-item tab-item-active">, <a class="tab-item">, <a class="tab-item">, …] (4) (cordova.js, line 1413)
E [<a class="tab-item tab-item-active">, <a class="tab-item">, <a class="tab-item">, <a class="tab-item">]E (4)

I now see this again

Screenshot 2023-03-30 at 10 29 58 AM
shankari commented 1 year ago

Fixed in https://github.com/e-mission/e-mission-phone/pull/938 and https://github.com/e-mission/e-mission-phone/pull/941