Fix blank screen issue when scrolling on trip list

[jiji14]

Issue

A blank screen appears when scrolling on the trip list.
https://github.com/e-mission/e-mission-phone/assets/47590587/6949486f-0237-42e8-99e4-83efacd05348

Flashlist performance optimization

Upon reviewing the FlashList issue pages, I attempted to find solutions, but none seemed to address the problem effectively.
Here are the details:
Issue 1
Issue 2
Issue 3
Issue 4

Next, I consulted the documentation shared by Jack regarding FlashList performance optimization:

getItemType ✅

This prop is useful when FlashList has different types of cell components that are vastly different.

estimatedItemSize

we are already using this prop.

memo

Since there is no repeated calculation in our case, using memo won't fix this issue.

Flashlist configuration

Additionally, there are configurations for determining whether items are viewable. I tried these configurations:

viewAreaCoveragePercentThreshold ✅

Percent of viewport that must be covered for a partially occluded item to count as "viewable", 0-100. Fully visible items are always considered viewable. A value of 0 means that a single pixel in the viewport makes the item viewable, and a value of 100 means that an item must be either entirely visible or cover the entire viewport to count as viewable.

itemVisiblePercentThreshold ✅

Similar to viewAreaCoveragePercentThreshold, but considers the percent of the item that is visible, rather than the fraction of the viewable area it covers.

waitForInteraction ✅

If it is true, nothing is considered viewable until the user scrolls or recordInteraction is called after render.

Referacne

Leaflet rendring

When rendering without Leaflet, the blank screen issue did not occur.
https://github.com/e-mission/e-mission-phone/assets/47590587/159c21ff-696d-43e3-b391-0694b8a880b1

When rendering a Leaflet map without any additional calculations (= calling the initMap function), the performance improved but a blank screen still appears.
https://github.com/e-mission/e-mission-phone/assets/47590587/a435cf63-1ac7-4f69-99e1-c6bf1284cb9f

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.37%. Comparing base (034bee2) to head (8b5c6f6).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1132   +/-   ##
=======================================
  Coverage   78.37%   78.37%           
=======================================
  Files          28       28           
  Lines        1702     1702           
  Branches      366      366           
=======================================
  Hits         1334     1334           
  Misses        368      368           

Flag	Coverage Δ
unit	78.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[jiji14]

After using getItemType and configuration, the scrolling performance seemed to improve, but it didn't fully resolve the issue. I then tried to optimize the Leaflet Map rendering. I checked for any unnecessary rendering, but found none. Additionally, even with just basic Map rendering without any calculations (i.e., tileLayer, pointToLayer), the black screen issue still exists.

Should we try this updated codes on staging app first?

[JGreenlee]

@jiji14 Great investigating process! I think we can utilize the FlashList optimizations, even if they only help a little.

As for the main issue:

You noted that the problem still exists when you removed the call to initMap(). I am thinking this means it isn't the graphics rendering that's the problem.
I tried this myself by commenting out line 43 and confirmed what you found:

e-mission-phone/www/js/components/LeafletView.tsx

Lines 35 to 44 in 034bee2

	useEffect(() => {
	// if a Leaflet map already exists (because we are re-rendering), remove it before creating a new one
	if (leafletMapRef.current) {
	leafletMapRef.current.remove();
	mapSet.delete(leafletMapRef.current);
	}
	if (!mapElRef.current) return;
	const map = L.map(mapElRef.current, opts || {});
	initMap(map);
	}, [geojson]);

Next I tried also removing the call to L.map() by commenting out both line 42 and line 43.
When I did this, the scrolling was perfectly smooth.

So, the slowness isn't from initializing the maps; it's from creating them in the first place. I think this is because the map has to be re-anchored to a DOM element when it comes back into view because FlashList is recycling the DOM elements.

[jiji14]

@JGreenlee

the slowness isn't from initializing the maps; it's from creating them in the first place. - Yes, that's what I wanted to say! haha Thanks for clarification.

[JGreenlee]

I think this is because the map has to be re-anchored to a DOM element when it comes back into view because FlashList is recycling the DOM elements.

If this is true, I don't see a good way around it.
Leaflet works by attaching itself to a fixed DOM element. FlashList works by dynamically cycling through DOM elements.

These 2 libraries have differing strategies that don't mix very well...

[jiji14]

If we want to fully resolve the blank space issue, I think we should either 1. use a different Map library or 2. use a different ListView. However, I also tried FlatList, and it had the same issue.

[JGreenlee]

I think it could be worth considering having these maps be static images that are generated on the server.

These maps aren't interactable. You can't zoom or pan around on them. Functionally, their purpose is just to give a snapshot, or "preview" of a trip to aid in recall. They could serve this same purpose if they were static images.

Once you click on the trip we would still have the interactable Leaflet map you can zoom + pan. On that screen, there is only one map shown at a time and there is no performance issue.

[JGreenlee]

The main obstacle to this approach would be the color-coded trajectories depending on mode.

And when the trip is labeled, changing the trajectory color to correspond to the labeled mode. This is the only dynamic aspect of those maps.

[JGreenlee]

If each trip had a static image of its map preview and we knew the coordinates of the corners of the image, we would still be able to draw lines over that image using SVGs and coordinates of the trajectories.

We would not need leaflet to do that. But it feels a bit like 're-inventing the wheel'.

[jiji14]

@shankari Please share your thought on this issue

[JGreenlee]

A different performance optimization we could try might be something like this:

• Use Leaflet to create and generate the maps
• Once all the tiles have loaded for a particular map, serialize the DOM element for that map as an HTML string and cache it in a dictionary
• If there exists a cached HTML string for that trip, we can unmount/destroy the Leaflet instance and just render the HTML string (which should be static now because it was serialized)

[JGreenlee]

We would still incur the upfront performance cost of creating the Leaflet maps. But once they're serialized + unmounted, I think it would be smoother to scroll back through already-rendered maps

[jiji14]

A different performance optimization we could try might be something like this:

• Use Leaflet to create and generate the maps
• Once all the tiles have loaded for a particular map, serialize the DOM element for that map as an HTML string and cache it in a dictionary
• If there exists a cached HTML string for that trip, we can unmount/destroy the Leaflet instance and just render the HTML string (which should be static now because it was serialized)

I will try that! Thanks :)

[jiji14]

@JGreenlee
I updated it to store the HTML string after the first map rendering and then parse the string when the map is stored in the cache.

[JGreenlee]

I see! That looks like a good implementation of what I had in mind.

Did it improve performance in your testing? Unfortunately as far as I've tested it, it doesn't seem to help that much.

[JGreenlee]

After a lot of investigating, I think another significant performance bottleneck is the reversed property.

I remember this from when I first added FlatList / FlashList. The reversed property inverts the list in a sort of hacky way: it uses a CSS transform of scale(-1) on the entire list to flip the order, and also applies the same transformation to each of the list's items so they appear upright again.
That means all the content is constantly being flipped twice and makes FlatList perform worse when inverted. (A bunch of people on react-native repo have complained about this being a bad implementation but no one has stepped up to fix it yet.)

Edit: I meant to say inverted, not reversed https://reactnative.dev/docs/virtualizedlist#inverted

[jiji14]

@JGreenlee

Caching seems to have improved scrolling performance slightly, but not significantly. After deleting the inverted property, I can see a significant improvement. Let me also do some research about this property!

[jiji14]

Checking if I can reverse the trip cards without using the inverted property.

[shankari]

@jiji14 @JGreenlee

Next I tried also removing the call to L.map() by commenting out both line 42 and line 43.
When I did this, the scrolling was perfectly smooth.

given this, is it possible to just lazily load the leaflet data? effectively mark the list item as "loaded" before the map is created. The list will then have some information, including start and end times, and addresses. We can then create the maps in a promise and "fill it in". And if we are scrolling too fast for the promise to complete, we can just cancel the promise.

Does that work?

[JGreenlee]

It's not that the maps are too slow to load or render; it's that after they are loaded and rendered, they are too heavy for the browser to keep up graphically / repainting while scrolling

And, the context in which the maps are rendered (inverted list via CSS transforms) contributes to the graphical strain

[JGreenlee]

I used the Profiler in React devtools to see if some components were taking too long to render in React.
I found that scrolling is choppy even when none of the components are being re-rendered.

[jiji14]

Update:
Couldn't find any workaround to reverse the list on FlatList or FlashList without using transform. Checking concurrent rendering now: React v18.

[jiji14]

[Summary]

The scroll issue was triggered during painting, not rendering. Previously, we used the inverted property to display trip cards in reverse order. The inverted property worked by reversing the parent component using the transform CSS property and also reversing the children components using the same property. This process took time and caused a blank screen to appear on older iPhones.

Since this issue was not caused by rendering, rendering optimization solutions such as adding properties for Flashlist optimization and optimizing the initial rendering of the map did not resolve the issue. Therefore, we decided to stop using the inverted property to display reversed trip cards. Instead, Jack discovered that we can reverse the trip cards by using flexDirection: 'column-reverse'.

Here is the screen recording:
https://github.com/e-mission/e-mission-phone/assets/47590587/a3dcc8f0-78b2-4dc7-8296-8044212ab271

@JGreenlee , please add more if I missed something!

[shankari]

I actually see three changes related to performance:

• Caching the rendered HTML in the Leaflet cache
• Using lower res tiles on the list view
• dangerously set innerHTML
• The new fkexDirection property

And a couple of non-performance related changes to the annotation.
All looks good to me and I am happy to merge to get this onto staging...