Add overflow option to labels configuration

[stockiNail]

Fix #105

This is PR is adding new option to labels configuration in order to control what happens to a label that is too big to fit into a rectangle.

Name	Type	Default
overflow	'cut'|'hidden'	'cut'

The option can be set to:

• 'cut' (default and the same behavior before this PR) if the label is too big, it will be cut to stay inside the rectangle.
• 'hidden', the label is removed altogether if the rectangle is too small for it

A boolean is not used because the option could also have another value, 'auto', addressing the issue #67

TODO

• test cases
• type definitions update
• documentation

[stockiNail]

@kurkle off-topic: I see that some files are growing (ie elements). Could make sense to create an helpers folder (like annotation plugin) and move over there some functions?

[kurkle]

should use spriting on fixtures, they don't work in different environments without that.

[stockiNail]

should use spriting on fixtures, they don't work in different environments without that.

I wanted but I have seen that it wasn't used in many other fixtures... What do you think to update all test cases in a specific PR?

[stockiNail]

should use spriting on fixtures, they don't work in different environments without that.

I did some tests using spriteText: true but I have a predicable result and errors on the tests. Maybe I miss something

[kurkle]

needs a rebase. btw, thanks for your awesome work! :)

[stockiNail]

needs a rebase.

rebased, adding spriteText on overflow tests.

thanks for your awesome work! :)

Thank you too! Always a pleasure to collaborate, I can learn many things!

[stockiNail]

thanks for your awesome work! :)

@kurkle I have prepared a TODO list of items to implement in the controller, related to the label drawing (and some which should fix some open issues) but also other stuff, like the restructuring with helpers. Make sense to you? Or do you prefer to stay "simple"?

[kurkle]

thanks for your awesome work! :)

@kurkle I have prepared a TODO list of items to implement in the controller, related to the label drawing (and some which should fix some open issues) but also other stuff, like the restructuring with helpers. Make sense to you? Or do you prefer to stay "simple"?

Restructuring is ok and welcome :)

It would be really nice if the labels could be handled completely with datalabels plugin, was there some issues with that?

Anyway, if datalabels does not cut it (with minor changes maybe?), then I think its ok to start developing all the label needs here.

The "minor changes" I stated above, comes from me recalling datalabels having some logic based on element type..

[stockiNail]

Restructuring is ok and welcome :)

PR #117

It would be really nice if the labels could be handled completely with datalabels plugin, was there some issues with that?

The main issue that I see it's related to the label for groups. To do that, you need to create the enough space to draw the label (reducing the sub rectangle to pass recursively). You need to know font, padding and the text to draw and we should go to the plugin options to get them.

Anyway, if datalabels does not cut it (with minor changes maybe?), then I think its ok to start developing all the label needs here.

The cutting couldn't be an issue because datalabels plugin can be configured to disable the label if they are overflowing (display option, if I remember well). And the issue #67 could be addressed with the rotation plugin options, via scriptable options.

The "minor changes" I stated above, comes from me recalling datalabels having some logic based on element type..

You recall correctly but there is the default (when the element doesn't match) and it work for this controller. But I should go more in deep about fallback.

function getPositioner(el) {
  if (el instanceof ArcElement) {
    return positioners.arc;
  }
  if (el instanceof PointElement) {
    return positioners.point;
  }
  if (el instanceof BarElement) {
    return positioners.bar;
  }
  return positioners.fallback;
}

[stockiNail]

To be more precise, here is the code in datalabels plugin for fallback case:

  fallback: function(el, config) {
    var v = orient(el, config.origin);


    return compute({
      x0: el.x,
      y0: el.y,
      x1: el.x + (el.width || 0),
      y1: el.y + (el.height || 0),
      vx: v.x,
      vy: v.y
    }, config);
  }
};

[stockiNail]

What datalabels plugin is not supporting is what reported in issue #92. This should be closed as "not doable". Instead, having internally, we can change the text drawing (and size calculation) taking care about more fonts and colors.

[kurkle]

Ok then, will have to keep labels internal (at least group labels, and I guess that will cover most of the code for other labels too)

[stockiNail]

Ok then, will have to keep labels internal (at least group labels, and I guess that will cover most of the code for other labels too)

That's what I thought. Of course, who wants to use datalabels can do it (for labels, no groups)

[stockiNail]

@kurkle if I may, off topic, the borderWidth as object is different form the other plugins and controllers where a "simple" borderWidth is implemented. Do you know if this was required by anyone? Having a look to the history, I didn't find it.

[kurkle]

@kurkle if I may, off topic, the borderWidth as object is different form the other plugins and controllers where a "simple" borderWidth is implemented. Do you know if this was required by anyone? Having a look to the history, I didn't find it.

I think I needed in somewhere.. but not anymore. BTW, just renamed next to main.

[stockiNail]

BTW, just renamed next to main.

I have never done... I'm still master in my projects. ;) Let me have a look how to do it, without destroying everything

[stockiNail]

You have already done! ;)

[stockiNail]

You have already done! ;)

what about existing PR? I think I need to fork again with main.

[stockiNail]

You have already done! ;)

what about existing PR? I think I need to fork again with main.

Changed my branch name in main

[kurkle]

You have already done! ;)

what about existing PR? I think I need to fork again with main.

existing pr's should have been updated

[kurkle]

testing actions a bit with this, so closing / reopening (at least once)

[stockiNail]

@kurkle I don't want to bore you and forgive me but maybe we could change the "Main branch" in SonarCloud, from "next" to main". https://community.sonarsource.com/t/how-to-change-main-branch/1167

[kurkle]

already did :)