Fix some issues in diagrams #369
Conversation
|
This PR is a draft with no activity for a while. Did you mean to submit it for review, or is there still work to be done here? (Also, as first review comment: the last commit message is cut off.) |
freshavocado7
force-pushed
the
fix-diagrams
branch
from
6d06edd to
555e9b9
Compare
capellambse/svg/style.py
Outdated
| @@ -31,7 +31,7 @@ class Styling: | |||
| def __init__( | |||
| self, diagram_class: str | None, class_: str, prefix: str = "", **attr | |||
| ): | |||
| self._diagram_class = diagram_class | |||
| self._diagram_class = diagram_class or "" | |||
There was a problem hiding this comment.
This change seems unnecessary. Is this a left-over from a previous iteration, or is it actually needed somewhere I can't see/find?
There was a problem hiding this comment.
diagram_class can be None. There was a point were I didn't want to do the or ... stuff repetitively, which was removed again. So it is not needed anymore.
| } | ||
| if "class" in builder.label: | ||
| textattrs["class_"] = builder.label["class"] | ||
|
|
||
| if transform := getattr(builder.labelstyle, "transform", None): |
There was a problem hiding this comment.
Maybe I'm missing a detail in this new branch, but how does this handle multi-line labels? The new code doesn't seem to call any functions related to that. Are those calls not needed here after all?
There was a problem hiding this comment.
They are probably needed, but I only tested it on single line text labels.
There was a problem hiding this comment.
We'll simply raise an error for all unsupported cases for now. Something to worry about later. :)
Wuestengecko
force-pushed
the
fix-diagrams
branch
from
95384ff to
55e8cfa
Compare
Several changes needed for the
realization_viewdiagram feature in the context-diagrams project.