Declare LayerGroup view_module and model_module #873
Conversation
| @@ -10,6 +10,8 @@ export class LeafletLayerGroupModel extends layer.LeafletLayerModel { | |||
| return { | |||
| _view_name: 'LeafletLayerGroupView', | |||
| _model_name: 'LeafletLayerGroupModel', | |||
| _view_module: 'jupyter-leaflet', | |||
| _model_module: 'jupyter-leaflet', | |||
There was a problem hiding this comment.
I guess this may not actually be enough, do we also need the _view_module_version and the _model_module_version?
There was a problem hiding this comment.
Do I just initialize them to null?
|
Thanks! I wonder why/how it was working without this Oo |
I think this is only really critical in the static html renderer where these are used to fetch the library from NPM. Anything else I can do here? Not sure what to initialize the version values to. |
|
Hello, is there any Solution for Python Users too? Or do i just not get how its done?? |
|
The fix is in JavaScript but it fixes the Python ipyleaflet widget. |
| @@ -10,6 +10,10 @@ export class LeafletLayerGroupModel extends layer.LeafletLayerModel { | |||
| return { | |||
| _view_name: 'LeafletLayerGroupView', | |||
There was a problem hiding this comment.
Sorry for not looking at this more deeply the other time. This is actually missing a call to the super's defaults method:
...super.defaults(),Similar to here: https://github.com/jupyter-widgets/ipyleaflet/blob/master/js/src/layers/AntPath.js#L11
This will merge the superclass defaults attribute which contains the proper module names and versions.
LayerGroup is actually not the only one missing this, ImageOverlay also misses it (and maybe others).
|
I just opened #894 that replaces this PR. Thank you @philippjfr for catching this issue! |
|
Thanks @martinRenou, sorry I didn't get a chance to follow up. |
|
No worries. I should have properly reviewed your PR the first time to not waste everyone's time, totally my fault. |
The
LayerGroupwas not inheriting the_view_moduleand_model_modulefrom the super, which meant that the view and model could not be resolved in exported contexts. I'm guessing it was intentional not to call...super.defaults()to avoid inheriting a bunch of other attributes from theLeafletLayerModelbut I'd love a confirmation of that from you @martinRenou.Fixes #811
Fixes #761