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

Getting height of label in scriptable options? #43

Open
sect2k opened this issue Mar 22, 2018 · 8 comments
Open

Getting height of label in scriptable options? #43

sect2k opened this issue Mar 22, 2018 · 8 comments
Labels

Comments

@sect2k
Copy link

sect2k commented Mar 22, 2018

I'm looking to implement functionality that will display label inside or outside of bar, depending on how much space is available. The way I've set to do this is by passing a function to align and then doing some calculations based on x,y coordinates of individual charts and chart size.

The sample code below works, but to it uses a fixed threshold, where I would like to compute it based on label width/height.

datalabels: {
    align: context => {
        let meta = context.chart.getDatasetMeta(context.datasetIndex),
            bar = meta.data[context.dataIndex]._model,
            threshold = 15
            // label = context.chart['$datalabels'].labels[context.datasetIndex][context.dataIndex]._model;
        
        if (bar.y + threshold > context.chart.chartArea.bottom) {
            return 'end'
        }
        return 'start'
    }
}

I've tried using label (commented in above code) to try and get sizing info, but all seem to be null on first pass (_model, _hitbox._rect, ...)

Any ideas, suggestions? Thanks.

@simonbrunel
Copy link
Member

It's not possible because the label effective bounding box is computed after all the options are fully evaluated. The threshold approach is not so bad: simple, efficient, predictable and works in most use cases. I'm very careful when it comes to expose/extend public APIs, so unfortunately I don't have idea to suggest right now.

I notice that you are trying to manipulate chart.$datalabels, _model, ... properties: these are private members (starting by $ or _) and I would highly discourage accessing them. No backward compatibility and it can change in patch and minor versions, breaking your implementation. Actually, I may prefix all chart.$datalabels.* properties by _ to insist on the their private nature.

@sect2k
Copy link
Author

sect2k commented Mar 23, 2018

Thank for the info. Is there a particular reason why this calculation is deferred? Could it be calculated immediately? I'm not very versed with canvas but if there is no technical limit, that prevents this, I'd be willing to tackle it.

While I agree that the threshold approach is simple, I'm not sure I would call it clean and effective, since it can break with many factors (font size change, resize, etc,...).

Lastly, I understand that those variables are private, I was just looking a way to get the label box sizing while playing around and are otherwise fully aware of the consequences of using them.

@simonbrunel
Copy link
Member

simonbrunel commented Mar 23, 2018

Is there a particular reason why this calculation is deferred?

Yes. As you said, many options impact the bounding box (font, padding, etc.) and it would be a waste of time to compute geometry every time we evaluate a single option. Computed sizes would be inconsistent between 2 scriptable options (depending on the evaluation order), so absolutely not reliable. Finally, when options will be animable (if that's possible), the bounding box could change at every animation frame (e.g. when animating the padding), but options are evaluated one time, before the animation.

I'm not sure I would call it clean and effective ...

If you want to handle all possible option variants, then I agree, a threshold is not the right choice.

@sect2k
Copy link
Author

sect2k commented Mar 23, 2018

Does it have be evaluated every time, wouldn't it be possible to compute the final dimensions ahead of time and pass them as part of context?

To give you a better picture, what I'm aiming to do is have "smart" labels that know if there is enough room to be rendered inside the bar, and if not, render them outside. Since I don't know the exact length of text inside the label ahead of time, using a fixed threshold becomes a mess. Also I would like this to work for bar and horizontalBar alike.

Computed sizes would be inconsistent between 2 scriptable options (depending on the evaluation order)

What determines the above mentioned evaluation order? So far I have used context, to store computed values for subsequent scriptable options without issue?

@simonbrunel
Copy link
Member

wouldn't it be possible to compute the final dimensions ahead of time ...

No, because how would you resolve the following case:

datalabels: {
  padding: function(context) {
     // What's the value of context.theLabelFinalGeometry ?
     return 8;
  },
  font: function(context) {
     // What's the value of context.theLabelFinalGeometry ?
     return { size: 42 };
  }
}

What determines the above mentioned evaluation order?

The evaluation order is officially undefined and can change in future versions, that's why the context should not depend on any order, it must be stable.

I have used context, to store computed values for subsequent scriptable options without issue.

datalabels: {
  align: function(context) {
     context.bar = true;
     return context.foo === true ? 'start' : 'end';
  },
  anchor: function(context) {
     context.foo = true;
     return context.bar === true ? 'start' : 'end';
  }
}

What's the value of align and anchor? Looking at the code, anchor is evaluated after align, so in this case: align = 'end' and anchor = 'start'. But we can decide to evaluate anchor before align for any internal reason, which would break your configuration.

@sect2k
Copy link
Author

sect2k commented Mar 23, 2018

OK, so clearly using scriptable options to tackle this issue is not the way forward. What would alternative approaches be?

Could callbacks work? Something like this

datalabels: {
    callbacks: {
        beforeDraw: function(label) {
            // modify label state as needed
            return label // or false|null|undefined to not draw the label
        }
    }
}

Quickly looking at the code in plugin.js and label.js it should be possible or am I missing something.

// plugin.js
function drawLabels(chart, datasetIndex) {
    for (...) {
        label = el[EXPANDO_KEY];
        if (label &&  callback) {
            label = callback(label)
        }
        if (label) {
            label.draw(chart.ctx);
        }
    }
}

@simonbrunel
Copy link
Member

label is private and will not be exposed as-is since I don't want to give full control over our internals and be locked with implementation details. callbacks in the options is not ideal, neither flexible, a plugin approach would maybe be better. I need to think about it but I don't have time right now to investigate this request deeper, so will keep that ticket open and follow up if an elegant API comes to my mind.

@sect2k
Copy link
Author

sect2k commented Mar 23, 2018

I think plugins for a plugin is a bit of an overkill, maybe a better approach to this, coming from a more object oriented world, would be to refactor the plugin to be a set of es6 classes which can then be extended if needed. It would also resolve #14.

Then in my case I could just import the module, extend it and override the draw method to do what I want. The internal API is not exposed, since I'm doing the override on my own module and there is no need for an elaborate plugin API or callbacks.

Anyways, thanks for your feedback, since this is something I'm very interested in, let me know if there is anything I can do to help get things moving along.

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

No branches or pull requests

2 participants