perfectly-preserved-pie / larentals

An interactive map of for-sale & rental property listings in Los Angeles County, updated weekly.
https://wheretolive.la
15 stars 3 forks source link

Popup closes when map is panned or zoomed #86

Closed perfectly-preserved-pie closed 1 year ago

perfectly-preserved-pie commented 1 year ago

Since using dl.GeoJSON, if a popup is open and the map is panned or zoomed even a tiny bit, the popup closes. Clicking on a marker also automatically pans to fit the popup inside the screen, but that panning action means the popup closes, so it takes anywhere from 2-4 clicks to get the popup to stay open. Wack behavior.

anaypat5 commented 1 year ago

Hey, I've been working on a dash leaflet project recently, and I ran into a somewhat similar issue, where I lost some functionality of Tooltips and Popups when moving from dl.Markers and dl.Polylines to a dl.GeoJSON, and referencing this solved my issue. It seems like you could do something similar, where instead of looking at the properties of the feature, you check if the feature is a point, and if so, create a marker with the latitude and longitude thats already passed in, and then you can specify the autoPan option there. My JavaScript knowledge is also pretty limited, but from how I understand it, adding onto the reference something along the lines of:

if (feature.geometry.type == 'Point') {
               var lat = feature.geometry.coordinates[1]
               var lon = feature.geometry.coordinates[0]
               layer = L.Marker([lon, lat], { autoPan: false })

Unfortunately, I am not able to test if this works, but I would look at all other issues regarding people creating custom onEachFeature functions and how they are implemented, as I think that is the way to solve this.

Edit: Looking a little deeper, I think the assign function might be easier. You can look at the example on the dash leaflet docs with custom geojson icons here, just remove the L.icon creation, and just do return L.Marker(latlng, {autoPan: false});

perfectly-preserved-pie commented 1 year ago

@anaypat5 hey, thanks for that sweet piece of info, I really appreciate it.

I'm close, but setting the autoPan property via assign errors out. If I set the title property, the title does show up, indicating that the assign function is working correctly:

yo = assign("""function(feature, latlng){
  return L.marker(latlng, {title: 'THIS IS A CUSTOM TITLE'});
  }""")

...

return dl.GeoJSON(
    id=str(uuid.uuid4()),
    data=geojson,
    cluster=True,
    zoomToBoundsOnClick=True,
    superClusterOptions={ # https://github.com/mapbox/supercluster#options
      'radius': 160,
      'minZoom': 3,
    },
    options=dict(pointToLayer=yo),

autoPan isn't a property of L.marker but rather L.popup so I used that instead: But I try to set the popup's autoPan property to False:

yo = assign("""
  function(feature, latlng){
    return L.popup(
      latlng,
      {autoPan: false}
    );
  }""")

I get a lot of No match for [dashExtensions.default.function63] in the global window object. errors in the Dash console.

Do you have a working code snippet from your project you'd be willing to share?

anaypat5 commented 1 year ago

Unfortunately I don't have access to my work computer at the moment so I can't share my code, but if it actually is a Popup property then just directly using the reference code like so should work:

window.dash_props = Object.assign({}, window.dash_props, {
    module: {
        on_each_feature: function(feature, layer, context) {
            if (!feature.properties) {
                return
            }
            if (feature.properties.popup) {
                layer.bindPopup(feature.properties.popup, {autoPan: false})
            }
            if (feature.properties.tooltip) {
                // here you can change all leaflet tooltip options
                layer.bindTooltip(feature.properties.tooltip)
            }
        }
    }
});

You have to save this as a .js file (I saved mine as window.js but I'm not sure if the file name matters) and put it in an 'assets' folder where your code is, and then you can implement it in your code using

from dash_extensions.javascript import Namespace
...
ns = Namespace("dash_props", "module")
dl.GeoJSON(..., options=dict(onEachFeature=ns("on_each_feature")))
perfectly-preserved-pie commented 1 year ago

Holy shit that worked!!! Thank you so much! I was even able to resize the popup without resorting to using CSS (which made the popup and the marker mis-aligned)

    module: {
        on_each_feature: function(feature, layer, context) {
            if (!feature.properties) {
                return
            }
            if (feature.properties.popup) {
                layer.bindPopup(feature.properties.popup, {
                    // Here you can customize the popup
                    // https://leafletjs.com/reference.html#popup-option
                    autoPan: false,
                    closeButton: false,
                    // Set the maxHeight to 500px if the device is mobile
                    maxHeight: window.innerWidth < 768 ? 500 : null,
                    // Set the maxWidth to 175px if the device is mobile
                    maxWidth: window.innerWidth < 768 ? 175 : null,

                })
            }
        }
    }
});

This solves the autopanning issue, which stops the popup from closing when a marker is clicked. Still, that's more of a workaround for my ultimate issue: Do you know if it's possible to stop the popup from closing whenever the map is zoomed or panned? That would completely solve my issue. That behavior wasn't present with dl.Marker when I used it.

anaypat5 commented 1 year ago

Maybe you can try setting closePopupOnClick=False in your dl.Map, and/or adding closeOnClick: true to the popup options you defined in the javascript function. The dash leaflet docs say that closeOnClick defaults to what the map property is set as so just the former should work.

perfectly-preserved-pie commented 1 year ago

Unfortunately that only modifies the behavior when the map is clicked but not when it's panned/moved :(

anaypat5 commented 1 year ago

Ah, I was hoping that Leaflet would consider panning as a "click & drag" and it would just work out. I'm currently trying to get a version of your source code working so that I can try my own edits, but in the meantime, I just realized that you said autoPan was a L.popup property that you tried to set on L.marker. Maybe you could try binding the popup to the marker as they would do in Leaflet, and then setting the options on the marker, such as the following:

yo = assign("""
  function(feature, latlng){
    return L.popup(latlng).bindPopup(..., {autoPan: false, ...});
  }""")

To be honest, if this doesn't error out, I'm not sure how different this would be from what you're already doing with onEachFeature, but maybe something under the hood changes here to make this work lol. Although with this I'm not sure if this would still keep the content you currently have set in the popup, but its maybe worth a shot? My Dash Leaflet experience is coming up on like two weeks now haha, so if this doesn't work as intended I'd have to do some more research into it unfortunately. I do not remember if I have the same popup behavior in my code, so I'll be able to look into it in more detail on Monday.

anaypat5 commented 1 year ago

Hey, so I took a look at my code and I saw that my popups don't close when the map is panned or zoomed, and the properties I have set in my Javascript function for the popups is autoClose: false and closeOnClick: false, so try setting both of those in your Javascript function and hopefully that will work.

perfectly-preserved-pie commented 1 year ago

I tried that but it didn't change anything. I think the difference might be how we're generating our markers. How are you generating yours? I'm using dl.GeoJSON inside a Dash callback like this

@callback(
  Output(component_id='lease_geojson', component_property='children'),
  [ <a bunch of inputs>
  ]
...

# Create an empty list for the markers
  markers = []
  # Iterate through the dataframe, create a marker for each row, and append it to the list
  for row in df_filtered.itertuples():
    markers.append(
      dict(
        lat=row.Latitude,
        lon=row.Longitude,
        popup=row.popup_html
        )
    )
  # Generate geojson with a marker for each listing
  geojson = dlx.dicts_to_geojson([{**m} for m in markers])

...

ns = Namespace("dash_props", "module")
  # Generate the map
  return dl.GeoJSON(
    id=str(uuid.uuid4()),
    #children=[dl.Popup(id='2popup')],
    data=geojson,
    cluster=True,
    zoomToBoundsOnClick=True,
    superClusterOptions={ # https://github.com/mapbox/supercluster#options
      'radius': 160,
      'minZoom': 3,
    },
    options=dict(onEachFeature=ns("on_each_feature"))
  )
anaypat5 commented 1 year ago

I am not creating my GeoJSONs in a callback, but it seems that the way we make them are pretty similar. The way I do it is:

ns = Namespace('dash_props', 'module)
datalayers = [dl.LayerGroup([dl.GeoJSON(data=self.op_geojson[i], 
                                        hideout = [], 
                                        id = {'type': op_geojson, 'index': i},
                                        options = {'style': style_handle, 'onEachFeature': ns('on_each_feature')}
                                        ), 
                             dl.GeoJSON(data=self.left_op_geojson[i], 
                                        hideout = [], 
                                        id = {'type': left_op_geojson, 'index': i},
                                        options = {'style': style_handle, 'onEachFeature': ns('on_each_feature')}
                                        ),
                             dl.GeoJSON(data=self.right_op_geojson[i], 
                                        hideout = [], 
                                        id = {'type': right_op_geojson, 'index': i},
                                        options = {'style': style_handle, 'onEachFeature': ns('on_each_feature')}
                                        )
                           ]
                         )
              for i in range(len(self.op_geojson))]

Having looking at my code now, is it possible that its because I'm putting all my GeoJSONs in a LayerGroup? Also, In my case, I had to add some additional things to my GeoJSONs so I make the dicts manually instead of using dlx.dicts_to_geojson. The format is the same though, so I would hope that is not what causes the popup behavior difference. It's also worth noting that I also then end up putting each LayerGroup in a dl.Overlay, and then use dl.LayersControl with it for the checkboxes, so maybe one of these things is changing the behavior.

perfectly-preserved-pie commented 1 year ago

Good thought - I think it is because you're using a LayerGroup. I was able to stop the popups from closing:

# return a LayerGroup with GeoJSON as one of its layers
  return dl.LayerGroup([dl.GeoJSON(
      data=geojson,
      cluster=False,
      superClusterOptions={
          'radius': 160,
          'minZoom': 3,
      },
      zoomToBoundsOnClick=True,
      options=dict(onEachFeature=ns("on_each_feature"))
  )])

That stops the popup from closing when the map is panned, but as you can see, without cluster=True, I don't have clustering. When I enable clustering, the behavior reverts to closing whenever the map is moved. Are you using clustering in your app? If so, how'd you get around this?

perfectly-preserved-pie commented 1 year ago

Looks like this is because of a limitation of the Supercluster library, but I am curious to know if you found a way around it or are just avoiding cluster=True

anaypat5 commented 1 year ago

Unfortunately the data I am attaching to a GeoJSON is sets of polylines, so I have not really experimented with marker clustering at all. My guess at a work around would be to use a custom clusterToLayer function inside the GeoJSON, which might even be as simple as just returning the basic clustering that is already done. You could try modifying the example code here to work with your app, but unfortunately I haven't done too much reasearch into clustering, so I hope this leads you in the right direction.

perfectly-preserved-pie commented 1 year ago

Resolved via #115 https://github.com/thedirtyfew/dash-leaflet/issues/180#issuecomment-1694490970 🎉