Maps tiles are sometimes only partially loaded on android

[shankari]

Sometimes, the app has this weird behavior on android in which the map tiles are only partially loaded. Reloading the data does not change anything - the issue is only fixed when the app is killed and reloaded.

This issue was recently re-reported by @immesys, so let's track the issue and try to fix it before we get out of beta.

[shankari]

Let's poke around a bit to see if we can find the map since that would be the next fix if resetting the geojson does not work.

[shankari]

ok so I think that I have a reproducible error case:

• go to label screen (Mon 25 Apr 5:23 De Anza to De Foe)
• open a map
• close the map
• go to diary
• values are grayed out
• reloading does not fix anything
• kill + reopen fixes the issue

opened from label	broken map

Note that not even the trajectories are correct any more

Yay! Reproducible on physical phone as well.

[shankari]

So one of the issues, and the reason why we can't look up the map, is because we use collection-repeat. This means that a bunch of items (including maps) are created ahead of time. This means that in the ui-leaflet module, all the keys are "map_". The ids change when we load the diary for a day, but the keys are not remapped since we don't watch on the id.

But I'm still not sure if this is related to the broken behavior. The popup map has a static ID (detailPopoverMap) so it shouldn't conflict with the map_ keys.

Let's see if this is due to the geojson...

[shankari]

Just before we show the detail, we set the geojson. And then we invalidate size. Let's start by commenting out the invalidate.

  $scope.showDetail = function($event, trip) {
    Timeline.confirmedTrip2Geojson(trip).then((tripgj) => {
        $scope.currgj = trip;
        $scope.currgj.data = tripgj;
        $scope.currgj.pointToLayer = DiaryHelper.pointFormat;
        $scope.tripDetailPopover.show();
        leafletData.getMap("detailPopoverMap").then(function(map) {
            map.invalidateSize();
        });
    });
  }

[shankari]

Removed the invalidateSize, did not fix the issue. Still broken if:

• there is a list in the diary view,
• we open the map in the label view
• go back to the diary view

If there is no list in the diary view, the maps are initialized correctly when going from label -> map -> diary. Once the diary maps are in "a bad state", they stay in a bad state as we move between days.

Just to clarify, this only happens after the maps are initialized. Concretely:

• go to diary screen
• maps all initialized with id = map_
• go to label
• open popup
• go back to diary screen
• select date with data, which sets the id = map_34343...
• go back to label screen
• open popup
• go back to diary screen

everything is broken.

This might just be related to the map_ id issue given that it occurs only after the IDs change.

[shankari]

Since it is not the invalidateSize it must be the geojson. I still don't see why the maps are linked. After a time-bounded investigation, we may want to use a different way of displaying the label map, similar to the detail screen, to avoid this issue.

[shankari]

Maybe related to this:

                    if(!_hasSetLeafletData){//only do this once and play with the same ref forever
                        _hasSetLeafletData = true;
                        leafletData.setGeoJSON(leafletGeoJSON, attrs.id);
                    }

[shankari]

Couple of fixes for the map_ issue:

• watch for attr.id as well and change the mapping in leafletData
• make the ID depend on the index instead of the trip information

Given that the id seems to be a foundation of leafletData, the second option seems like the safer option.

[shankari]

making the id depend on the index still doesn't solve it because the items don't have an index until they are filled in.

Blank screen

Screen	Map

Loaded screen

Screen	Map

[shankari]

Switched to ng-repeat to see if that helps.

Blank list

Hit leaflet directive breakpoints with map_0 through map_8 Hit geojson directive breakpoints with map_0 through map_8

Populated list

• Switch to label view
• Click on popup
• Go back to list, everything is broken

So it is not the map_x mapping.

Going to give up for now and try to use something similar to the detail screen instead of a popup. We might end up using images in the future anyway... This also makes it closer to the list view prior to unifying them.

[shankari]

I think I didn't do that before because the detail screen looked up the tripgj from the cached list, and we weren't caching the trip objects from the infinite scroll list. But after unifying the code, tracked in https://github.com/e-mission/e-mission-docs/issues/674, fixed in https://github.com/e-mission/e-mission-phone/pull/799, this should be simpler?

[shankari]

ok, so there is a tripMap and a tripWrapperMap

      timeline.getTrip = function(tripId) {
        return angular.isDefined(timeline.data.tripMap)? timeline.data.tripMap[tripId] : undefined;
      };

      timeline.getTripWrapper = function(tripId) {
        return angular.isDefined(timeline.data.tripWrapperMap)? timeline.data.tripWrapperMap[tripId] : undefined;
      };

trip map is filled in for

       var processTripsForDay = function(day, tripListForDay) {
        console.log("About to show 'Processing trips'");
        $ionicLoading.show({
          template: 'Processing trips...'
        });
        tripListForDay.forEach(function(item, index, array) {
          console.log(index + ":" + item.properties.start_fmt_time+", "+item.properties.duration);

        });
        timeline.data.currDay = day;
        timeline.data.currDayTrips = tripListForDay;

        timeline.data.tripMap = {};

        timeline.data.currDayTrips.forEach(function(trip, index, array) {
          timeline.data.tripMap[trip.id] = trip;
        });

In the list view, we convert the trips to trip wrappers and store them as well.

          var currDayTripWrappers = Timeline.data.currDayTrips.map(
            DiaryHelper.directiveForTrip);
          Timeline.setTripWrappers(currDayTripWrappers);

However, we never set it from the infinite scroll list

[shankari]

Options are:

• create a map for the infinite scroll list as well
• pass in the trip object directly

Let's start with the second option since:

• it is easier
• we don't actually have the trip id in the trip object since we get out the data (ctList.phone_data.map((ct) => ct.data))

[shankari]

we can't pass in the trip directly because the params are passed in as strings.

$stateParams
{tripId: '[object Object]'}
   tripId: "[object Object]"
   [[Prototype]]: Object

Falling back to the first option

[shankari]

The first option was fairly complex, but works! Looks like the popup was the key to messing up the other maps, although I don't quite understand why.

[shankari]

Experimented with the ability to label directly from the detail screens (commits will show up soon). Thanks to the prior modularization of the multi-label UI, this was fairly straightforward. HOWEVER, doing so brings back our bad-maps-in-list problem

Even if we set the values in the label detail, which uses a completely different list than the list view (see video below). And it happens as soon as we set any of the labels, even if we don't set both. No time to dig deeper into this; removing the labeling functionality for now pending a restructuring/unification later.

https://user-images.githubusercontent.com/2423263/166113481-50832f2f-d693-453b-97a4-7c4cca44b52e.mov

[shankari]

Argh! Terrible news! Even if I set the values from the infinite list, the map is reset. Maybe we have to switch to images or something...

https://user-images.githubusercontent.com/2423263/166125979-e5ce15ec-71c2-43c9-8af2-501f6a428e9b.mov

[shankari]

ok, so after adding breakpoints in the leaflet code directly, I can see the issue. The problem is that opening the popover (even the label popover) triggers a resize event. And leaflet calls invalidateSize on reset

    _onResize: function () {
        cancelAnimFrame(this._resizeRequest);
        this._resizeRequest = requestAnimFrame(
                function () { this.invalidateSize({debounceMoveend: true}); }, this);
    },

invalidateSize tries to compute the old size and the new size. All of this works as long as the map is visible.

However, if the map is not visible (because it is in a hidden tab), it has a new size of zero. Which means that we end up moving the map view, and breaking the list map.

        var newSize = this.getSize(),
            oldCenter = oldSize.divideBy(2).round(),
            newCenter = newSize.divideBy(2).round(),
            offset = oldCenter.subtract(newCenter);

        if (!offset.x && !offset.y) { return this; }

        if (options.animate && options.pan) {
            this.panBy(offset);

        } else {
            if (options.pan) {
                this._rawPanBy(offset);
            }

One simple hack might be to ignore invalidateSize if the new size is 0.

[shankari]

Reproduction case:

• Load diary view
• Maps are visible
• trigger "resize"; maps stay valid
• Load label view
• Maps are hidden
• trigger "resize"
• go back to list view; maps become invalid

https://user-images.githubusercontent.com/2423263/166132288-1637cd3e-2b37-49fb-bd95-935095751917.mp4

[shankari]

Hacked around this by editing the leaflet src

                    oldCenter = oldSize.divideBy(2).round(),
                    newCenter = newSize.divideBy(2).round(),
                    offset = oldCenter.subtract(newCenter);
+
+        // Hack added by shankari to fix
+        // https://github.com/e-mission/e-mission-docs/issues/111#issuecomment-1114128934
+        if (newSize.x == 0 && newSize.y == 0) { return this; }

                if (!offset.x && !offset.y) { return this; }

That seems to work.

[shankari]

Tracking the longer-term fix in https://github.com/e-mission/e-mission-docs/issues/726

[shankari]

Still broken for the following use case:

• load diary
• go to label
• click popup
• click elsewhere to close popup without any changes
• go back to diary (all looks good)
• reload page (all is broken)

https://user-images.githubusercontent.com/2423263/166133933-71d9881c-bb54-4103-ad5f-3a92eb047328.mp4

[shankari]

As I guessed, this is because the old zero size is cached somewhere and messes up the reload. It is cached because invalidateSize calls getSize to get the new size, and getSize caches the size

    getSize: function () {^M
        if (!this._size || this._sizeChanged) {^M
            this._size = new Point(^M
                this._container.clientWidth || 0,^M
                this._container.clientHeight || 0);^M
^M
            this._sizeChanged = false;^M
        }^M
        return this._size.clone();^M
    },^M

Changing the hack to read the container sizes directly without calling getSize works

        // Hack added by shankari to fix
        // https://github.com/e-mission/e-mission-docs/issues/111#issuecomment-1114128934
        // console.log("LEAFLET: clientWidth = "+this._container.clientWidth+" clientHeight = "+this._container.clientHeight + " combined: "+ (this._container.clientWidth && this._container._clientHeight));
        if ((this._container.clientWidth || 0) == 0 && (this._container.clientHeight || 0) == 0) { return this; }

[shankari]

Finally, we ensure that labels set in the diary screen are reflected in the label screen and vice versa. This ensures that, no matter which screen the user chooses, they see a consistent view of their trip information.

At this point, with https://github.com/e-mission/e-mission-phone/pull/823/commits/c302d65904662253214dce9c5ea966060b7e9a65, we can set values from:

• diary list
• diary detail
• label list
• label detail

and it will be reflected in all those areas without any further user input.

https://user-images.githubusercontent.com/2423263/166184862-3c4db6b1-f2b1-414f-901e-b9bdf4fc9839.mp4

[shankari]

Closing this issue for now Tracking the longer-term fix in conjunction with the leaflet maintainers at https://github.com/e-mission/e-mission-docs/issues/726