Leaflet control

[hansthen]

Based on the original draft by @Conengmo

1. Create a new base class Control, which can be used to more easily create new control based plugins.
2. Update the original CustomControl so that it also accepts a style parameter.
3. Change all the plugins that create L.Control subclasses to use Control as a base class.

This is to some extent a work-in-progress. Once https://github.com/python-visualization/folium/pull/1898 is merged I can make Control inherit from JSCSSMixin and remove that class from all the plugins and just use the inherited method from JSCSSMixin inheritance to add stylesheets and javascript.

Furthermore, many of the plugins could be simplified. In the current implementation options are very often coded in the _Template. By using the Control base class and JsCode parameters as part of the specified options we can avoid that. Or, at least, we can move flow code from the Jinja templates into the __init__ method, which is cleaner.

Also, I tried to make map.LayerControl as subclass of Control, but that resulted in circular dependencies.

I did not do everything at once, because I want to take small steps and check if everything stays backward compatible.

[hansthen]

Thanks for the review. I will rework it as requested.

[hansthen]

@Conengmo I removed the parts that could be handled as separate Pull Requests. I also rewrote the MousePosition control plugin. I think it clarifies why adding Control allows us to simplify a large part of the code base. Let me know what you think.

I would prefer to rewrite the other plugins incrementally. However, that means that for a while there will be inconsistent approaches in the code.

[hansthen]

First off, thanks for looking at the Pull Request. I know you are not enthusiastic about making major changes to Folium, so it is appreciated that you are willing to consider this.

it's used in the inheritance tree to indicate something is a control. Indeed. I also have the vague hope it will in the future help the developers of streamlit-folium. This package allows for dynamic changes to the generated Leaflet map. They do fairly complicated stuff now with regular expressions to separate Layers and Controls. Having a simple marker in the python object hierarchy would make their life simpler.

What I find difficult is that in that second case the template and the init of Control should be skipped. But the init it ran, and creates some self.options that are not used. That sounds like it could cause issues, running extra code in the tree that's not needed.

Just to clarify, you have no difficulties with how I rewrote the mouse_position? It is rather that in all the other instances, there is an unused parent class? I agree with you there, that the situation is not ideal. I would rewrite all the plugins (and native Controls) similarly, but it is a lot of work. Not all of it will be as straightforward as for the mouse_position plugin. I would prefer to approach this incrementally after (and if) we come to an agreement on the basic approach.

Would it help if I added the Control class also incrementally whenever I update the a plugin to use the new approach?

[Conengmo]

I also have the vague hope it will in the future help the developers of streamlit-folium. This package allows for dynamic changes to the generated Leaflet map. They do fairly complicated stuff now with regular expressions to separate Layers and Controls. Having a simple marker in the python object hierarchy would make their life simpler.

That's interesting! I'd like to know more about that.

I gotta run, but just as a quick thought for now. Does it make sense to split this class into a Control class that's basically empty but indicates the object name in Javascript is a control, and a ControlTemplate or whatever (bad name) that can be used to create any control object? Where Control is inherited from by whichever class it applies to, and ControlTemplate inherits from Control and is used to build control objects?

I understand a bit better know to have our Python classes only implement one Javascript class. Question I have now is how we then bundle these in a way that's convenient for our Python users?

[Conengmo]

Just to clarify, you have no difficulties with how I rewrote the mouse_position?

No, I like the abstraction. Only doubt would be whether the abstraction is common enough to warrant abstracting it.

Would it help if I added the Control class also incrementally whenever I update the a plugin to use the new approach?

Not sure if I fully understand, but if you're talking about not adding Control as a parent to a class that implements multiple Javascript classes, then yes, doing that incrementally sounds like a good plan.

[hansthen]

Not sure if I fully understand, but if you're talking about not adding Control as a parent to a class that implements multiple Javascript classes, then yes, doing that incrementally sounds like a good plan.

I meant that in the current PR, I added Control already to several python classes that generate a Leaflet Control without adapting their implementation. These classes just have a different base class, but do not use their parent's template of constructor. This should not break anything, but it is maybe not the cleanest solution. I wanted to rewrite all the plugins later to cleanup the code, calling the parent constructor and probably removing their template code.

My alternative would be to not add Control as base class for the other plugins now, but only after I have adapted the code.