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.

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

Clearing title and label with vis-network #739

Closed
jan-johansson-mr opened this issue May 21, 2020 · 6 comments · Fixed by visjs/vis-data#327
Closed

Clearing title and label with vis-network #739

jan-johansson-mr opened this issue May 21, 2020 · 6 comments · Fixed by visjs/vis-data#327

Comments

@jan-johansson-mr
Copy link

Hello,

I've discovered that ones I set a title or label with a vis-network edge, then it's hard to clear it out. I can clear out the label for an edge by providing a string with a space, e.g. ' ', but it will not work with the title. The title will show a small pop-up that is empty. I've tried updating the edge with undefined (causes no change to the edge), null and empty string '' and string with one space ' ' only causes the small pop-up to be empty (with mouse over the edge).

I've looked through the issues and so on, and not seen anything about this particular case, that is, clearing out a title or label for edge or node. It would be great to know if there is an issue with vis-network (maybe in combination with DataSet) or if there is some other way it's supposed to be done.

I'm currently on Using Windows 10, Chrome and Edge Chrome.

Thanks

@jan-johansson-mr
Copy link
Author

I've discovered how to remove title and label from edge and node, but in a very goofy way... By in a unit of work first remove the object from the DataSet, and then add the modified object. This can however trigger a 'ripple' effect throughout the network...

@jan-johansson-mr
Copy link
Author

jan-johansson-mr commented May 22, 2020

It turns out that vis-network handle node and edge differently on further investigation. E.g. when I update a node and set property label to undefined (I want to clear out the label) then the label is cleared, but the node remains the same size as with the label. If I set property label to undefined for an edge, then nothing happens.

To do the goofy way as I put yesterday (remove the node or edge from the DataSet, update the property with undefined and then add it back to the DataSet) then of course the labels are cleared, but the side effect is that the network is redrawn and now we have a bad user experience (especially with graphs with several nodes and edges).

Please, have a look at why it's impossible to clear out label/title with edges (not good with nodes either). The second alternative is to simply put a space as label/title, but then an annoying pop-up (for title) is displayed every time a mouse cursor hovers over, e.g. an edge. Not a good user experience either.

Thanks

@jan-johansson-mr
Copy link
Author

jan-johansson-mr commented May 22, 2020

Further examining the source code, maybe the problem is with this...

When the edge in the DataSet is updated, it's updated with undefined in the DataSet and everything is fine, however, when the network next gets updated (e.g. redrawn) it comes to the following checkpoint for validity of the label

// Only copy label if it's a legal value.
if (ComponentUtil.isValidLabel(newOptions.label)) {
  parentOptions.label = newOptions.label;
} else if (!ComponentUtil.isValidLabel(parentOptions.label)) {
  parentOptions.label = undefined;
}

Remember that I want to clear out the label, so the value of the label is undefined at this stage.

The call gets to the actual checking of the label, and here it only accepts type of string and further that we deal with a non-empty string (that is '' is not a valid string according to this checkpoint)

static isValidLabel(text) {
  // Note that this is quite strict: types that *might* be converted to string are disallowed
  return  (typeof text === 'string' && text !== '');
}

But if we back up in the call chain, we see that undefined is indeed a valid value (for clearing the label out), and specifically we see that at (where the label is actually for some condition set to undefined by the code).

Perhaps it would be good to do the following with the check for validity of the label:

static isValidLabel(text) {
  // Note that this is quite strict: types that *might* be converted to string are disallowed
  return (text === undefined) ||  (typeof text === 'string' && text !== '');
}

To enable the possibility of clearing out labels in the network.

Thanks
@mojoaxel and @Thomaash

@jan-johansson-mr
Copy link
Author

jan-johansson-mr commented May 22, 2020

Hi,

Doing the change as I earlier mentioned, breaks another code section that relies on isValidLabel, since it uses the label value for regular expression

    if (!ComponentUtil.isValidLabel(text)) {
      return this.lines.finalize();
    }

    var font = this.parent.fontOptions;

    // Normalize the end-of-line's to a single representation - order important
    text = text.replace(/\r\n/g, '\n');  // Dos EOL's
    text = text.replace(/\r/g, '\n');        // Mac EOL's

If adding the following check (and the change as put earlier)

    if ((text === undefined) || !ComponentUtil.isValidLabel(text)) {
      return this.lines.finalize();
    }

Then we have the ability to clear out the label, by setting the label to undefined.

The thing remaining is clearing out the title (the text that pops up when mouse hovering on the element).

Thanks
@mojoaxel

@Thomaash
Copy link
Member

I'd like to solve this in Vis Data. There really should be a way to remove props from items in data sets.

However the fact that undefined behaves differently for nodes and edges seems like a bug to me.

@jan-johansson-mr
Copy link
Author

Thanks @Thomaash,
I'll keep on using the goofy solution, until we have the ability of removing props from the data set. I took a quick look at your work, and it looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants