Skip to content

disable hover for sankey traces when hovermode is false #2949

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 29, 2018
Merged

disable hover for sankey traces when hovermode is false #2949

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
edc20b0
disable hover when hovermode is false
antoinerg Aug 28, 2018
45f06c4
sankey test that labels are not shown when hovermode is false
antoinerg Aug 28, 2018
98c18f8
clean up sankey test
antoinerg Aug 28, 2018
aafa886
bail out of the unhover handlers if hovermode is false
antoinerg Aug 28, 2018
3d32b6f
sankey test: adding a hovermode: false case to the click/hover events
antoinerg Aug 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions test/jasmine/tests/sankey_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,28 @@ describe('sankey tests', function() {
.catch(failTest)
.then(done);
});

it('should not show labels if hovermode is false', function(done) {
var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);

function _hover(px, py) {
mouseEvent('mousemove', px, py);
mouseEvent('mouseover', px, py);
Lib.clearThrottle();
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we push this to an outer scope so we're not just repeating the function from above?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh I see, it was already duplicated - great, we can clean them all up into just one _hover


Plotly.plot(gd, mockCopy).then(function() {
return Plotly.relayout(gd, 'hovermode', false);
})
.then(function() {
_hover(404, 302);

assertNoLabel();
})
.catch(failTest)
.then(done);
});
});

describe('Test hover/click event data:', function() {
Expand Down Expand Up @@ -620,3 +642,8 @@ function assertLabel(content, style) {
fontColor: style[4]
});
}

function assertNoLabel() {
var g = d3.selectAll('.hovertext');
expect(g[0].length).toBe(0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

g.size() is better than g[0].length

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

}