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.

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

Fixup pie/sunburst unhover post plot/calc restyle calls #3662

Merged
merged 2 commits into from
Mar 28, 2019
+186 −18
Merged

Fixup pie/sunburst unhover post plot/calc restyle calls #3662

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65c2678
fix visible true|false toggle for pie traces
etpinard Mar 22, 2019
e1084fa
fix #3618 edge for pie and sunburst
etpinard Mar 22, 2019
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
3 changes: 1 addition & 2 deletions src/traces/pie/base_plot.js
View file
Original file line number Diff line number Diff line change
@@ -16,8 +16,7 @@ exports.name = 'pie';
exports.plot = function(gd) {
var Pie = Registry.getModule('pie');
var cdPie = getModuleCalcData(gd.calcdata, Pie)[0];

if(cdPie.length) Pie.plot(gd, cdPie);
Pie.plot(gd, cdPie);
Copy link
Contributor

Choose a reason for hiding this comment

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

@etpinard Could you please elaborate why the condition check is no longer required here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we need Pie.plot to .exit().remove() the old pie traces.

};

exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
16 changes: 8 additions & 8 deletions src/traces/pie/plot.js
View file
Original file line number Diff line number Diff line change
@@ -313,12 +313,12 @@ function attachFxHandlers(sliceTop, gd, cd) {

// hover state vars
// have we drawn a hover label, so it should be cleared later
var hasHoverLabel = false;
if(!('_hasHoverLabel' in trace)) trace._hasHoverLabel = false;
// have we emitted a hover event, so later an unhover event should be emitted
// note that click events do not depend on this - you can still get them
// with hovermode: false or if you were earlier dragging, then clicked
// in the same slice that you moused up in
var hasHoverEvent = false;
if(!('_hasHoverEvent' in trace)) trace._hasHoverEvent = false;

sliceTop.on('mouseover', function(pt) {
// in case fullLayout or fullData has changed without a replot
@@ -390,33 +390,33 @@ function attachFxHandlers(sliceTop, gd, cd) {
gd: gd
});

hasHoverLabel = true;
trace._hasHoverLabel = true;
}

trace._hasHoverEvent = true;
gd.emit('plotly_hover', {
points: [eventData(pt, trace2)],
event: d3.event
});
hasHoverEvent = true;
});

sliceTop.on('mouseout', function(evt) {
var fullLayout2 = gd._fullLayout;
var trace2 = gd._fullData[trace.index];
var pt = d3.select(this).datum();

if(hasHoverEvent) {
if(trace._hasHoverEvent) {
evt.originalEvent = d3.event;
gd.emit('plotly_unhover', {
points: [eventData(pt, trace2)],
event: d3.event
});
hasHoverEvent = false;
trace._hasHoverEvent = false;
}

if(hasHoverLabel) {
if(trace._hasHoverLabel) {
Fx.loneUnhover(fullLayout2._hoverlayer.node());
hasHoverLabel = false;
trace._hasHoverLabel = false;
}
});

16 changes: 8 additions & 8 deletions src/traces/sunburst/plot.js
View file
Original file line number Diff line number Diff line change
@@ -530,12 +530,12 @@ function attachFxHandlers(sliceTop, gd, cd) {

// hover state vars
// have we drawn a hover label, so it should be cleared later
var hasHoverLabel = false;
if(!('_hasHoverLabel' in trace)) trace._hasHoverLabel = false;
// have we emitted a hover event, so later an unhover event should be emitted
// note that click events do not depend on this - you can still get them
// with hovermode: false or if you were earlier dragging, then clicked
// in the same slice that you moused up in
var hasHoverEvent = false;
if(!('_hasHoverEvent' in trace)) trace._hasHoverEvent = false;

sliceTop.on('mouseover', function(pt) {
var fullLayoutNow = gd._fullLayout;
@@ -603,33 +603,33 @@ function attachFxHandlers(sliceTop, gd, cd) {
gd: gd
});

hasHoverLabel = true;
trace._hasHoverLabel = true;
}

trace._hasHoverEvent = true;
gd.emit('plotly_hover', {
points: [makeEventData(pt, traceNow)],
event: d3.event
});
hasHoverEvent = true;
});

sliceTop.on('mouseout', function(evt) {
var fullLayoutNow = gd._fullLayout;
var traceNow = gd._fullData[trace.index];
var pt = d3.select(this).datum();

if(hasHoverEvent) {
if(trace._hasHoverEvent) {
evt.originalEvent = d3.event;
gd.emit('plotly_unhover', {
points: [makeEventData(pt, traceNow)],
event: d3.event
});
hasHoverEvent = false;
trace._hasHoverEvent = false;
}

if(hasHoverLabel) {
if(trace._hasHoverLabel) {
Fx.loneUnhover(fullLayoutNow._hoverlayer.node());
hasHoverLabel = false;
trace._hasHoverLabel = false;
}
});

102 changes: 102 additions & 0 deletions test/jasmine/tests/pie_test.js
View file
Original file line number Diff line number Diff line change
@@ -818,6 +818,26 @@ describe('Pie traces', function() {
.catch(failTest)
.then(done);
});

it('should be able to toggle visibility', function(done) {
var mock = Lib.extendDeep({}, require('@mocks/pie_title_multiple.json'));

function _assert(msg, exp) {
return function() {
var layer = d3.select(gd).select('.pielayer');
expect(layer.selectAll('.trace').size()).toBe(exp, msg);
};
}

Plotly.plot(gd, mock)
.then(_assert('base', 4))
.then(function() { return Plotly.restyle(gd, 'visible', false); })
.then(_assert('both visible:false', 0))
.then(function() { return Plotly.restyle(gd, 'visible', true); })
.then(_assert('back to visible:true', 4))
.catch(failTest)
.then(done);
});
});

describe('pie hovering', function() {
@@ -1384,3 +1404,85 @@ describe('pie relayout', function() {
.then(done);
});
});

describe('Test pie interactions edge cases:', function() {
var gd;

beforeEach(function() { gd = createGraphDiv(); });

afterEach(destroyGraphDiv);

function _mouseEvent(type, v) {
return function() {
var el = d3.select(gd).select('.slice:nth-child(' + v + ')').node();
mouseEvent(type, 0, 0, {element: el});
};
}

function hover(v) {
return _mouseEvent('mouseover', v);
}

function unhover(v) {
return _mouseEvent('mouseout', v);
}

it('should keep tracking hover labels and hover events after *calc* edits', function(done) {
var mock = Lib.extendFlat({}, require('@mocks/pie_simple.json'));
var hoverCnt = 0;
var unhoverCnt = 0;

// see https://github.com/plotly/plotly.js/issues/3618

function _assert(msg, exp) {
expect(hoverCnt).toBe(exp.hoverCnt, msg + ' - hover cnt');
expect(unhoverCnt).toBe(exp.unhoverCnt, msg + ' - unhover cnt');

var label = d3.select(gd).select('g.hovertext');
expect(label.size()).toBe(exp.hoverLabel, msg + ' - hover label cnt');

hoverCnt = 0;
unhoverCnt = 0;
}

Plotly.plot(gd, mock)
.then(function() {
gd.on('plotly_hover', function() {
hoverCnt++;
// N.B. trigger a 'calc' edit
Plotly.restyle(gd, 'textinfo', 'percent');
});
gd.on('plotly_unhover', function() {
unhoverCnt++;
// N.B. trigger a 'calc' edit
Plotly.restyle(gd, 'textinfo', null);
});
})
.then(hover(1))
.then(function() {
_assert('after hovering on first sector', {
hoverCnt: 1,
unhoverCnt: 0,
hoverLabel: 1
});
})
.then(unhover(1))
.then(function() {
_assert('after un-hovering from first sector', {
hoverCnt: 0,
unhoverCnt: 1,
hoverLabel: 0
});
})
.then(hover(2))
.then(function() {
_assert('after hovering onto second sector', {
hoverCnt: 1,
unhoverCnt: 0,
hoverLabel: 1
});
})
.catch(failTest)
.then(done);
});
});
67 changes: 67 additions & 0 deletions test/jasmine/tests/sunburst_test.js
View file
Original file line number Diff line number Diff line change
@@ -1031,3 +1031,70 @@ describe('Test sunburst tweening:', function() {
.then(done);
});
});

describe('Test sunburst interactions edge cases', function() {
var gd;

beforeEach(function() { gd = createGraphDiv(); });

afterEach(destroyGraphDiv);

it('should keep tracking hover labels and hover events after *calc* edits', function(done) {
var mock = Lib.extendFlat({}, require('@mocks/sunburst_first.json'));
var hoverCnt = 0;
var unhoverCnt = 0;

// see https://github.com/plotly/plotly.js/issues/3618

function _assert(msg, exp) {
expect(hoverCnt).toBe(exp.hoverCnt, msg + ' - hover cnt');
expect(unhoverCnt).toBe(exp.unhoverCnt, msg + ' - unhover cnt');

var label = d3.select(gd).select('g.hovertext');
expect(label.size()).toBe(exp.hoverLabel, msg + ' - hover label cnt');

hoverCnt = 0;
unhoverCnt = 0;
}

Plotly.plot(gd, mock)
.then(function() {
gd.on('plotly_hover', function() {
hoverCnt++;
// N.B. trigger a 'plot' edit
Plotly.restyle(gd, 'textinfo', 'none');
});
gd.on('plotly_unhover', function() {
unhoverCnt++;
// N.B. trigger a 'plot' edit
Plotly.restyle(gd, 'textinfo', null);
});
})
.then(hover(gd, 1))
.then(function() {
_assert('after hovering on first sector', {
hoverCnt: 1,
unhoverCnt: 0,
hoverLabel: 1
});
})
.then(unhover(gd, 1))
.then(function() {
_assert('after un-hovering from first sector', {
hoverCnt: 0,
unhoverCnt: 1,
hoverLabel: 0
});
})
.then(hover(gd, 2))
.then(function() {
_assert('after hovering onto second sector', {
hoverCnt: 1,
unhoverCnt: 0,
hoverLabel: 1
});
})
.catch(failTest)
.then(done);
});
});