Skip to content

Commit

Permalink
Merge pull request #4454 from plotly/sunburst-better-click-events
Browse files Browse the repository at this point in the history 
Improve sunbust and treemap click events behavior
  • Loading branch information
@etpinard
etpinard authored Jan 2, 2020
2 parents be93eb6 + 1df563c commit 968054c
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 60 deletions.
45 changes: 23 additions & 22 deletions src/traces/sunburst/fx.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,27 +217,33 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {
var fullLayoutNow = gd._fullLayout;
var traceNow = gd._fullData[trace.index];

var clickVal = Events.triggerHandler(gd, 'plotly_' + trace.type + 'click', {
var noTransition = isSunburst && (helpers.isHierarchyRoot(pt) || helpers.isLeaf(pt));

var id = helpers.getPtId(pt);
var nextEntry = helpers.isEntry(pt) ?
helpers.findEntryWithChild(hierarchy, id) :
helpers.findEntryWithLevel(hierarchy, id);
var nextLevel = helpers.getPtId(nextEntry);

var typeClickEvtData = {
points: [makeEventData(pt, traceNow, opts.eventDataKeys)],
event: d3.event
});
};
if(!noTransition) typeClickEvtData.nextLevel = nextLevel;

// 'regular' click event when sunburst/treemap click is disabled or when
// clicking on leaves or the hierarchy root
if(
clickVal === false ||
isSunburst && (
helpers.isHierarchyRoot(pt) ||
helpers.isLeaf(pt)
)
) {
if(fullLayoutNow.hovermode) {
gd._hoverdata = [makeEventData(pt, traceNow, opts.eventDataKeys)];
Fx.click(gd, d3.event);
}
return;
var clickVal = Events.triggerHandler(gd, 'plotly_' + trace.type + 'click', typeClickEvtData);

if(clickVal !== false && fullLayoutNow.hovermode) {
gd._hoverdata = [makeEventData(pt, traceNow, opts.eventDataKeys)];
Fx.click(gd, d3.event);
}

// if click does not trigger a transition, we're done!
if(noTransition) return;

// if custom handler returns false, we're done!
if(clickVal === false) return;

// skip if triggered from dragging a nearby cartesian subplot
if(gd._dragging) return;

Expand All @@ -251,13 +257,8 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {
level: traceNow.level
});

var id = helpers.getPtId(pt);
var nextEntry = helpers.isEntry(pt) ?
helpers.findEntryWithChild(hierarchy, id) :
helpers.findEntryWithLevel(hierarchy, id);

var frame = {
data: [{level: helpers.getPtId(nextEntry)}],
data: [{level: nextLevel}],
traces: [trace.index]
};

Expand Down
52 changes: 33 additions & 19 deletions test/jasmine/tests/sunburst_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -811,11 +811,16 @@ describe('Test sunburst clicks:', function() {
if(trackers.sunburstclick.length === 1) {
expect(trackers.sunburstclick[0].event).toBeDefined();
expect(trackers.sunburstclick[0].points[0].label).toBe('Seth');
expect(trackers.sunburstclick[0].nextLevel).toBe('Seth');
} else {
fail('incorrect plotly_sunburstclick triggering');
}

if(trackers.click.length) {
if(trackers.click.length === 1) {
expect(trackers.click[0].event).toBeDefined();
expect(trackers.click[0].points[0].label).toBe('Seth');
expect(trackers.click[0].nextLevel).not.toBeDefined();
} else {
fail('incorrect plotly_click triggering');
}

Expand Down Expand Up @@ -888,16 +893,6 @@ describe('Test sunburst clicks:', function() {
it('should not trigger animation when graph is transitioning', function(done) {
var mock = Lib.extendDeep({}, require('@mocks/sunburst_first.json'));

// should be same before and after 2nd click
function _assertCommon(msg) {
if(trackers.click.length) {
fail('incorrect plotly_click triggering - ' + msg);
}
if(trackers.animating.length !== 1) {
fail('incorrect plotly_animating triggering - ' + msg);
}
}

Plotly.plot(gd, mock)
.then(setupListeners())
.then(click(gd, 2))
Expand All @@ -907,27 +902,49 @@ describe('Test sunburst clicks:', function() {
if(trackers.sunburstclick.length === 1) {
expect(trackers.sunburstclick[0].event).toBeDefined(msg);
expect(trackers.sunburstclick[0].points[0].label).toBe('Seth', msg);
expect(trackers.sunburstclick[0].nextLevel).toBe('Seth', msg);
} else {
fail('incorrect plotly_sunburstclick triggering - ' + msg);
}

_assertCommon(msg);
if(trackers.click.length === 1) {
expect(trackers.click[0].event).toBeDefined(msg);
expect(trackers.click[0].points[0].label).toBe('Seth', msg);
expect(trackers.click[0].nextLevel).not.toBeDefined(msg);
} else {
fail('incorrect plotly_click triggering - ' + msg);
}

if(trackers.animating.length !== 1) {
fail('incorrect plotly_animating triggering - ' + msg);
}
})
.then(click(gd, 4))
.then(function() {
var msg = 'after 2nd click';

// should trigger plotly_sunburstclick twice, but not additional
// plotly_click nor plotly_animating
// should trigger plotly_sunburstclick and plotly_click twice,
// but not plotly_animating

if(trackers.sunburstclick.length === 2) {
expect(trackers.sunburstclick[0].event).toBeDefined(msg);
expect(trackers.sunburstclick[0].points[0].label).toBe('Awan', msg);
expect(trackers.sunburstclick[0].nextLevel).toBe('Awan', msg);
} else {
fail('incorrect plotly_sunburstclick triggering - ' + msg);
}

_assertCommon(msg);
if(trackers.click.length === 2) {
expect(trackers.click[0].event).toBeDefined(msg);
expect(trackers.click[0].points[0].label).toBe('Awan', msg);
expect(trackers.click[0].nextLevel).not.toBeDefined(msg);
} else {
fail('incorrect plotly_click triggering - ' + msg);
}

if(trackers.animating.length !== 1) {
fail('incorrect plotly_animating triggering - ' + msg);
}
})
.catch(failTest)
.then(done);
Expand All @@ -947,10 +964,7 @@ describe('Test sunburst clicks:', function() {
fail('incorrect plotly_sunburstclick triggering');
}

if(trackers.click.length === 1) {
expect(trackers.click[0].event).toBeDefined();
expect(trackers.click[0].points[0].label).toBe('Seth');
} else {
if(trackers.click.length !== 0) {
fail('incorrect plotly_click triggering');
}

Expand Down
53 changes: 34 additions & 19 deletions test/jasmine/tests/treemap_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1042,11 +1042,16 @@ describe('Test treemap clicks:', function() {
if(trackers.treemapclick.length === 1) {
expect(trackers.treemapclick[0].event).toBeDefined();
expect(trackers.treemapclick[0].points[0].label).toBe('Seth');
expect(trackers.treemapclick[0].nextLevel).toBe('Seth');
} else {
fail('incorrect plotly_treemapclick triggering');
}

if(trackers.click.length) {
if(trackers.click.length === 1) {
expect(trackers.click[0].event).toBeDefined();
expect(trackers.click[0].points[0].label).toBe('Seth');
expect(trackers.click[0].nextLevel).not.toBeDefined();
} else {
fail('incorrect plotly_click triggering');
}

Expand Down Expand Up @@ -1084,16 +1089,6 @@ describe('Test treemap clicks:', function() {
it('should not trigger animation when graph is transitioning', function(done) {
var mock = Lib.extendDeep({}, require('@mocks/treemap_first.json'));

// should be same before and after 2nd click
function _assertCommon(msg) {
if(trackers.click.length) {
fail('incorrect plotly_click triggering - ' + msg);
}
if(trackers.animating.length !== 1) {
fail('incorrect plotly_animating triggering - ' + msg);
}
}

Plotly.plot(gd, mock)
.then(setupListeners())
.then(click(gd, 2))
Expand All @@ -1103,27 +1098,50 @@ describe('Test treemap clicks:', function() {
if(trackers.treemapclick.length === 1) {
expect(trackers.treemapclick[0].event).toBeDefined(msg);
expect(trackers.treemapclick[0].points[0].label).toBe('Seth', msg);
expect(trackers.treemapclick[0].nextLevel).toBe('Seth', msg);
} else {
fail('incorrect plotly_treemapclick triggering - ' + msg);
}

_assertCommon(msg);
if(trackers.click.length === 1) {
expect(trackers.click[0].event).toBeDefined(msg);
expect(trackers.click[0].points[0].label).toBe('Seth', msg);
expect(trackers.click[0].nextLevel).not.toBeDefined(msg);
} else {
fail('incorrect plotly_click triggering - ' + msg);
}

if(trackers.animating.length !== 1) {
fail('incorrect plotly_animating triggering - ' + msg);
}
})
.then(click(gd, 4))
.then(function() {
var msg = 'after 2nd click';

// should trigger plotly_treemapclick twice, but not additional
// plotly_click nor plotly_animating
// should trigger plotly_treemapclick and plotly_click twice,
// but not plotly_animating

if(trackers.treemapclick.length === 2) {
expect(trackers.treemapclick[0].event).toBeDefined(msg);
expect(trackers.treemapclick[0].points[0].label).toBe('Awan', msg);
expect(trackers.treemapclick[0].nextLevel).toBe('Awan', msg);
} else {
fail('incorrect plotly_treemapclick triggering - ' + msg);
}

_assertCommon(msg);
if(trackers.click.length === 2) {
expect(trackers.click[0].event).toBeDefined(msg);
expect(trackers.click[0].points[0].label).toBe('Awan', msg);
expect(trackers.click[0].nextLevel).not.toBeDefined(msg);
} else {
fail('incorrect plotly_click triggering - ' + msg);
}


if(trackers.animating.length !== 1) {
fail('incorrect plotly_animating triggering - ' + msg);
}
})
.catch(failTest)
.then(done);
Expand All @@ -1143,10 +1161,7 @@ describe('Test treemap clicks:', function() {
fail('incorrect plotly_treemapclick triggering');
}

if(trackers.click.length === 1) {
expect(trackers.click[0].event).toBeDefined();
expect(trackers.click[0].points[0].label).toBe('Seth');
} else {
if(trackers.click.length !== 0) {
fail('incorrect plotly_click triggering');
}

Expand Down

0 comments on commit 968054c

Please sign in to comment.