[Bug Fix] Leaflet gets broken after updating labels

[jiji14]

Overview

After optimizing the Leaflet map, a bug appears when updating labels.
We cache the map after rendering a Leaflet map and reset the leafletMapRef. However, when the map information changes, we need to re-render the map using initMap. Currently, this doesn't happen because we reset leafletMapRef, causing it to fail the conditional check.

Before

After

Screen.Recording.2024-03-19.at.1.02.00.PM.mov

[jiji14]

@JGreenlee @shankari Please check this PR 🙏

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.49%. Comparing base (5812eee) to head (a5a8367).
Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1137   +/-   ##
=======================================
  Coverage   77.49%   77.49%           
=======================================
  Files          29       29           
  Lines        1693     1693           
  Branches      370      370           
=======================================
  Hits         1312     1312           
  Misses        381      381           

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

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

[shankari]

LGTM! Once Jack reviews, I will merge

[shankari]

@JGreenlee tagging you for review

[JGreenlee]

It works well and looks good to me.

I tested this locally in the devapp because I wanted to make sure the color of the trajectory updates if the mode changes from one base mode to another (for example, bike -> car changes from green to red)

[shankari]

@JGreenlee @jiji14 just to check, why was this not caught by the unit tests? It is a 100% reproducible bug for a fairly common function. Do we not have a test that pretends to click the button and label the trip?

[JGreenlee]

@shankari I think we would have needed integration testing or end-to-end testing for this to have been caught. We started with integrating Appium, but it is a significant effort. After encountering roadblocks with setup, we have not been able to prioritize it since there have been more urgent tasks lately

I have been loosely formulating a plan which I think would make integration testing more easily achievable, which is to support a version of the UI that runs as an Expo web app, detached from the native code.
Essentially, when Cordova is not available it would use a web compatibility layer for things the UI needs like usercache and servercomm. There would be no tracking; it would only display data that was already synced from another device.

Because it would be web-only, we could test the full extent of its capability, including integration testing with actual server calls.
And there's a couple bonus benefits, i) it will make it easier to transition to Expo, and ii) we will have a web version lying around if someone ever asks for it

[shankari]

the web compatibility layer is an interesting idea and definitely the standard approach towards software based testing (e.g. creating mock/proxies/hardware abstraction layer). It would be interesting to see how thick the layer would need to be. If we are not testing the data collection, maybe only the usercache and the server calls. Would be great to have the web version "in our back pockets" in case there is a request for it.