Conversation
StRhys
force-pushed
the
issue-793/Tabular-view-of-data/data-download-of-filtered-charts
branch
from
3c30de8 to
4834a38
Compare
eldeal
left a comment
eldeal
left a comment
There was a problem hiding this comment.
I'll take another look at it on Monday. Some of the logic is not quite clicking for me but that could just be a Friday thing! :)
|
|
||
| const onLegendClick = (e: any) => { | ||
| // check if the passed 'e.curveNumber' is in filteredColumnIndexes | ||
| // if it is, that means its filter is on and should be turned off |
There was a problem hiding this comment.
I'm finding it tricky to keep track of whether 'filtered' means present or not present. These 2 lines of comment I think accentuate the point. Is it worth considering renaming it to 'hidden' or 'visible' so its clearer what the consequence of the filter action is? so filteredColumnIndexes might become visibleColumnIndexes or something?
If not, I think at least in these comments it would be helpful to be clearer that if it is, that means its filter is on and the column should not be visible or whatever the appropriate consequence is
There was a problem hiding this comment.
I've renamed filteredColumnIndexes to hiddenColumnIndexes, and reworded some comments for clarity.
I think I'm biased so if it's still not clear I'm happy to expand further.
| const filteredData = data.filter( | ||
| (_element: any, index: number) => !indexes.includes(index), | ||
| ); | ||
| // this setting of filteredColumnIndex is here because onLegendClick fires twice when |
There was a problem hiding this comment.
I dont know if this is the same thing, but would configuring the double click delay help prevent it getting picked up as 2 clicks? https://plotly.com/javascript/configuration-options/#double-click-delay
There was a problem hiding this comment.
Good find, I had a play with it but it was still firing the single click twice. It was just delaying how long it took to put the filter on the actual chart, this could be an issue with plotly though.
https://plotly.com/javascript/reference/layout/#layout-legend-itemdoubleclick
We do have the option of changing what the double click does.
- rename filteredColumnIndexes to hiddenColumnIndexes
eldeal
left a comment
eldeal
left a comment
There was a problem hiding this comment.
The comment and variable updates make it much clearer 👍
…-of-filtered-charts
StRhys
deleted the
issue-793/Tabular-view-of-data/data-download-of-filtered-charts
branch
3 participants
This ticket allows for filters that have been applied to charts to feed through into the data tab view and the downloadable csv. Filters can be applied by clicking on the legend of a chart. Legends can also be double clicked to have different effects on filters depending on what filters are already applied.
This could have been me doing something wrong but Plotly has a few issues which tie to clicking on the legend.
The first being when a legend click event triggers it will pass an event which contains the data the chart, with also has series visibility on (true, undefined, and 'legendonly'). The issue with this is it will contain the data of the chart pre click, so using it to set the filters on the chart becomes complicated and impossible when a double click triggers, which can turn on/off multiple filters. So this method isn't used.
So I decided to use an array to track which series have been filtered. Event data passes a 'curveNumber' which is the index of the clicked series. The issue that arose was that this click trigger event wouldn't lead to the state of the index array being set, it was only set when switching tabs. The logic behind tracking filtered indexes needs an up to date array and so in the end a normal variable array was used to track the current index array, with another one used to keep state when switching tabs.