Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Fix: Network nodes ignoring widthConstraint after hover #4154

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

moppius
Copy link

@moppius moppius commented Oct 11, 2018

There is an issue where the widthConstraint value of a node appears to be ignored after the mouse has hovered over a node.

I tracked this down to the ctx not being initialized correctly the second time the nodes are refreshed, leading to an incorrect font face and size being supplied to the method that splits long text strings, which causes the measurements for the calculated pixel size of the string to be entirely incorrect.

This bug has been reported in a few different issues - see the discussion thread on this #3928 for my investigation and reasoning for the fix. I basically just pass the context through in another couple of places to ensure it's set before the string split is calculated.

I've added an example HTML file to validate the issue as hoverLabelSplitterEdgeCase.html in the existing test/network/ folder.

This PR fixes #3872 and fixes #3928 (although one should probably be closed as a dupe).

This fixes labels being split along the wrong lines. It doesn't take
font modifiers into account, though, so still might be wrong in some
circumstances. Just less wrong than before.

Fixes almende#3928 for the most part. Arrowheads on edges can still be slightly
misaligned in some cases.
Example test page for validating bug regression.
@@ -735,7 +735,7 @@ class Label {
state.width = this.fontOptions.minWdt;
}

this.size.labelHeight =state.height;
this.size.labelHeight = state.height;
Copy link
Author

Choose a reason for hiding this comment

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

This isn't vital to the bug fix, but it was an inconsistency from the coding standard that I figured I should fix, since I was editing around there anyway.

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

Successfully merging this pull request may close these issues.

2 participants