Skip "hoverinfo": "none" trace display for hover modes

[Domino987]

Fixes #5516 5516

As documented, hoverinfo: none should trigger hover events, but should not be displayed in the hover field.

Currently, if the hovermode is "x/y unified", it groups the labels together but does not skip over the 'hoverinfo': 'none' data.
It skips the text resulting in an empty row.

This PR skips the 'hoverinfo': 'none' data for the labels so that the events are still triggerd but the empty row is not displayed.

[archmoj]

Thanks very much for the PR.
I noticed some tests failed.
You could run the click_test locally using the command below.

npm run test-jasmine click

Also please see my other comments below. Hope that may help fix the problem.

[archmoj]

@Domino987 how about picking these two commits: b0eb6db and c52953d
which you can find on this dev branch: https://github.com/plotly/plotly.js/compare/pull-5854_dev ?

[archmoj]

Now let's add a test somewhere in this block:

plotly.js/test/jasmine/tests/hover_label_test.js

Line 4482 in 9c06996

describe('hovermode: (x|y)unified', function() {

[archmoj]

All the tests used to pass at 3fe967f commit. Why they started failing after you added new test? Please revert the changes you made after that commit and only keep the new test.

[Domino987]

That's because the code was not correct because the first header was still taken from the first entry even if this was hoverinfo = "skip". I fixed the code and new tests failed and i am still looking into it.

[archmoj]

That's because the code was not correct because the first header was still taken from the first entry even if this was hoverinfo = "skip". I fixed the code and new tests failed and i am still looking into it.

Thank you! I added this PR to the next milestone which is due for August 26, 2021. In case you were able to get this to the finish line we could include it in the v2.4.0 release.

[archmoj]

This test fails on CircleCI. Does it pass on your machine?

[Domino987]

No it does not and I have problems with the tests in the first place on my machine, since its a windows.
But yes, that should work.

[archmoj]

The test is not passing.
Please simply replace it with

it('should not display hover for display: none', function(done) {
        Plotly.newPlot(gd, {
            data: [{
                name: 'A',
                y: [1]
            }, {
                name: 'B',
                y: [2],
                hoverinfo: 'none'
            }],
            layout: {
                hovermode: 'x unified',
                showlegend: false,
                width: 500,
                height: 500,
                margin: {
                    t: 50,
                    b: 50,
                    l: 50,
                    r: 50
                }
            }
        })
        .then(function() {
            _hover(gd, { xpx: 200, ypx: 200 });
            assertLabel({title: '0', items: [
                'A : 1'
            ]});
        })
        .then(done, done.fail);
    });

[archmoj]

@Domino987 Could you please adjust the test as mentioned above? It looks like the only remaining item here.

[archmoj]

Also please create draftlog/5854_fix.md file as described here for example

 - Fix to include traces with "none" `hoverinfo` in event data [[#5854](https://github.com/plotly/plotly.js/pull/5854)],
   with thanks to @Domino987 for the contribution!

[Domino987]

Also please create draftlog/5854_fix.md file as described here for example

 - Fix to include traces with "none" `hoverinfo` in event data [[#5854](https://github.com/plotly/plotly.js/pull/5854)],
   with thanks to @Domino987 for the contribution!

I thought I did here ?

[archmoj]

Also please create draftlog/5854_fix.md file as described here for example

 - Fix to include traces with "none" `hoverinfo` in event data [[#5854](https://github.com/plotly/plotly.js/pull/5854)],
   with thanks to @Domino987 for the contribution!

I thought I did here ?

That's correct.

[archmoj]

💃