HandsOnDataViz/leaflet-storymaps-with-google-sheets

Scroll listener is initialized multiple times (?)

slead opened this issue · 3 comments

slead commented

Correct me if I'm wrong, but it appears that because the scroll listener is located inside the onEachFeature function, the listener is called once for every feature in the featureCollection. This means that the flyTo function is called repeatedly, which could have performance issues.

onEachFeature: function (feature, layer) {
  (function(layer, properties) {
    ...
    $('div#contents').scroll(function() {
      ...
      map.flyTo(....

I don't yet have an alternative approach, I just thought I'd raise the issue in case you're already aware of it. I'm also aware that you've parked this project for the meanwhile.

Cheers,
Steve

Thanks @slead - I'll discuss this with @ilyankou

Hey @slead, yes indeed it could have been done more efficiently – for example, by creating an array of positions of each chapter relative to the parent <div> (you would also need to store zoom level + coordinates for each chapter). Then you only need one event listener that would wait for scrolling to happen and figure from the array where to fly. On the other hand, since the storymap is usually limited by a few dozen chapters (at most) and thus a few dozen event listeners, I don't think it will affect performance much (look at the crazy 3D stuff modern browsers are capable of rendering!).

Thanks a lot for the suggestions – we will be rebuilding it on Node.js in a few months (hopefully) and will use more cool libraries, including Handlebars for better readability.

slead commented

Thanks guys. I've tied each Chapter to a list of layers which should be switched on when the map zooms to the region, and I noticed that my function was being called repeatedly, hence this question.

I look forward to seeing the new version!