Skip to content

Commit 968054c

Browse files
etpinardetpinard
authored
Merge pull request #4454 from plotly/sunburst-better-click-events
Improve sunbust and treemap click events behavior
2 parents be93eb6 + 1df563c commit 968054c

File tree

3 files changed

+90
-60
lines changed

3 files changed

+90
-60
lines changed

‎src/traces/sunburst/fx.js‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -217,27 +217,33 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {
217217
var fullLayoutNow = gd._fullLayout;
218218
var traceNow = gd._fullData[trace.index];
219219

220-
var clickVal = Events.triggerHandler(gd, 'plotly_' + trace.type + 'click', {
220+
var noTransition = isSunburst && (helpers.isHierarchyRoot(pt) || helpers.isLeaf(pt));
221+
222+
var id = helpers.getPtId(pt);
223+
var nextEntry = helpers.isEntry(pt) ?
224+
helpers.findEntryWithChild(hierarchy, id) :
225+
helpers.findEntryWithLevel(hierarchy, id);
226+
var nextLevel = helpers.getPtId(nextEntry);
227+
228+
var typeClickEvtData = {
221229
points: [makeEventData(pt, traceNow, opts.eventDataKeys)],
222230
event: d3.event
223-
});
231+
};
232+
if(!noTransition) typeClickEvtData.nextLevel = nextLevel;
224233

225-
// 'regular' click event when sunburst/treemap click is disabled or when
226-
// clicking on leaves or the hierarchy root
227-
if(
228-
clickVal === false ||
229-
isSunburst && (
230-
helpers.isHierarchyRoot(pt) ||
231-
helpers.isLeaf(pt)
232-
)
233-
) {
234-
if(fullLayoutNow.hovermode) {
235-
gd._hoverdata = [makeEventData(pt, traceNow, opts.eventDataKeys)];
236-
Fx.click(gd, d3.event);
237-
}
238-
return;
234+
var clickVal = Events.triggerHandler(gd, 'plotly_' + trace.type + 'click', typeClickEvtData);
235+
236+
if(clickVal !== false && fullLayoutNow.hovermode) {
237+
gd._hoverdata = [makeEventData(pt, traceNow, opts.eventDataKeys)];
238+
Fx.click(gd, d3.event);
239239
}
240240

241+
// if click does not trigger a transition, we're done!
242+
if(noTransition) return;
243+
244+
// if custom handler returns false, we're done!
245+
if(clickVal === false) return;
246+
241247
// skip if triggered from dragging a nearby cartesian subplot
242248
if(gd._dragging) return;
243249

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

254-
var id = helpers.getPtId(pt);
255-
var nextEntry = helpers.isEntry(pt) ?
256-
helpers.findEntryWithChild(hierarchy, id) :
257-
helpers.findEntryWithLevel(hierarchy, id);
258-
259260
var frame = {
260-
data: [{level: helpers.getPtId(nextEntry)}],
261+
data: [{level: nextLevel}],
261262
traces: [trace.index]
262263
};
263264

‎test/jasmine/tests/sunburst_test.js‎

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -811,11 +811,16 @@ describe('Test sunburst clicks:', function() {
811811
if(trackers.sunburstclick.length === 1) {
812812
expect(trackers.sunburstclick[0].event).toBeDefined();
813813
expect(trackers.sunburstclick[0].points[0].label).toBe('Seth');
814+
expect(trackers.sunburstclick[0].nextLevel).toBe('Seth');
814815
} else {
815816
fail('incorrect plotly_sunburstclick triggering');
816817
}
817818

818-
if(trackers.click.length) {
819+
if(trackers.click.length === 1) {
820+
expect(trackers.click[0].event).toBeDefined();
821+
expect(trackers.click[0].points[0].label).toBe('Seth');
822+
expect(trackers.click[0].nextLevel).not.toBeDefined();
823+
} else {
819824
fail('incorrect plotly_click triggering');
820825
}
821826

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

891-
// should be same before and after 2nd click
892-
function _assertCommon(msg) {
893-
if(trackers.click.length) {
894-
fail('incorrect plotly_click triggering - ' + msg);
895-
}
896-
if(trackers.animating.length !== 1) {
897-
fail('incorrect plotly_animating triggering - ' + msg);
898-
}
899-
}
900-
901896
Plotly.plot(gd, mock)
902897
.then(setupListeners())
903898
.then(click(gd, 2))
@@ -907,27 +902,49 @@ describe('Test sunburst clicks:', function() {
907902
if(trackers.sunburstclick.length === 1) {
908903
expect(trackers.sunburstclick[0].event).toBeDefined(msg);
909904
expect(trackers.sunburstclick[0].points[0].label).toBe('Seth', msg);
905+
expect(trackers.sunburstclick[0].nextLevel).toBe('Seth', msg);
910906
} else {
911907
fail('incorrect plotly_sunburstclick triggering - ' + msg);
912908
}
913909

914-
_assertCommon(msg);
910+
if(trackers.click.length === 1) {
911+
expect(trackers.click[0].event).toBeDefined(msg);
912+
expect(trackers.click[0].points[0].label).toBe('Seth', msg);
913+
expect(trackers.click[0].nextLevel).not.toBeDefined(msg);
914+
} else {
915+
fail('incorrect plotly_click triggering - ' + msg);
916+
}
917+
918+
if(trackers.animating.length !== 1) {
919+
fail('incorrect plotly_animating triggering - ' + msg);
920+
}
915921
})
916922
.then(click(gd, 4))
917923
.then(function() {
918924
var msg = 'after 2nd click';
919925

920-
// should trigger plotly_sunburstclick twice, but not additional
921-
// plotly_click nor plotly_animating
926+
// should trigger plotly_sunburstclick and plotly_click twice,
927+
// but not plotly_animating
922928

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

930-
_assertCommon(msg);
937+
if(trackers.click.length === 2) {
938+
expect(trackers.click[0].event).toBeDefined(msg);
939+
expect(trackers.click[0].points[0].label).toBe('Awan', msg);
940+
expect(trackers.click[0].nextLevel).not.toBeDefined(msg);
941+
} else {
942+
fail('incorrect plotly_click triggering - ' + msg);
943+
}
944+
945+
if(trackers.animating.length !== 1) {
946+
fail('incorrect plotly_animating triggering - ' + msg);
947+
}
931948
})
932949
.catch(failTest)
933950
.then(done);
@@ -947,10 +964,7 @@ describe('Test sunburst clicks:', function() {
947964
fail('incorrect plotly_sunburstclick triggering');
948965
}
949966

950-
if(trackers.click.length === 1) {
951-
expect(trackers.click[0].event).toBeDefined();
952-
expect(trackers.click[0].points[0].label).toBe('Seth');
953-
} else {
967+
if(trackers.click.length !== 0) {
954968
fail('incorrect plotly_click triggering');
955969
}
956970

‎test/jasmine/tests/treemap_test.js‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,11 +1042,16 @@ describe('Test treemap clicks:', function() {
10421042
if(trackers.treemapclick.length === 1) {
10431043
expect(trackers.treemapclick[0].event).toBeDefined();
10441044
expect(trackers.treemapclick[0].points[0].label).toBe('Seth');
1045+
expect(trackers.treemapclick[0].nextLevel).toBe('Seth');
10451046
} else {
10461047
fail('incorrect plotly_treemapclick triggering');
10471048
}
10481049

1049-
if(trackers.click.length) {
1050+
if(trackers.click.length === 1) {
1051+
expect(trackers.click[0].event).toBeDefined();
1052+
expect(trackers.click[0].points[0].label).toBe('Seth');
1053+
expect(trackers.click[0].nextLevel).not.toBeDefined();
1054+
} else {
10501055
fail('incorrect plotly_click triggering');
10511056
}
10521057

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

1087-
// should be same before and after 2nd click
1088-
function _assertCommon(msg) {
1089-
if(trackers.click.length) {
1090-
fail('incorrect plotly_click triggering - ' + msg);
1091-
}
1092-
if(trackers.animating.length !== 1) {
1093-
fail('incorrect plotly_animating triggering - ' + msg);
1094-
}
1095-
}
1096-
10971092
Plotly.plot(gd, mock)
10981093
.then(setupListeners())
10991094
.then(click(gd, 2))
@@ -1103,27 +1098,50 @@ describe('Test treemap clicks:', function() {
11031098
if(trackers.treemapclick.length === 1) {
11041099
expect(trackers.treemapclick[0].event).toBeDefined(msg);
11051100
expect(trackers.treemapclick[0].points[0].label).toBe('Seth', msg);
1101+
expect(trackers.treemapclick[0].nextLevel).toBe('Seth', msg);
11061102
} else {
11071103
fail('incorrect plotly_treemapclick triggering - ' + msg);
11081104
}
11091105

1110-
_assertCommon(msg);
1106+
if(trackers.click.length === 1) {
1107+
expect(trackers.click[0].event).toBeDefined(msg);
1108+
expect(trackers.click[0].points[0].label).toBe('Seth', msg);
1109+
expect(trackers.click[0].nextLevel).not.toBeDefined(msg);
1110+
} else {
1111+
fail('incorrect plotly_click triggering - ' + msg);
1112+
}
1113+
1114+
if(trackers.animating.length !== 1) {
1115+
fail('incorrect plotly_animating triggering - ' + msg);
1116+
}
11111117
})
11121118
.then(click(gd, 4))
11131119
.then(function() {
11141120
var msg = 'after 2nd click';
11151121

1116-
// should trigger plotly_treemapclick twice, but not additional
1117-
// plotly_click nor plotly_animating
1122+
// should trigger plotly_treemapclick and plotly_click twice,
1123+
// but not plotly_animating
11181124

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

1126-
_assertCommon(msg);
1133+
if(trackers.click.length === 2) {
1134+
expect(trackers.click[0].event).toBeDefined(msg);
1135+
expect(trackers.click[0].points[0].label).toBe('Awan', msg);
1136+
expect(trackers.click[0].nextLevel).not.toBeDefined(msg);
1137+
} else {
1138+
fail('incorrect plotly_click triggering - ' + msg);
1139+
}
1140+
1141+
1142+
if(trackers.animating.length !== 1) {
1143+
fail('incorrect plotly_animating triggering - ' + msg);
1144+
}
11271145
})
11281146
.catch(failTest)
11291147
.then(done);
@@ -1143,10 +1161,7 @@ describe('Test treemap clicks:', function() {
11431161
fail('incorrect plotly_treemapclick triggering');
11441162
}
11451163

1146-
if(trackers.click.length === 1) {
1147-
expect(trackers.click[0].event).toBeDefined();
1148-
expect(trackers.click[0].points[0].label).toBe('Seth');
1149-
} else {
1164+
if(trackers.click.length !== 0) {
11501165
fail('incorrect plotly_click triggering');
11511166
}
11521167

0 commit comments

Comments
 (0)