Skip to content
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.

Sign up for GitHub

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

save textnode movement nad #126

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

save textnode movement nad #126

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d1263ad
save textnode movement nad
souissimai Nov 21, 2024
a3ae771
Merge branch 'main' into sava_textnode-movement_for_nad
souissimai Nov 21, 2024
9c4e197
Merge branch 'main' into sava_textnode-movement_for_nad
sBouzols Nov 26, 2024
283af77
renaming..
souissimai Nov 27, 2024
23ac3ed
Merge branch 'main' into sava_textnode-movement_for_nad
sBouzols Dec 2, 2024
ab36cd8
Merge branch 'main' into sava_textnode-movement_for_nad
sBouzols Dec 4, 2024
5425e2c
Merge branch 'main' into sava_textnode-movement_for_nad
sBouzols Dec 6, 2024
06a246b
Merge branch 'main' into sava_textnode-movement_for_nad
sBouzols Dec 9, 2024
bac8d43
re apply css rules to display moved label
souissimai Dec 12, 2024
f49683b
Merge branch 'main' into sava_textnode-movement_for_nad
souissimai Dec 13, 2024
0870cee
cleaning
souissimai Dec 19, 2024
3fed7ac
Merge branch 'main' into sava_textnode-movement_for_nad
sBouzols Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 64 additions & 20 deletions src/components/network-area-diagram-viewer/network-area-diagram-viewer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ export type OnMoveTextNodeCallbackType = (
connectionShiftX: number,
connectionShiftY: number,
connectionShiftXOrig: number,
connectionShiftYOrig: number
connectionShiftYOrig: number,
mousePosition: Point
) => void;

export type OnSelectNodeCallbackType = (equipmentId: string, nodeId: string) => void;
Expand Down Expand Up @@ -181,18 +182,37 @@ export class NetworkAreaDiagramViewer {
);
return node?.svgId || null;
}

private getTextNodeIdFromEquipmentId(equipmentId: string) {
const node: TextNodeMetadata | undefined = this.diagramMetadata?.textNodes.find(
(node) => node.equipmentId == equipmentId
);
return node?.svgId || null;
}
public moveNodeToCoordinates(equipmentId: string, x: number, y: number) {
const nodeId = this.getNodeIdFromEquipmentId(equipmentId);
if (nodeId != null) {
const elemToMove: SVGElement | null = this.container.querySelector('[id="' + nodeId + '"]');
if (elemToMove) {
const newPosition = new Point(x, y);
this.onDragStart(elemToMove);
this.onDragEnd(newPosition, false);
this.onDragEnd(newPosition, false, false);
}
}
}
public moveTextNodeToCoordinates(equipmentId: string, x: number, y: number) {
const textnodeId = this.getTextNodeIdFromEquipmentId(equipmentId);
if (textnodeId != null) {
const elemToMove: SVGElement | null = this.container.querySelector('[id="' + textnodeId + '"]');
if (elemToMove) {
const newPosition = new Point(x, y);
this.onDragStart(elemToMove);
this.onDragEnd(newPosition, false, false);
}
//Update css rules to diplay labels
const firstChild: HTMLElement = <HTMLElement>this.svgDraw?.node.firstChild;
this.injectDisplayCssRules(firstChild);
Comment on lines +211 to +213
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the purpose is to reapply css rules to display labels

}
}

public init(
minWidth: number,
Expand Down Expand Up @@ -433,17 +453,17 @@ export class NetworkAreaDiagramViewer {
// check if I moved or selected an element
if (this.draggedElement) {
// moving node
this.onDragEnd(this.getMousePosition(event), true);
this.onDragEnd(this.getMousePosition(event), true, true);
} else if (this.selectedElement) {
// selecting node
this.onSelectEnd();
}
}

private onDragEnd(newPosition: Point, callMoveNodeCallback: boolean) {
private onDragEnd(newPosition: Point, callMoveNodeCallback: boolean, callMoveTextNodeCallback: boolean) {
if (this.textNodeSelected) {
this.dragVoltageLevelText(newPosition);
this.updateTextNodeMetadataCallCallback(newPosition);
this.updateTextNodeMetadataCallback(newPosition, callMoveTextNodeCallback);
} else {
this.dragVoltageLevelNode(newPosition);
this.updateNodeMetadataCallCallback(newPosition, callMoveNodeCallback);
Expand Down Expand Up @@ -1236,7 +1256,7 @@ export class NetworkAreaDiagramViewer {
}
}

private updateTextNodeMetadataCallCallback(mousePosition: Point) {
private updateTextNodeMetadataCallback(mousePosition: Point, callMoveTextNodeCallback: boolean) {
if (this.onMoveTextNodeCallback != null) {
// get from metadata node connected to moved text node
const node: NodeMetadata | undefined = this.diagramMetadata?.nodes.find(
Expand All @@ -1255,19 +1275,22 @@ export class NetworkAreaDiagramViewer {
textNode.connectionShiftX = textNodeMoves[1].xNew;
textNode.connectionShiftY = textNodeMoves[1].yNew;
// call the node move callback, if defined
this.onMoveTextNodeCallback(
node.equipmentId,
node.svgId,
textNode.svgId,
textNodeMoves[0].xNew,
textNodeMoves[0].yNew,
textNodeMoves[0].xOrig,
textNodeMoves[0].yOrig,
textNodeMoves[1].xNew,
textNodeMoves[1].yNew,
textNodeMoves[1].xOrig,
textNodeMoves[1].yOrig
);
if (callMoveTextNodeCallback) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move test over this.onMoveTextNodeCallback here

if (this.onMoveTextNodeCallback != null && callMoveTextNodeCallback) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should create another bug I think. If you don't define a onMoveTextNodeCallback function then moving textNode should not work at all

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? still able to move node even if onMoveTextNodeCallback (tested)

this.onMoveTextNodeCallback(
node.equipmentId,
node.svgId,
textNode.svgId,
textNodeMoves[0].xNew,
textNodeMoves[0].yNew,
textNodeMoves[0].xOrig,
textNodeMoves[0].yOrig,
textNodeMoves[1].xNew,
textNodeMoves[1].yNew,
textNodeMoves[1].xOrig,
textNodeMoves[1].yOrig,
mousePosition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need mousePosition here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mousePosition to save the newPosition of dragged text node

);
}
}
}
}
Expand Down Expand Up @@ -1368,6 +1391,27 @@ export class NetworkAreaDiagramViewer {
}
}

public injectDisplayCssRules(htmlElementSvg: HTMLElement) {
const styleTag = htmlElementSvg.querySelector('style');
if (styleTag && 'textContent' in styleTag) {
const cssSelectors = ['.nad-label-box', '.nad-text-edges', '.nad-edge-infos'];

// update the styles to display labels if needed
cssSelectors.forEach((selector) => {
const styleTags = [...document.querySelectorAll('style')].filter((styleTag) =>
Copy link
Contributor

@sBouzols sBouzols Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could optimize this code a lot
You should call document.querySelectorAll('style') only once
Then filter styles nullish values .filter(Boolean) and not matching cssSelectors.
and finally loop over to set textContent.

This way you could avoid optional chaining operator elsewhere styleTag?.

By the way you have a problem because you have 2 local variables with the same name styleTag in injectDisplayCssRules and it's confusing...

Do you want to update the style on the given HTMLElement ? or on all Elements of the document ?

styleTag?.textContent?.includes(selector)
);
styleTags.forEach((styleTag) => {
styleTag.textContent =
styleTag?.textContent !== null
? styleTag?.textContent.replace(`{display: none;}`, `{display: block;}`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line bothers me : there is an unnecessary "?", and if the css rule is slightly different, it will not work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if using a ternary operator impacts readability then switch to if..else code format I think

: '';
});
});
} else {
console.error('Failed to create Style tag in SVG file!');
}
}
public getCurrentlyMaxDisplayedSize(): number {
const viewbox = this.getViewBox();
return Math.max(viewbox?.height || 0, viewbox?.width || 0);
Expand Down
Loading