`nodeDimensionsIncludeLabels` doesn't seem to work when using HTML labels

[cappslock]

I've noticed that the nodeDimensionsIncludeLabels option from various layouts doesn't seem to do anything when using this library. Specifically I'm noticing this with the dagre layout. Normal labels seem to work fine.

Can this be fixed?

I can provide a minimal example if that is helpful.

[josejulio]

I think that nothing can be done directly from this lib, I'm trying to do something similar but need some changes here and on cytoscape code, see: https://github.com/cytoscape/cytoscape.js/issues/2497

[georgecrawford]

@josejulio Since cytoscape/cytoscape.js#2497 is merged, what needs to be done to implement this? If you don't have the time, I might be able to open a PR to implement setting bounds-expansion as you describe. However, I'm new to cytoscape and don't know where to start!

[josejulio]

Long story short: I did that on other fork.

If you could port it to this (or even provide a better implementation!), we could merge it here and improve this project, I'm no longer looking into adding anything else to the fork: https://github.com/josejulio/cytoscape-node-html-label/commits/master

If you want to test how it works, you can consume it from npm but be warned that I don't have any plans on merging any PR on THAT fork anymore :)

edit: In case you are curious, this is what I did with the fork: https://github.com/kiali/kiali-ui/pull/1385

[dahnny012]

is there a reason we dont just merge that fix into master?

[blazespinnaker]

@josejulio Can you give us a very brief walkthrough of what you've done on your fork and kiali's use of it? It looks fairly extensive on the fork. How specific are your changes to your own use cases on that fork? Can that become the mainline and changes in this repo merged into it? I see that the change for kiali is optimized only for labels at the center/bottom and was wondering if similar short cuts were performed in your repo. Fwiw, I was able to get all your code to work after some minor changes. i would offer to help with the merge, but I just started looking at cytoscape today for the first time Also, I am really not a front end eng.

[josejulio]

I don't work on Kiali anymore, so i'll try to list what was added and for what reason (focusing on public API / functionality). Note that I forked it off, because I had some changes that I needed, but couldn't find the maintainer, eventually I found him and gave me access to this repo.

• Added event: nodehtml-create-or-update and nodehtml-delete for listening create/update and deletion of labels.
• Added a way to communicate with the library (i.e. returning an object on the setup) - see next point
• Force a refresh of some labels - As a consumer, I needed to update some labels that depended on external values (i.e. values that didn't trigger a natural refresh)

That's what I remember on top of my head and from quickly looking the changes in the fork, but if you see the code and have some questions, please do.

On the consumer side (Kiali) I implemented:

• A way for the cy node to be away of the label and consider it into its bounds-expansion every time there is an update with the help of the said changes. IIRC this was to prevent the html labels from overlapping with other nodes/labels.
• A mechanism to prevent "clicks" on nodes when clicking the label.

The changes in the fork could be integrated into this, they seem (IIRC) fairly generic. The chages in Kiali are a bit more specific, could eventually be integrated if they were generic enough.

Fwiw, I was able to get all your code to work after some minor changes

Nice!

i would offer to help with the merge, but I just started looking at cytoscape today for the first time Also, I am really not a front end eng.

Sharing your changes could help someone else to integrate, or if i even find time I could try. But honestly haven't found time to setup CI for this repo (help wanted)

Thanks!

[blazespinnaker]

I didn't do much in terms of getting changes to work. I just pulled the fork down via NPM and copied the nodehtml update events from kiali in. Once they were in, the grid layout was able to include the proper width of the labels and no overlap. I thought I had height working (my labels are above not below the nodes), but it turns out there are some edge cases giving me problems and my understanding of the bounding boxes is missing something.

[josejulio]

I thought I had height working (my labels are above not below the nodes), but it turns out there are some edge cases giving me problems and my understanding of the bounding boxes is missing something.

From the bounds-expansion docs on js.cytoscape.org (emph is mine)

bounds-expansions accepts 1 value (for all directions), 2 values, ([topAndBottom, leftAndRight]) or 4 values ([top, right, bottom, left]).

On Kiali's code, the top wasn't touched (here) You might try:

newBE[1] += requiredWidth * 0.5;
newBE[3] += requiredWidth * 0.5;
newBE[0] = requiredHeight; // Changed the index from 2 to 0

So that the bounding's height is expanded on top of the node, and not below.

[blazespinnaker]

Brilliant, that worked. For others, I copied the code @josejulio linked to starting at line 373.

Given collective time constraints but general desire to see this fixed, @josejulio do you think a user configurable call back with parameters bb / DOMNode to provide the newBE would be a reasonable solution here?

Sec issues aside, the extension amplifies the value of the parent framework measurably. Be sad to see it wither.

[josejulio]

Great that it worked for you, note that on latest code (something i did after that PR) the check to see if the BE is different to current one is done in other way:

https://github.com/kiali/kiali-ui/blob/master/src/components/CytoscapeGraph/CytoscapeGraph.tsx#L456-L486

Basically accepting a small error (0.00001) as equal.

Given collective time constraints but general desire to see this fixed, @josejulio do you think a user configurable call back with parameters bb / DOMNode to provide the newBE would be a reasonable solution here?

I would say that this could even be integrated in this library (probably with a opt-in or opt-out flag e.g. updateBoundExpansion?) and use the label alignment to decide how to distribute the required values in the bounding expansion field.

[blazespinnaker]

Great that it worked for you, note that on latest code (something i did after that PR) the check to see if the BE is different to current one is done in other way:

https://github.com/kiali/kiali-ui/blob/master/src/components/CytoscapeGraph/CytoscapeGraph.tsx#L456-L486

Basically accepting a small error (0.00001) as equal.

Given collective time constraints but general desire to see this fixed, @josejulio do you think a user configurable call back with parameters bb / DOMNode to provide the newBE would be a reasonable solution here?

I would say that this could even be integrated in this library (probably with a opt-in or opt-out flag e.g. updateBoundExpansion?) and use the label alignment to decide how to distribute the required values in the bounding expansion field.

Yes, I initially went there as well but then I began to ponder the potential edge cases and didn't feel comfortable I could enumerate them all. For example, I plan on including some fairly exotic HTML in my labels (relatively speaking) for various visualization scenarios.

[josejulio]

There are always edge cases :)...

Anyhow, i suppose an implementation with a callback to provide the new-BE would be easier to implement and let the user decide how it's going to work.

Could you create the issue with the requirements?

[blazespinnaker]

As this is working for me, it might be a good idea to hear from some other folks and generate more of a consensus viewpoint.