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

Tooltip xAlign miscalculated when yAlign set to 'center' via options #5693

Closed
maritaria opened this issue Aug 16, 2018 · 4 comments · Fixed by #8646
Closed

Tooltip xAlign miscalculated when yAlign set to 'center' via options #5693

maritaria opened this issue Aug 16, 2018 · 4 comments · Fixed by #8646

Comments

@maritaria
Copy link

According to the alignment calculation code the tooltip will automatically align itself to bottom if it sticks out of the bottom of the graph. However, when using yAlign = 'center' in your tooltip config then the test is not correct because the final y-position of the tooltip is altered later here.
This test should include this alteration.

My case:

  • Have a tooltip with a height > half the graph
  • Set the tooltip to align to the center of the graph (using a positioner)
  • set yAlign to center

Result:

The tooltip will be y-aligned to center but the xAlign behaves as if the yAlign is set to 'bottom'.
When yAlign is set to 'center' the tooltip should have an xAlign of only 'left' or 'right', but when the mentioned test 'corrects' the yAlign to 'bottom' then the xAlign will become center and hover over the tooltip-ed value.

I can't write a full reproduction/fiddle right now but here are some settings I use in my situation:

                    tooltips: {
                        enabled: true,
                        mode: 'index',
                        axis: 'x',
                        intersect: false,
                        yAlign: 'center',
                        position: 'VerticalCenterPositioner',
                        xPadding: 24,
                        yPadding: 24,
                        BackgroundColor: 'rgba(0,0,0,0.9)',
                        titleFontFamily: 'Lato',
                        titleFontSize: 12,
                        titleFontWeight: 'regular',
                        titleSpacing: 20,
                        titleFontColor: '#ccc',
                        titleMarginBottom: 16,
                        bodyFontSize: 16,
                        bodyFontColor: '#fff',
                        bodyFontFamily: 'Lato',
                        caretSize: 16,
                        caretPadding: 8,
                        cornerRadius: 3,
                        bodySpacing: 10
                    },
                    scales: {
                        yAxes: [{
                            type: 'linear',
                            ticks: {
                                beginAtZero: true,
                                min: 0,
                                suggestedMax: 1
                            }
                        }],
                        xAxes: [{
                            type: 'time',
                            ticks: {
                                autoSkip: true,
                                maxTicksLimit: 11,
                            },
                            time: {
                                unit: 'day',
                                tooltipFormat: 'll'
                            },
                        }]
                    },

Positioner:

    Chart.Tooltip.positioners.VerticalCenterPositioner = function (elements) {
        // Because the tooltip scrolls over the x-axis all elements are vertically aligned.
        const element = elements[0];
        const chart = element._chart;
        return {
            x: element.tooltipPosition().x,
            y: (chart.chartArea.bottom - chart.chartArea.top) / 2,
        };
    };

And then for data I have something along the lines of:

    const count = 3; // lower numbers work fine, higher dont
    this.labels = _.times(12, i => this.labels.push(new Date(2018, 0, i)));
    this.datasets = _.times(count, i => ({
        data: this.labels.map(() => _.random(1, 10)),
        label: 'set' + i,
        fill: false,
        enabled: true,
    }));

When you have a vertically small graph (say 200px) then you can easily find a dataset count where the tooltip fits on the screen but xAlign misbehaves.

Screenshots

screen shot 2018-08-16 at 12 02 11

screen shot 2018-08-16 at 12 02 27

@maritaria
Copy link
Author

I understand this may be an edge-case (no pun intended) but a workable solution for me would be to put those functions used by Tooltip.update on the class/object instead of having them as local functions only. Currently the update() method does a lot of work with no possibility to override a smaller section of the behaviour

@maritaria
Copy link
Author

I ended up fixing this for my project be copying over the entire code for Chart.Tooltip.prototype.update and adding a fix for it there.

@etimberg etimberg changed the title xAlign miscalculated when yAlign = center xAlign miscalculated when yAlign set to 'center' via options Oct 17, 2020
@etimberg etimberg changed the title xAlign miscalculated when yAlign set to 'center' via options Tooltip xAlign miscalculated when yAlign set to 'center' via options Oct 17, 2020
@kurkle
Copy link
Member

kurkle commented Mar 10, 2021

still happens: https://codepen.io/kurkle/pen/BaQGEdK

@etimberg
Copy link
Member

Did some investigation into this. It's due to https://github.com/chartjs/Chart.js/blob/master/src/plugins/plugin.tooltip.js#L266-L267 where the option alignments are only considered at the end.

There seem to be 4 possible cases:

xAlign yAlign Outcome
undefined undefined Best alignment determined
set undefined Choose best yAlign for the xAlign
undefined set Choose best xAlign for the yAlign
set set Use user supplied options

I would advocate that we keep try and keep this logic small. It'll be easy to make it very complex, but with the full scriptable options users can supply their own methods to determine the alignment based on the data.

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

Successfully merging a pull request may close this issue.

3 participants