Draft for Node Label Position

[kmalyavina]

Closes #365.

Works fine for default size nodes with constant values for x/y offsets. Need to include the node radius/size in the offset instead.

I have a couple questions and thoughts/suggestions.

Questions:

1. In the label.size, what is the yLine?
2. How does it currently get the y value/distance to position the label below a node?
3. How can I get the node width/height to use as offset for label alignment?

Thoughts/Suggestions:

• Based on the usage and contents of the object, 'dimensions' seems a more appropriate name than 'size' (worried about changing it in case it's used elsewhere)
• The whole network body is passed in for label creation, but it looks like only view.scale is used - why not just pass in the scale?
• Positioning label given center/origin of a node would probably make more sense than adjusting based on current position at bottom (related to question 2 above)

[Thomaash]

Hi there,

thanks for working on this. I don't have much time right now and most likely won't have tomorrow. I'll have a look probably at the weekend.

PS: If you won't hear from me by the next week feel free to mention me here to remind me.

[Thomaash]

First of all thanks for the example. It's really nice to just check it out, run it and see it in action. The code you have so far also looks good given it's surrounding.

Answers

1. I have no idea what yLine is. I looked through the code but I'm as clueless as you.
2. The logic seems to be split between several methods and seems to be partly computed outside of the Label class. The interesting place here are the various draw methods of the shapes in lib/network/modules/components/nodes. In case you wonder this design pattern is commonly referred to as spaghetti code.
3. The node has a shape property which has bounding box property (node.shape.boundingBox) or it is possible to get much more accurate data for any specific angle but if the choices are top, right, bottom, left and inside there's no point in doing that.

Renaming size to dimensions

This seems much more descriptive. It's being used at

vis-network/lib/network/modules/components/Edge.js

Line 783 in 7d3361c

y: this.labelModule.size.yLine,

and there is also very interesting way of using it at

vis-network/lib/network/modules/components/Node.js

Line 537 in 7d3361c

return this.labelModule.size();

(Yes, this will throw when invoked. I think it's safe to just delete this one.)

Network body

The scale can change at any point so it has to be provided within an object. The body is a single object maintaining the state of the whole Network used all over the place so it's probably best left as it is if just for the sake of consistency.

Positioning the label

The way it is doesn't seem to make any sense whatsoever. What would maybe make more sense would be if the label class handled just the label itself, provided it's bounding box to the node and the positioning would be done by the node.

Afterthought

Going through the code maybe it would be best to throw it away and rewrite the whole thing from the scratch. This is just unmaintainable mess. Though who has the time to do something like that?

[kmalyavina]

Haha, rewriting the whole thing from scratch would be pretty insane.

I'll attempt to move all the label positioning parts to ShapeBase and see if I can get it a little better at least.

[kmalyavina]

Feels like I didn't do much to help the spaghetti code problem, but I at least got the node placement to work as intended.

Big YIKES at all the duplicateupdateBoundingBox. Trying to update them to make them accurate to take into account labels on the top/sides.

Kind of regret trying to rename size to dimensions haha

[Thomaash]

Hi there and thanks for your work. I pointed out a few things here and there. One thing that still doesn't work is selecting and deselecting the nodes. Otherwise it looks quite good.

[kmalyavina]

Hmm... when I go back to before renaming size to dimensions ("Label position initial draft" commit), I don't have the selecting/dragging issues. I'll try to go back to that point and redo the changes from that point...

[kmalyavina]

Ok, got those changed merged in after undoing the 'dimensions' rename.

I tried moving the font align into the Label setOptions, but it seems to be getting overwritten by something and I couldn't find what was doing it.

I noticed a weird thing though that looks like all networks are shifted up?? Not sure what could be causing it

And it really is astounding how much of a mess the code is for the components.... looks like it's in dire need of refactoring (and probably a conversion to TypeScript in the process haha). But that's beyond what I can put in at the moment.

[Thomaash]

I didn't manage to crash it or make it throw errors this time. Good job!

Looking through it I found one bug (unless I'm wrong in which case feel free to correct me) which causes the fit function to fit nodes or labels outside of the canvas. However it seems that you also somehow broke the initial fit. I have no clue how though.

I tried moving the font align into the Label setOptions, but it seems to be getting overwritten by something and I couldn't find what was doing it.

Yeah. I know this very well. The way options are handled here is terrible. There's actually WIP replacement but it's such a mess, there's always something lurking around the corner just waiting to force me to redesign the thing.

[Thomaash]

Fit works properly now and that's great but the network is still shifted to the bottom right on initial zoom.

The following example demonstrates the issue. Fit correctly zooms though so I suppose there's just some small omission somewhere and something doesn't get updated when it should.

<!DOCTYPE html>
<html>
  <head>
    <title>Vis Network | Labels | Label Alignment</title>

    <script
      type="text/javascript"
      src="../../../standalone/umd/vis-network.min.js"
    ></script>

    <style type="text/css">
      #mynetwork {
        width: 400px;
        height: 400px;
        border: 1px solid teal;
      }
    </style>
  </head>

  <body>
    <div id="mynetwork"></div>
    <script type="text/javascript">
      // create an array with nodes
      const nodes = [
        {
          id: 5,
          x: 200,
          y: 300,
          label: "Node 5:\nBottom Label\n(default)",
          shape: "dot"
        },
        {
          id: 6,
          x: 200,
          y: 100,
          label: "Node 6:\nTop Label",
          shape: "dot",
          labelPosition: "top",
          color: "yellow"
        },
        {
          id: 7,
          x: 200,
          y: 200,
          label: "Node 7:\nInside Label",
          shape: "dot",
          labelPosition: "inside",
          color: "yellow"
        },
        {
          id: 8,
          x: 100,
          y: 200,
          label: "Node 8:\nLeft Label",
          shape: "dot",
          labelPosition: "left",
          color: "orange"
        },
        {
          id: 9,
          x: 300,
          y: 200,
          label: "Node 9:\nRight Label",
          shape: "dot",
          labelPosition: "right",
          color: "orange"
        }
      ];

      // create an array with edges
      const edges = [];

      // create a network
      const container = document.getElementById("mynetwork");
      const data = {
        nodes: nodes,
        edges: edges
      };
      const options = { physics: false };
      const network = new vis.Network(container, data, options);

      (async () => {
        await new Promise(resolve => setTimeout(resolve, 3000));
        network.fit();
      })();
    </script>
  </body>
</html>

[kmalyavina]

Spent a lot of time trying to fix that initial load without much luck, and I'm at the end of the time I can dedicate to working on this :(