DRAFT: add legend.groupclick option #5849
Conversation
|
Thanks for this PR! This is definitely the right approach! The next step would be getting the CircleCI build to go green (see github status block above) and then we can look at which tests make sense. |
brussee
force-pushed
the
feature/legendgrouptoggle
branch
3 times, most recently
from
82b453a to
3f739a3
Compare
toggle individual items in a legendgroup or the legendgroup as a whole
brussee
force-pushed
the
feature/legendgrouptoggle
branch
from
3f739a3 to
b0ac05f
Compare
|
Rebased on master. |
|
This looks like an awesome PR, thank you! I'm sorry for the long delay in responding :) @alexcjohnson can we get a review on this one please? |
|
Please create |
| @@ -27,6 +28,8 @@ module.exports = function handleClick(g, gd, numClicks) { | |||
| else if(numClicks === 2) mode = itemDoubleClick; | |||
| if(!mode) return; | |||
|
|
|||
| var toggleGroup = groupClick === undefined || groupClick === 'togglegroup'; | |||
There was a problem hiding this comment.
groupclick has a dflt value, so is there ever a case where we still get undefined?
There was a problem hiding this comment.
No defined case indeed. (So only if it is possible to change this value manually or in case of a bug.)
| 'Determines the behavior on legend group item click.', | ||
| '*toggleitem* toggles the visibility of the individual item clicked on the graph.', | ||
| '*togglegroup* toggles the visibility of all items in the same legendgroup as the item clicked on the graph.', | ||
| '*false* disables legend group click interactions.' |
There was a problem hiding this comment.
What's the use case for groupclick: false that isn't covered by itemclick: false? I guess you have some traces in a group and others ungrouped, and you want to allow clicks on the ungrouped traces but not on the groups? That feels weird. Unless I'm missing something, can we remove false as an option here?
There was a problem hiding this comment.
I tend to agree that false is not a valid option here, and that groupclick should only determine what happens when you click on a grouped item AND itemclick is not false
There was a problem hiding this comment.
A better way to 'lock' certain traces/legendgroups would be with a dedicated feature, so I agree to remove the "false" option here.
|
Looks great, thanks for the typo fixes! Aside from the comments above, it would be nice to include a test of the new behavior - I guess just a variant of the legendgroup visibility test with |
|
@brussee wondering if you would possibly be able to get this PR to the finish line today? If not I'd be happy to help with that. |
toggle individual items in a legendgroup or the legendgroup as a whole
Fixes #3135
This is a draft. I can add tests if there is consensus about the approach taken here. Also this is my first PR to this repo, please forgive me if I made rookie mistakes :)