leeoniya / uPlot

📈 A small, fast chart for time series, lines, areas, ohlc & bars
MIT License
8.51k stars 371 forks source link

Suggestion: allow to set custom focus colors for series #429

Closed zefirka closed 3 years ago

zefirka commented 3 years ago

Hi! What do you think about to allow to set series focus color? Currently focus behavior just change alpha of series color, but if I want to make focus behavior a little different (e.g. B&W vs Colored, or a bit darken unfocused lines) I should change colors of series manually and call .redraw(). I suggest just add property focusStroke/focusFill/... for series for custom focus colors.

leeoniya commented 3 years ago

Currently focus behavior just change alpha of series color

if you noticed, alpha is actually applied to all non-focused series. are you asking to add more overrides to non-focused series, or also the focused series? cause they would have to be different sets of options.

it looks like you're asking to add overrides to each series. i'm not a big fan of duplicating all the styling opts just to have customized focus behvior.

i'm open to adding a callback or two that makes it easier to add these yourself in a pluhin, though.

but if I want to make focus behavior a little different (e.g. B&W vs Colored, or a bit darken unfocused lines)

i would like to add defocus styling for more than alpha, but sadly, Safari is new IE: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/filter

zefirka commented 3 years ago

if you noticed, alpha is actually applied to all non-focused series.

Yep, I've mean custom options for blurred lines too.

i'm open to adding a callback or two that makes it easier to add these yourself in a pluhin, though.

Sure, it would be very useful.

leeoniya commented 3 years ago

here's the path of least resistance, i think:

add a ._focus property to each series that gets toggled here:

https://github.com/leeoniya/uPlot/blob/e870fc16cef352fb283a050957ec08374e21a47f/src/uPlot.js#L1854-L1856

then each series can implement dynamic styling by checking that property.

e.g.

https://github.com/leeoniya/uPlot/blob/e870fc16cef352fb283a050957ec08374e21a47f/demos/gradient-fill.html#L73-L81

can you try this route and see how it works out?

leeoniya commented 3 years ago

the only tricky one is dynamic lineWidth, but that's a deep rabbithole: https://github.com/leeoniya/uPlot/issues/403

zefirka commented 3 years ago

Hi, I'm a little confused about place to hold focus callback. Do you suggest to implement something like this:

series: [
  {},
  {
    label: "Serie",
    stroke: "green",
    focus: (u, seriesIdx) => {
        let s = u.series[seriesIdx];

        if (s._focus) {
            return /** focus color */
        } else {
            return /** unfocused color */
        }
    }
]
leeoniya commented 3 years ago

you'd use the regular stroke or fill callbacks and check the s._focus in there:

fill: (u, seriesIdx) => { 
  let s = u.series[seriesIdx]; 

  if (s._focus) {
    return /** focus color */
  } else {
    return /** unfocused color */
  }
}, 
zefirka commented 3 years ago

@leeoniya I think it's pretty nice way, but it could a bit conflicting with default focus behavior. Suppose I'm using default focus with this series config:

fill: (u, seriesIdx) => { 
  let s = u.series[seriesIdx]; 
  return s._focus ? 'red' : 'black';
}, 

Then will it both set alpha for unfocused lines and apply new fill color? Won't there be a problem with that?

leeoniya commented 3 years ago

we can also make alpha a callback with a default implementation that also tests s._focus.

or, more simply, you can just set opts.focus.alpha: 1 and do everything else inside fill and stroke callbacks?

zefirka commented 3 years ago

I tried approach with s._focus and it worked well. But I suggested some enhancements in previous message because of I want to implement consistent workflow for users without requirement to keep in mind that fill can conflict with focus.

we can also make alpha a callback with a default implementation that also tests s._focus. or, more simply, you can just set opts.focus.alpha: 1 and do everything else inside fill and stroke callbacks?

I think that this approach will bring pretty much complexity in code. As a user I would like to see restrictions directly in library API, which doesn't allows to me mess up options.

I haven't any really good solutions in mind. Maybe allow for main focus configuration to use callback and if that callback not provided then we can create default callback which sets alpha:

export type Focus = ((u: uPlot, serieIdx: number) => void) | {
    /** alpha-transparancy of de-focused series */
    alpha: number;
}

// in options
focus: (u, idx) => {
    const serie = u.series[idx];
    if (serie._focus) { ... } else { ... }
}

But I think that this approach is bad because of possible messing up fill/stroke values in series (you can overwrite current value when series is not focused, so you should cache old value to restore it when it's focused) etc

If you haven't any comments about the above then I could prepare PR with just setting s._focus.

leeoniya commented 3 years ago

@zefirka heads up, this behavior is changing slightly with https://github.com/leeoniya/uPlot/commit/b4abda5048f62bbbc36e740c80269ae7425292b6. https://github.com/leeoniya/uPlot/commit/9debaf3599f8dd2bd429b79e1e6f84aeb7699a14. TL;DR: use focus.alpha: 1.1.

reason for this is that we ran into an issue (https://github.com/grafana/grafana/issues/31774) where Grafana sets alpha: 1 to trigger the setSeries hook to update the active series in the tooltip. but during cursor movement with many series the chart would also keep redrawing to "set focus". i added a condition that avoids calling u.redraw() when alpha: 1, so now you have to either call it manually from the setSeries hook (b4abda5048f62bbbc36e740c80269ae7425292b6), or just set an alpha > 1 (9debaf3599f8dd2bd429b79e1e6f84aeb7699a14).

zefirka commented 3 years ago

@leeoniya Ok, thanks for mention!