py-capellambse
py-capellambse copied to clipboard
fix: Add missing default styling of `PhysicalComponent`s for natures
Does solve #167 but not ideally and dry.
Have a look at the capstyle default style dict. With this solution we needed to add many styleclasses with the same style-dict due to the part_factory
. We dynamically add Node
or Behavior
to the .styleclass
attribute of the object like we did in the Human
and Actor
case. Indeed it seems like the is_human
attribute (causing Human
in the styleclass) doesn't change the styling in anyway. In the stylesheet generation it seems like we are matching the whole styleclass which limits us to also use the whole styleclass in the key in capstyle's default styledict. We have 2 ways to improve the situation:
- Make the stylesheet generator compatible with wildcards/regex. This enables usage of styleclasses like e.g.:
Physical*Actor
,Physical*NodeComponent
andPhysical*BehaviorComponent
and these 3 would be the only needed, differing style pairs. - Instead of having one styleclass add support of multiple classes, where the first one is the inferred/original one from the xtype (here
PhysicalComponent
). This would lead to styleclassesPhysicalComponent
,Human
,Actor
,Node
orBehavior
. How would we determine which styling class is superior to the others? Take in mind this option does need more changes, especially in the file that declares diagram objects (Diagram
,Box
,Edge
, etc.) and the svg submodule.
I'm looking forward to discuss moving forward.
Make the stylesheet generator compatible with wildcards/regex.
Bad idea, this would give up all the performance benefits of hashable keys, and AIRD diagram rendering is already slow enough.
Instead of having one styleclass add support of multiple classes, where the first one is the inferred/original one from the xtype (here PhysicalComponent).
That could work, but we'd still need to distinguish between different derived classes within one "original" class. For example, the PhysicalBehaviorActor
and PhysicalBehaviorComponent
introduced in this very PR both derive from PhysicalComponent
. So just specifying the original class wouldn't be enough, we'd need to make up a separate "styling-only class" ... which would introduce a not-insignificant amount of complexity to this already very complex module.
Another option could be to have some kind of dynamism in the definition itself. Right now the definitions are static: they're looked up by a predefined name and that's it, if you want a different definition you need to change the name you look up. We could make it possible to define a function instead that takes a (semantic) model element to be styled, and returns the default styling for that element. Then we might not even need to modify the styleclass
at all like we currently do.
But this would probably also have some ripple effects in the SVG part of the discussion (as it also defers to capstyle
), and in general is a large enough change that I would postpone it to when we attack #107 - one more breaking change there wouldn't stand out ;)
Instead of having one styleclass add support of multiple classes, where the first one is the inferred/original one from the xtype (here PhysicalComponent).
That could work, but we'd still need to distinguish between different derived classes within one "original" class. For example, the PhysicalBehaviorActor and PhysicalBehaviorComponent introduced in this very PR both derive from PhysicalComponent. So just specifying the original class wouldn't be enough, we'd need to make up a separate "styling-only class" ... which would introduce a not-insignificant amount of complexity to this already very complex module.
What I meant is that we'll always keep the original styleclass that is received from the xtype (e.g.: PhysicalComponent
). This one has some style definitions in capstyle's style-dict. Now we add other, custom styleclasses (e.g.: Actor
or Node
) that come with their own default style-dicts, maintained in capstyle's style-dict. The problem then is which overlapping CSS-definitions from the custom default styles are superior and to be taken...
Also: Even without having the default styles registered in capstyle's style-dict, the correct colors and gradients (I suppose obj_styles
but not text_styles
!) are recognized in the rendering step. We may invest too much time into default style declaration that we never use... I'm not 100% sure about that.
always keep the original styleclass that is received from the xtype […]. Now we add other, custom styleclasses (e.g.:
Actor
orNode
) that come with their own default style-dicts, maintained in capstyle's style-dict. The problem then is which overlapping CSS-definitions from the custom default styles are superior and to be taken...
That would be a problem that I don't really think we should have to deal with if we can avoid it. Although I guess dealing with it could also be as simple as making the ordering of those additional classes significant, and the last one wins. But still, IMHO having to deal with multiple style classes at all is already too much complexity.
Also: Even without having the default styles registered in capstyle's style-dict, the correct colors and gradients (I suppose
obj_styles
but nottext_styles
!) are recognized in the rendering step. We may invest too much time into default style declaration that we never use... I'm not 100% sure about that.
That happens because Capella often saves all style information explicitly in the AIRD, and the parser picks it up if it differs from its known defaults. But our use case also entails supporting other diagram sources like the context diagrams extension, which greatly benefits from having accurate defaults. So IMO this is time well spent.