Cleanup MacroElements #2088
Cleanup MacroElements #2088
Conversation
There was a problem hiding this comment.
Nice that you identified some areas where templates are not used in the default way. I added some comments to places that may require some more attention. If you could limit this PR to just the straight forward cases, it should be easy to merge. Then we can maybe look at the more complicated cases separately or leave them as is.
Earlier I noted many macroelements do not fully use the facilities of Figure, but instead in render manipulate the parent Figure. Step 1: remove GlobalSwitches as Element
hansthen
force-pushed
the
cleanup_macroelements
branch
from
f40bd8c to
0e9ef12
Compare
I made the requested changes. |
| @@ -60,6 +60,29 @@ class Draw(JSCSSMixin, MacroElement): | |||
|
|
|||
| _template = Template( | |||
| """ | |||
| {% macro html(this, kwargs) %} | |||
| {% if this.export %} | |||
| <style> | |||
There was a problem hiding this comment.
this style element was originally added to the header. Here it's in the body. I guess it doesn't make a difference really, does it?
During an earlier change, I noted that many
MacroElementsubclasses do not fully use the templating facilities of their parent class.This PR makes the code more internally consistent by using the template route where possible. I skipped the
DualMap,VegaandVegaliteclasses because I do not fully understand what is happening there.