Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tooltips or an API to support their implementation #63

Closed
silverwind opened this issue Dec 6, 2019 · 12 comments
Closed

Tooltips or an API to support their implementation #63

silverwind opened this issue Dec 6, 2019 · 12 comments

Comments

@silverwind
Copy link
Contributor

silverwind commented Dec 6, 2019

I like to have tooltips instead of the legend to show the currently hovered value. Would you consider implementing tooltips yourself or alternatively provide an API like getValuesAtCursor to allow a implementation with libraries like popper.js? Something like this:

https://codepen.io/FezVrasta/pen/bWezaY

@leeoniya
Copy link
Owner

leeoniya commented Dec 6, 2019

i definitely want to flesh out an event api to make extensibility possible, but probably not much beyond that.

thinking bigger picture, what can be useful for an onmove cursor event besides values? also, providing values as an array can be sub-optimal even with the existing rAF-debouncing, since it'll need to allocate up yo 60 arrays/s.

i think providing the index within the data should be sufficient, and the user can easily read off the raw values directly from the data struct.

the left,top position of the cursor would be needed.

an exposed method of translating a position into a value along a desired scale. (and vice-versa).

{onmove: (leftPos, topPos, idx) => {}}
.idxAt(leftPos)
.valAt(pos, scaleKey)
.posOf(val, scaleKey)

idx in onmove is not technically necessary since it can be queried via .idxAt(), but if the cursor intersection points are already being shown along each series, then this call is already done internally, so passing it through probably makes sense for perf. i guess this brings up the question of why not also pass the coords of the points through? probably cause translating a data value to a scale pos (redundantly) and returning a plain number is cheaper than either allocating an object/array for all values or searching a potentially multi-million-datapoint array for the nearest index redundantly.

thoughts?

@leeoniya
Copy link
Owner

leeoniya commented Dec 6, 2019

ok, this turned out pretty well:

https://leeoniya.github.io/uPlot/demos/tooltips.html

@silverwind
Copy link
Contributor Author

silverwind commented Dec 7, 2019

The reason I linked the popper.js example is that it is able to deal with tooltip overflow. I see two ways a tooltip can overflow and which one to choose depends on chart size and tooltip size:

  1. If the chart is bigger than tooltip, confine tooltip to the chart area and reposition it once it get to the right/bottom edges (for right/bottom anchored tooltips).
  2. If the chart is about equal or smaller than the tooltip (think sparklines), confine the tooltip to the page. It is important to me that this works even when overflow: hidden is set on the chart or any of its parent elements.

I strongly recommend either popper.js or maybe the lightweight placement.js for the tooltip placement. They both allow to set a bounding element that could be set depending on chart and expected tooltip size and they put their rendered elements inside a "portal" DOM node on body to avoid any overflow or z-index issues.

So my suggestion would be a mousemove event with access to:

  • The original DOM mousemove event triggered on the .uplot element, which allows to calculate the DOM x/y postion via event.target.getBoundingClientRect() and event.clientX and event.clientY.
  • The x and y coordinates in the chart's domain.
  • The data array index of the nearest point. Not strictly neccesary as it can be calulated from x and y. I could see this (presumably) O(n) calculation getting expensive on big charts.

@leeoniya
Copy link
Owner

leeoniya commented Dec 7, 2019

the demo is just a demo. i think all the pieces are there to do what you need with popper.js.

  • the target will always be uplot.root.querySelector(".plot"), so you can always query its getBoundingClientRect.
  • x and y are provided and you can get the clientX and clientY by subtracting x/y from the bounding rect.
  • you already get the index.

the main reason i dont want to simply pass through the mousemove events is because they're rAF-debounced for reducing unnecesaary cursor, legend and focus updates. i suppose i can cache the raw event object internally until the rAF fires and pass it along with the other arguments.

however, everything you mentioned is already attainable without raw event access.

I could see this (presumably) O(n) calculation getting expensive on big charts.

it's a binary search, so O(log n).

@silverwind
Copy link
Contributor Author

Yes, looks like the necessities are all there, I will try to set up a demo using placement.js and report back.

@silverwind
Copy link
Contributor Author

https://codesandbox.io/s/uplot-tooltip-placement-l4b9u

I'm happy with the result. To change it to be confied to the chart area, set bound = plot. I opted to hide .cursor-hz always and only show .cursor-vt when the mouse is over the chart. Maybe this is something you want to consider supporting more directly (e.g. a option to hide a cursor line and some methods to show/hide them). Also maybe cursorenter and cursorleave events might be useful.

(For some reason that Sandbox only works in Firefox for me)

@leeoniya
Copy link
Owner

leeoniya commented Dec 7, 2019

Maybe this is something you want to consider supporting more directly

i'd like to keep the opts minimal and delegate to css styling or dom whenever possible.

Also maybe cursorenter and cursorleave events might be useful.

there's nothing internally that tracks this right now, so it'll be adding code & event listeners for the sake of adding them. could be useful in the future if someone has a burning need for it, but i'll leave this alone for now. these events would coincide with plain old mouseleave and mouseenter events which could just be attached and handled without uplot's help since there's not much useful data they can carry besides their already-accessible exit/entry coords.

heads up, i'll probably be changing cursor-vt/hz to cursor-x/y before v1.0 (for consistency).

@silverwind
Copy link
Contributor Author

there's nothing internally that tracks this right now

One thing I do not like about the default cursor lines is that they get stuck on the chart after mouseleave. I think it would be more expected if they properly show/hide with the cursor entering/leaving. If you want to do that as well, I guess you would need those events internally.

(Also my demo has a bug that if the cursor is over the chart while the page is loading, the tooltip will not show, so the style would needed to be toggled on on cursormove too.)

@leeoniya
Copy link
Owner

leeoniya commented Dec 7, 2019

yeah those cursor improvements sound reasonable. not sure if leave/enter would be better than some cursortoggle that would trigger as a result of enter/leave.

i do want to keep the cursor always visible if it's clicked/locked.

edit: another benefit of having the cursor off by default is that i can stop moving it to the center on initial render, and thus avoid a move event.

@leeoniya
Copy link
Owner

@silverwind

now that the final API is mostly baked, here's how i'd do it:

https://jsfiddle.net/bg1uLx8s/

since uPlot does not handle the final dom insertion, it's necessary for the plugin to attach a mutation observer so we can avoid calling the rather expensive getBoundingClientRect on every mousemove, since its results are bogus at time of initial init & setCursor.

this isn't the only way to get things done but it does the trick.

i'll make a note to document that these hooks fire before dom insertion.

leeoniya added a commit that referenced this issue Dec 28, 2019
this delays firing hooks/plugins in case they need to measure the dom
@leeoniya
Copy link
Owner

leeoniya commented Dec 28, 2019

ok, this inelegance was bugging me.

i added a ready callback to the constructor which will defer firing hooks until e.g. dom insertion. i figured this was more flexible than adding target and assuming appendChild or similar.

anyways, this cleaned things up significantly: 7f3c51e#diff-b12b7f66b7a99297dec560a423dd5ed6

leeoniya added a commit that referenced this issue Dec 28, 2019
this delays firing hooks/plugins in case they need to measure the dom
@leeoniya
Copy link
Owner

the third constructor param can now accept an HTMLElement container to append into. this signature also defers firing hooks until after dom insertion.

https://github.com/leeoniya/uPlot/blob/master/demos/cursor-tooltip.html#L108

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants