New Added support for node label alignment#78
New Added support for node label alignment#78parmar-abhinav wants to merge 1 commit intomemgraph:release/1.0.0from
Conversation
|
@tonilastre Can you pls review the PR. |
tonilastre
left a comment
tonilastre
left a comment
There was a problem hiding this comment.
This is a really cool feature, but there are some issues (e.g. top, or combining left vs right). Please check the comments.
| The enum `NodeLabelAligment` which is used for the node `label alignment` property is defined as: | ||
|
|
||
| ```typescript | ||
| export enum NodeLabelAligment { |
There was a problem hiding this comment.
A typo, missing n -> Alig + n + ment
There was a problem hiding this comment.
Please check other usage of NodeLabelAlignment because the typo is there too.
| HEXAGON = 'hexagon', | ||
| } | ||
|
|
||
| export enum NodeLabelAligment { |
There was a problem hiding this comment.
When thinking about this, I feel like this should be called NodeLabelPosition. At first (before running it), I thought this aligns the label, and the text within the label, e.g.
LEFT ALIGNMENT
This is
multiline
text
RIGHT ALIGNMENT
This is
multiline
text
So I would propose renaming this to NodeLabelPosition because it is actually a label position, rather than alignment.
| switch (node.getLabelAlignment()) { | ||
| case NodeLabelAligment.BOTTOM: | ||
| labelY += distance; | ||
| labelTextAlign = LabelTextAlign.CENTER; | ||
| labelTextBaseline = LabelTextBaseline.TOP; | ||
| break; | ||
| case NodeLabelAligment.TOP: | ||
| labelY -= distance; | ||
| labelTextAlign = LabelTextAlign.CENTER; | ||
| labelTextBaseline = LabelTextBaseline.BOTTOM; | ||
| break; | ||
| case NodeLabelAligment.LEFT: | ||
| labelX -= distance; | ||
| labelTextAlign = LabelTextAlign.RIGHT; | ||
| labelTextBaseline = LabelTextBaseline.MIDDLE; | ||
| break; | ||
| case NodeLabelAligment.RIGHT: | ||
| labelX += distance; | ||
| labelTextAlign = LabelTextAlign.LEFT; | ||
| labelTextBaseline = LabelTextBaseline.MIDDLE; | ||
| break; | ||
| default: | ||
| labelY += distance; | ||
| labelTextAlign = LabelTextAlign.CENTER; | ||
| labelTextBaseline = LabelTextBaseline.TOP; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Actually, this should be part of Label logic. I would propose the following:
NodeLabelPosition.{TOP, BOTTOM, LEFT, RIGHT} // default = BOTTOM
NodeLabelAlignment.{LEFT, RIGHT, CENTER} // default = CENTER
EdgeLabelPosition.{CENTER} // default = CENTER
EdgeLabelAlignment.{LEFT, RIGHT, CENTER} // default = CENTER
And then Label logic receives position + alignment and handles the rest. Please take care of multiple lines because it is not working well. Check the image below for the "top".
There was a problem hiding this comment.
Node logic should just handle the start position: x, y and give that to the Label logic. Label should take the start position + label position + label alignment + text lines to figure out where each line should be.
|
|
4 participants
This PR introduces a new feature allowing users to have greater control over the node label alignment in the graph by defining custom not label positions i.e. top, left, right, bottom