Open danprince opened 1 year ago
This looks like a great start to scoping out the feature.
How would this differ from the existing edge endpoint properties? Are there different use cases or edge cases?
https://js.cytoscape.org/#style/edge-endpoints
What’s the rationale for implementing this as a node property, as opposed to an edge property?
On Sep 12, 2023, at 08:33, Dan Prince @.***> wrote:
Description of new feature
What should the new feature do? For visual features, include an image/mockup of the expected output.
Add a margin property which allows users to control the spacing between nodes and edge connection points.
https://user-images.githubusercontent.com/1266011/267327341-60e6f180-a920-4eaf-b8df-20c1ad136d1b.png This would involve adding one new style property to Cytoscape, which would only apply to nodes.
margin the space to include between the outside edge of a node and its edge connection points Considerations:
Should relative margins be supported (e.g. 25%)? How would they work on shapes with irregular aspect ratios? Motivation for new feature
Describe your use case for this new feature.
This styling can't be achieved as part of an extension (although it can be hacked to some degree if you're happy to sacrifice having a border, which is a dealbreaker for us) because it involves adding new style properties and adjusting the calculations which are used to position edges.
Part of trying to achieve Kumu parity, from #3145 https://github.com/cytoscape/cytoscape.js/discussions/3145.
For reviewers
Reviewers should ensure that the following tasks are carried out for incorporated issues:
Ensure that the reporter has adequately described their idea. If not, elicit more information about the use case. You should iteratively build a spec together. Ensure that the issue is a good fit for the core library. Some things are best done in extensions (e.g. UI-related features that aren't style-related). Some things are best done by app authors themselves -- instead of in Cytoscape libraries. The issue has been associated with a corresponding milestone https://github.com/cytoscape/cytoscape.js/milestones. The commits have been incorporated into the unstable branch via pull request. The corresponding pull request is cross-referenced. — Reply to this email directly, view it on GitHub https://github.com/cytoscape/cytoscape.js/issues/3162, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHRO47F5RZQBZEL6LVCB5LX2BJCNANCNFSM6AAAAAA4UZB25U. You are receiving this because you are subscribed to this thread.
How would this differ from the existing edge endpoint properties? Are there different use cases or edge cases?
The edge-endpoints
properties do what we need, but we want to move the edge endpoints away consistently from a given set of nodes.
In Kumu we use :to(...)
and :from(...)
selectors, which select all edges to or from a given node selector. This would make these two rules functionally equivalent.
node[type=person] {
margin: 10px;
}
:to(node[type=person]), :from(node[type=person]) {
source-distance-from-node: 10px;
target-distance-from-node: 10px;
}
I think the use-cases are pretty much the same: create more space and breathing room around nodes. In our opinion the user experience is more intuitive if you can control the spacing consistently based on the type of node.
The obvious edge case that I hadn't identified before is around how you handle conflicts between the two properties. I think that because the edge-endpoint properties are more granular, they should always override the margin properties.
This may motivate revisiting arrow selectors for edges, e.g. .foo -> .bar
or node.foo -> node
. That would remove conflicts arising from the addition of new properties: The existing edge properties would suffice so long as you can use selectors to specify edges relative to particular sorts of nodes.
The arrow selectors were deprecated for performance and other reasons, so if we go that route, then we ought to do a bit of analysis on the use cases and the trade-offs.
For traversals generally, it's best to avoid them in selectors. They can be very expensive, just as they are in CSS proper. They can be simulated by applying classes using the traversal API once a network has been loaded. For small networks, the performance may not matter and the convenience of selectors may win out. It depends on the use cases.
Ref:
This issue has been automatically marked as stale, because it has not had activity within the past 14 days. It will be closed if no further activity occurs within the next 7 days. If a feature request is important to you, please consider making a pull request. Thank you for your contributions.
Pinned
TBD:
I'm currently leaning towards supporting both the edge offset properties and node margin properties. A simple approach that may feel intuitive to devs would be to be consistent with CSS: In CSS proper, when you have two adjacent elements with margins, the larger margin 'wins out'. So if we were to be consistent with that, then an edge with both an offset and a connected node margin would use the max(edge offset, node margin)
when drawing the edge.
How does that sound, @danprince?
Sounds great @maxkfranz, should be back into a usual working rhythm now. Should get a chance to land some PRs in the next couple of weeks.
Sounds good. Looking forward to it.
Description of new feature
What should the new feature do? For visual features, include an image/mockup of the expected output.
Add a margin property which allows users to control the spacing between nodes and edge connection points.
This would involve adding one new style property to Cytoscape, which would only apply to nodes.
margin
the space to include between the outside edge of a node and its edge connection pointsConsiderations:
Motivation for new feature
Describe your use case for this new feature.
This styling can't be achieved as part of an extension (although it can be hacked to some degree if you're happy to sacrifice having a border, which is a dealbreaker for us) because it involves adding new style properties and adjusting the calculations which are used to position edges.
Part of trying to achieve Kumu parity, from https://github.com/cytoscape/cytoscape.js/discussions/3145.
For reviewers
Reviewers should ensure that the following tasks are carried out for incorporated issues:
unstable
branch via pull request. The corresponding pull request is cross-referenced.