Clickable legends #3641
Clickable legends #3641
Conversation
|
I am still working on this pull request. I need to add tests, as well as hook up the click event to the proper listener to filter on legend click. |
|
It still doesn't seem to be triggering the click handler for the Vis object. |
|
I fixed the test issues as well as the issue with clickable legends not being hooked up. It should be hooked up and working (mostly) properly. There is however one issue, which is when splitting a chart by
Now, here I click on the legend value -
I am not sure why this is. Perhaps @simianhacker you have an idea why this occurs. |
| if (obj.values && obj.values.length) { | ||
| var values = self._filterZeroInjectedValues(obj.values); | ||
| var aggConfigResult = values.length && values[0].aggConfigResult ? | ||
| values[0].aggConfigResult.$parent : undefined; |
There was a problem hiding this comment.
Can you can you change this to:
var aggConfigResult;
if (values.length && values[0].aggConfigResult) {
aggConfigResult = values[0].aggConfigResult.$parent;
}Multiline ternaries are against our style guide
| @@ -73,6 +60,14 @@ define(function (require) { | |||
| } | |||
| } | |||
|
|
|||
| Data.prototype._getLabels = function (data) { | |||
| if (this.type === 'series') { | |||
| if (getLabels(data).length === 1 && getLabels(data)[0] === '') return [(this.get('yAxisLabel'))]; | |||
There was a problem hiding this comment.
Perhaps you should use variable for this if statement.
|
@panda01 I addressed your comments above. You may still be in the process of reviewing the pr, so I will keep you assigned. |
| var data = obj.columns || obj.rows || [obj]; | ||
| var names = []; | ||
|
|
||
| if (!_.isObject(obj)) { throw new TypeError('PieLabel expects an object'); } |
There was a problem hiding this comment.
This might be more helpful if it's before the var data = obj... assignment
|
I was able to filter down to no data and get a console error. Not sure if filtering to no data is intentional, but I'm assuming the error isn't. Other than this and my comment above, looks good.
|
|
@jbudz I made the change and fixed the bug above. Can you confirm that you no longer see the bug above? Having a hard time reproducing. |
|
@stormpython Confirming, no more error. |
|
This pr has been reviewed by at least 6 people, with 3 LGTMs. I will give it one more look over and if everything is ok, merge it tomorrow. |
Closes #2760.
The pull requests adds the ability to filter data by clicking
livalues in the legend. To implement this pr, I needed to restructure the way the legend is instantiated. Previously, the legend only received labels (among other things as input). In this pull, the legend gets the entire data set. This is needed so that we can pass theaggConfigobject in the click event response. TheaggConfigobject contains the appropriate functions used to filter items.In addition, to changes in the legend, the dispatch
eventResponsefunction needed to be modified since not all the values in the legend's data object correspond with those from elements in the chart. Changes here may affect other areas of the code base that rely on theeventResponsefunction code.I've also added a pointer on mouseover of the legend to signify to the user there is a click event listener here.
There are some important things to consider when reviewing this pull request. Its important to make sure that none of the other click events on the chart are affected. And, its important to test that the highlighting functions between the charts and the legend are still working properly for all chart types.
This pull request requires more than one reviewer.
Additional fixes
Closes #3873.
This appears to be a regression in which hovering over pie labels does not highlight legend values for numbers and dates. The reason for this has to due with the fact that for numbers and dates in the legend, the values were passed thru a field formatter function, and the resulting string values were changed. This was a problem because the
data-labelfor pie slices was different from thedata-labelfor legend items. We use thedata-labelto select both elements and when they differ, it leads to one (i.e. a pie slice or a legend value) being highlighted and not the other.In addition, to make legend values clickable for numbers and dates, we need to include the unformatted label values. Therefore, to fix this issue, I removed formatting the legend labels from the
vislib/components/labeldirectory and moved that functionality to only format the text value for the legend label inlegend.js. This allows thedata-labelfor pie slices and legend values to be the same and since they are unformatted, allows for clickable legends.