Skip to content

Merging #3662 into master #3690

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 3 commits into from
Mar 28, 2019
Merged

Merging #3662 into master #3690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 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
a7561ec
Merge pull request #3662 from plotly/pie-sunburst-unhover-post-restyle
etpinard Mar 28, 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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
Expand Down
16 changes: 8 additions & 8 deletions src/traces/pie/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
});

Expand Down
16 changes: 8 additions & 8 deletions src/traces/sunburst/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,12 +529,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;
Expand Down Expand Up @@ -602,33 +602,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;
}
});

Expand Down
102 changes: 102 additions & 0 deletions test/jasmine/tests/pie_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1053,3 +1053,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);
});
});