-
Notifications
You must be signed in to change notification settings - Fork 1
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
Move edge names when moving edges in network area diagram #77
Conversation
Signed-off-by: massimo.ferraro <massimo.ferraro@soft.it>
const nameElement: SVGGraphicsElement = | ||
edgeNode.lastElementChild as SVGGraphicsElement; | ||
if ( | ||
nameElement != null && | ||
nameElement.tagName == 'g' && | ||
(nameElement.id == null || nameElement.id.length == 0) | ||
) { | ||
// get DOM element containing name position | ||
const namePositionElement: SVGGraphicsElement = | ||
nameElement.lastElementChild as SVGGraphicsElement; | ||
if ( | ||
namePositionElement != null && | ||
namePositionElement.tagName == 'g' && | ||
(namePositionElement.id == null || | ||
namePositionElement.id.length == 0) | ||
) { |
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.
you could avoid all this with a querySelector, it would be more robust
const edgeNamePositionElement: SVGGraphicsElement =
edgeNode.querySelector(".nad-edge-label") as SVGGraphicsElement;
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.
Used querySelector for getting the element
const nameAngleElement: SVGGraphicsElement = | ||
namePositionElement.lastElementChild as SVGGraphicsElement; | ||
if ( | ||
nameAngleElement != null && | ||
nameAngleElement.tagName == 'text' | ||
) { | ||
edgeNameAngleElement = nameAngleElement; | ||
} |
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.
similarly
const edgeNameAngleElement: SVGGraphicsElement =
edgeNamePositionElement.querySelector("text") as SVGGraphicsElement;
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.
Used querySelector for getting the element
anchorPoint: Point, | ||
edgeStart: Point | ||
) { | ||
const edgeNameElements = DiagramUtils.getEdgeNameElements(edgeNode); |
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.
as the content of DiagramUtils.getEdgeNameElements
is very compact with querySelectors, I think you can remove it and call the querySelectors directly here
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.
Removed DiagramUtils function and used querySelector for getting the elements in this method
Signed-off-by: massimo.ferraro <massimo.ferraro@soft.it>
Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
no
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
In network area diagram, when moving an edge (due to the movement of a voltage level node) the edge name, if displayed, is not moved
What is the new behavior (if this is a feature change)?
When moving an edge (following the movement of a voltage level node) the edge name is correctly moved
Does this PR introduce a breaking change or deprecate an API?