-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Clickable Legend Titles #7698
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.
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
base: master
Are you sure you want to change the base?
Clickable Legend Titles #7698
Conversation
…multiple legends
|
@alexshoe Can you document the full API for the new properties in this PR description? Basically just the info that's in the plot schema. |
|
@alexshoe Can you remove the image test? You can make a Codepen and link it in the PR description for testing purposes, but this feature doesn't require adding an image test to the test suite. |
| // Don't create a click targets for group titles when groupclick is 'toggleitem' | ||
| if(d[0].groupTitle && legendObj.groupclick === 'toggleitem') return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexshoe Is this line an unrelated bugfix for groupclick? Or is it related to the titleclick functionality?
(it's fine either way just want to clarify)
| if(numClicks === 1 && legendObj.titleclick) { | ||
| const clickVal = Events.triggerHandler(gd, 'plotly_legendtitleclick', evtData); | ||
| if(clickVal === false) return; | ||
|
|
||
| legendObj._titleClickTimeout = setTimeout(function() { | ||
| if(gd._fullLayout) handleTitleClick(gd, legendObj, legendObj.titleclick); | ||
| }, doubleClickDelay); | ||
| } else if(numClicks === 2) { | ||
| if(legendObj._titleClickTimeout) clearTimeout(legendObj._titleClickTimeout); | ||
| gd._legendMouseDownTime = 0; | ||
|
|
||
| const dblClickVal = Events.triggerHandler(gd, 'plotly_legendtitledoubleclick', evtData); | ||
| if(dblClickVal !== false && legendObj.titledoubleclick) handleTitleClick(gd, legendObj, legendObj.titledoubleclick); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexshoe Could some of the logic in the clickOrDoubleClick() function possibly be reused here? Seems like a fair amount of code duplication, although I'm sure there are some subtle differences.
| function positionTitleToggle(scrollBox, legendObj, legendId) { | ||
| const titleToggle = scrollBox.select('.' + legendId + 'titletoggle'); | ||
| if(!titleToggle.size()) return; | ||
|
|
||
| const side = legendObj.title.side || 'top'; | ||
| const bw = legendObj.borderwidth; | ||
| var x = bw; | ||
| const width = legendObj._titleWidth + 2 * constants.titlePad; | ||
| const height = legendObj._titleHeight + 2 * constants.titlePad; | ||
|
|
||
|
|
||
| if(side === 'top center') { | ||
| x = bw + 0.5 * (legendObj._width - 2 * bw - width); | ||
| } else if(side === 'top right') { | ||
| x = legendObj._width - bw - width; | ||
| } | ||
|
|
||
| titleToggle.attr({ x: x, y: bw, width: width, height: height }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, likewise this function seems like it's duplicating a lot of the title placement logic. I don't think we should be referencing parameters like legendObj.title.side here at all. My feeling is that the titleToggle logic should be more parallel to the traceToggle placement logic.
| const legendItem = g.data()[0][0]; | ||
| if(legendItem.groupTitle && legendItem.noClick) return; | ||
|
|
||
| const legendId = legendItem.trace.legend || 'legend'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this fixes a bug where 'secondary' legends (e.g. legend2, legend3, etc.) used the click settings of legend rather than using their own settings, is that right? I've tried it out in Codepen and that seems to be the case.
I believe that fixes this issue, can you confirm @alexshoe ? If so, please tag in PR description.
| exports.handleTitleClick = function handleTitleClick(gd, legendObj, mode) { | ||
| const fullLayout = gd._fullLayout; | ||
| const fullData = gd._fullData; | ||
| const legendId = legendObj._id || 'legend'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps move the getId() function in src/components/legend/draw.js to helpers.js and use it here, since it does exactly this
| } | ||
| }; | ||
|
|
||
| exports.handleTitleClick = function handleTitleClick(gd, legendObj, mode) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also similar comment here to above -- check whether you can share some of this logic with the handleClick function to avoid TOO much code duplication
| } | ||
| }; | ||
|
|
||
| exports.handleTitleClick = function handleTitleClick(gd, legendObj, mode) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does bother me that this function signature isn't the same as the one for handleClick() -- maybe this signature is better and/or more appropriate for legend titles! But just double check that the signature makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like for the sake of consistency, would it be better to accept numClicks here? Or alternatively modify handleClick() to accept a mode argument?
| var toggleThisLegend; | ||
| var toggleOtherLegends; | ||
|
|
||
| if(mode === 'toggle') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's unclear what are the possible values for mode. Can you add a docstring for the handleTitleClick function?
| }); | ||
|
|
||
| toggleThisLegend = !anyVisibleHere; | ||
| toggleOtherLegends = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| toggleOtherLegends = null; | |
| toggleOtherLegends = false; |
I don't see why you wouldn't make this false
|
|
||
| for(var i = 0; i < allLegendItems.length; i++) { | ||
| const item = allLegendItems[i]; | ||
| const shouldShow = isInLegend(item) ? toggleThisLegend : toggleOtherLegends; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this right? I feel like this might give the wrong result when item.showlegend is false. I can try to come up with a reproducible example if that would help.
|
@robertcollar-kobold Pie charts do sometimes need unique logic compared to other trace types when it comes to legends, since pie chart legends contain one entry per data point rather than one entry per trace. That said, @alexshoe I can't see any reason why the Here's an example of a pie chart with multiple legends: Plot def: Details{
"data": [
{
"type": "pie",
"labels": ["a", "b", "c"],
"values": [3,5,4],
"showlegend": true,
"domain": {"row": 0, "column": 0},
},
{
"type": "pie",
"labels": ["d", "e", "f"],
"values": [1,3,7],
"legend": "legend2",
"showlegend": true,
"domain": {"row": 0, "column": 1},
},
],
"layout": {
"grid": {"rows": 1, "columns": 2},
"legend": {
"title": {"text": "Mylegend"},
"visible": true,
},
"legend2": {
"title": {"text": "Mylegend2"},
"visible": true,
"y": 0.15,
"itemclick": false,
},
}
}Image:
|
| var SHOWISOLATETIP = true; | ||
|
|
||
| module.exports = function handleClick(g, gd, numClicks) { | ||
| exports.handleClick = function handleClick(g, gd, numClicks) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename this function to handleItemClick() for clarity
|
@alexshoe @robertcollar-kobold One question I just thought of, whose answer is not immediately obvious to me (but maybe it is to you): Should clicking the legend title toggle the visibility of all traces listed in the legend (i.e. those with Here's a mock to make it clearer. Each legend has one trace for which
Detailsvar plotDef = {
"data": [
{
"x": [1,2,3],
"y": [2,4,3],
"showlegend": false,
},
{
"x": [1,2,3],
"y": [3,5,4],
"showlegend": true,
},
{
"x": [2,4,6],
"y": [2,1,5],
"legend": "legend2",
"showlegend": true,
},
{
"x": [2,4,6],
"y": [3,2,6],
"yaxis": "yaxis2",
"legend": "legend2",
"showlegend": false,
},
],
"layout": {
"shapes": [
{
"type": "rect",
"x0": 1,
"x1": 2.5,
"y0": 4,
"y1": 5.5,
"line": {"color": "#FF0000"},
"showlegend": true,
},
{
"type": "rect",
"x0": 5,
"x1": 6,
"y0": 1,
"y1": 2,
"line": {"color": "#FF00FF"},
"legend": "legend2",
"showlegend": false,
},
],
"legend": {
"title": {"text": "Mylegend"},
"visible": true,
},
"legend2": {
"title": {"text": "Mylegend2"},
"visible": true,
"y": 0.25,
},
"yaxis2": {
"overlaying": "yaxis",
}
}
}; |
| titleclick: 'toggle', | ||
| titledoubleclick: false | ||
| } | ||
| }).then(function() { | ||
| expect(gd._fullLayout.legend.titleclick).toBe('toggle'); | ||
| expect(gd._fullLayout.legend.titledoubleclick).toBe(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| titleclick: 'toggle', | |
| titledoubleclick: false | |
| } | |
| }).then(function() { | |
| expect(gd._fullLayout.legend.titleclick).toBe('toggle'); | |
| expect(gd._fullLayout.legend.titledoubleclick).toBe(false); | |
| titleclick: 'toggleothers', | |
| titledoubleclick: 'toggle', | |
| } | |
| }).then(function() { | |
| expect(gd._fullLayout.legend.titleclick).toBe('toggleothers'); | |
| expect(gd._fullLayout.legend.titledoubleclick).toBe('toggle'); |
Should test with values which are never the default


Description:
Adds the ability to click/double-click on legend titles to toggle legend visibility, making it easier to manage complex charts with multiple legends.
Example:
See this codepen for an interactive demo of this feature
New API:
legend.titleclick'toggle'|'toggleothers'|false'toggle'when there are multiple legends,falseotherwiselegend.titledoubleclick'toggle'|'toggleothers'|false'toggleothers'when there are multiple legends,falseotherwiseWhat each value for
titleclickandtitledoubleclickdoes:'toggle'toggles the visibility of all items in the clicked legend'toggleothers'toggles the visibility of all other legendsfalsedisables legend title double-click interactions