Repository navigation
[timelion] highlight series on legend mouseover - #15229
Conversation
thomasneirynck
left a comment
There was a problem hiding this comment.
I like the improvement, since it is aligning functionality, but also a little ambivalent, not in the context of this single change, but the direction we want to go with Kibana-charts as a whole.
imo, this issue is a good candidate that illustrates the problem with having different line-charts implementations across the product.
I think we really need to have a single XY-chart implementation, that wraps the common functionality (brushing, highlighting, click-handling, ...) in one place. We've talked about this a couple times, most recently in the context of trying to bring TSVB out of experimental.
The above is tentative now, and we're still trying to figure out space for it, but I do see us progressing in this direction, hopefully sooner than later.
So if we do this enhancement for Timelion, there's a good chance we'll end up replace it.
@timroes we talked about this earlier too. What do you think, is it worthwhile expanding the chart-capabilities if there is a change we'll end up replacing this in the (undetermined) future
There was a problem hiding this comment.
yes. Without the <br>, then the legend jumps (the x-axis value disappears) when you enter it. The jumping makes the mouseover target not very user friendly. By putting a <br> into the caption when empty, then the legend is always the same height and does not move around on the user
There was a problem hiding this comment.
could you add a little explanation like that in the code?
There was a problem hiding this comment.
seems somewhat arbitrary. I'd use null instead.
There was a problem hiding this comment.
This algorithm does a clean sweep with restoreColors, and then reapplies the colors. It uses a temp-property, _originalColor on the series object to keep track of older state. Is there a way we can simplify this, without the need for sweeping past the data twice and having to maintain that temp-property in-sync?
Would this work, using a Map? Maybe I'm forgetting something obvious.
This is the relevant part:
const originalColorMap = new Map();
$scope.chart.forEach((series, seriesIndex) => {
if (!series.color) {
const colorIndex = seriesIndex % defaultOptions.colors.length;
series.color = defaultOptions.colors[colorIndex];
originalColorMap.set(series, series.color);
}
});
const HIGHLIGHT_NOT_SET = -1;
let hightlightedSeries = HIGHLIGHT_NOT_SET;
function unhighlightSeries() {
if (hightlightedSeries === HIGHLIGHT_NOT_SET) {
return;
}
hightlightedSeries = HIGHLIGHT_NOT_SET;
$scope.chart.forEach((series, seriesIndex) => {
series.color = originalColorMap.get(series); //just reset the colors
});
drawPlot($scope.chart);
}
$scope.highlightSeries = _.debounce(function (id) {
if (hightlightedSeries === id) {
return;
}
hightlightedSeries = id;
$scope.chart.forEach((series, seriesIndex) => {
if (seriesIndex !== id) {
series.color = 'rgba(128,128,128,0.1)';//mark as grey
} else {
series.color = originalColorMap.get(series);//color it like it was
}
});
drawPlot($scope.chart);
}, DEBOUNCE_DELAY);
1f59095 to
2936bdb
Compare
|
The legends are currently focusable. I would really like to see the series highlight also when the legend receives focus, so that you can also highlight series via keyboard (keyword: accessibility :D). |
e9f9ca3 to
e62b69d
Compare
There was a problem hiding this comment.
focusedSeries || focusedSeries === 0 otherwise we loose the focus when focusing the first element, because 0 == false.
2e473b1 to
a3e8f35
Compare
* highlight series on mouseover * thomasneirynck review items * make highlighting keyboard accessible * add check for zero to focus if block
* highlight series on mouseover * thomasneirynck review items * make highlighting keyboard accessible * add check for zero to focus if block
* highlight series on mouseover * thomasneirynck review items * make highlighting keyboard accessible * add check for zero to focus if block
fixes #9845