Skip to content
This repository has been archived by the owner on Nov 18, 2022. It is now read-only.

Add metadata to enable moving nodes #87

Closed
wants to merge 3 commits into from
Closed

Conversation

tadam50
Copy link
Contributor

@tadam50 tadam50 commented Jul 13, 2022

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem ? If so, link to this issue using '#XXX' and skip the rest
powsybl/powsybl-diagram#443

Does this PR introduce a breaking change or deprecate an API? If yes, check the following:

  • The Breaking Change or Deprecated label has been added
  • The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

Other information:

(if any of the questions/checkboxes don't apply, please delete them entirely)

tadam50 added 2 commits July 13, 2022 15:52
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
@tadam50 tadam50 self-assigned this Jul 13, 2022
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

91.4% 91.4% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@flo-dup flo-dup left a comment

Choose a reason for hiding this comment

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

I'm wondering if after all it wouldn't be better to have tags in edges rather than in nodes. Indeed, each edge has only two node as attributes, so we could put those attribute in an empty xml element. It's less verbose than the other way around!
<nad:edge diagramId="x" equipmentId="z" node1="n1diagramId" node2="n2diagramId"/>

Nonetheless this would require a querySelectorAll in javascript to get the edges related to the clicked node.

What do you think?

@@ -110,6 +110,394 @@
}
]]></style>
<metadata xmlns:nad="http://www.powsybl.org/schema/nad-metadata/1_0">
<nad:voltageLevelNodes>
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 add a new tag? Couldn't the information be in the node tag?

@flo-dup flo-dup changed the title Issue 6 follow pr 73 Add metadata to enable moving nodes Jul 29, 2022
@flo-dup
Copy link
Contributor

flo-dup commented Nov 18, 2022

Transferred into repository powsybl-diagram: powsybl/powsybl-diagram#448

@flo-dup flo-dup closed this Nov 18, 2022
@flo-dup flo-dup deleted the issue_6_follow_PR_73 branch November 18, 2022 11:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants