-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Element API Overhaul #88
Conversation
Very much looking forward to this next release! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small pedantic suggestions...
py_src/ipyelk/elements/extended.py
Outdated
# TODO need ability to resize the min width based on label/child max width | ||
for child in self.children: | ||
child.layoutOptions = merge( | ||
opt.OptionsWidget( | ||
options=[ | ||
opt.NodeSizeConstraints(), | ||
opt.NodeSizeMinimum(width=self.width, height=20), | ||
opt.NodeSizeMinimum(width=int(self.width), height=20), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder if it would be worth it to add height as an attribute above...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe width
should be min_width
and height
added as an attribute called min_height
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a min_height but i think width is still cleaner. The intention is that this is width shared by all of the compartments. We might be able to set the width directly but then there is less feedback if the user "guessed" the wrong width (marks get rendered on top of each other instead of the compartment borders looking odd). There might be better ways to solve this in the future either with a dedicated "node:compartment" type (which has issues to being out of sync with how elkjs positioned the marks) or with multiple passes of layout calls to elkjs.
py_src/ipyelk/elements/symbol.py
Outdated
|
||
@classmethod | ||
def make_defs(cls, symbols: List["Symbol"]) -> Dict[str, "Symbol"]: | ||
"""Take a list of symbols and return the def dictionary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"""Take a list of symbols and return a dictionary with the symbol's identifier as the key
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reworked into a separate SymbolSpec
class
…dpointsymbol offset field names
Work In Progress
Goal is to try and streamline some of the api and schema for building diagrams.