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

Network: Fix handling of space before huge word in label text. #3470

Merged
merged 3 commits into from
Sep 22, 2017

Conversation

wimrijnders
Copy link
Contributor

Fixes #3464.

Changes

In the handling of label text, for the case of font.multi : false, spaces are now
'first class citizens', in the sense that they are treated as normal text.

Aside from solving the issue, this has the extra consequence that text width calculations
are now done correctly - previously, spaces could be ignored. This was a hidden bug.

Further:

  • DOS and Mac end of lines now taken into account, see LabelSplitter.process()
  • Added unit tests for the changes.

Cleanup

  • Moved LabelAccumulator to separate file.
  • Refactored code inLabelAccumulator related to height determination to separate method(s)
  • Refactored content of Label._processLabelText() to separate class LabelSplitter,
    the closures were only confusing.
  • Moved methods that deal with splitting of text from Label to LabelSplitter.
  • Added/adjusted commenting where deemed necessary.
  • Added unit tests for the handling of funky spaces in text.

@wimrijnders
Copy link
Contributor Author

Apologies if the change appears big. There was a lot of code movement due to refactoring - which I deemed necessary in order to make sense of the code.

@yotamberk yotamberk merged commit 9f7b1f9 into almende:develop Sep 22, 2017
@wimrijnders wimrijnders deleted the issue3464 branch September 22, 2017 14:41
primozs pushed a commit to primozs/vis that referenced this pull request Jan 3, 2019
…de#3470)

* Refactoring of label splitting; new unit test still failing.

* Label acc size calculations to separate methods.

* Final fixes and cleanup
mojoaxel added a commit to visjs/vis-network that referenced this pull request Jul 17, 2019
via @macleodbroad-wf, @wimrijnders
(almende/vis#3228, almende/vis#3470, almende/vis#3486, almende/vis#3511, almende#3520) , almende#3518, almende#3575, almende#3565, almende#3603, almende#3646)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants