Closed leeoniya closed 4 years ago
most of the necessary work is done in 10bf5b2b5739c41b1b3aeec4055ba21510f0a83b & 130fc217a2d29b1f6903951f78b1b036a6a01c49. i have a basic bi-directional demo working locally, but still not completely happy with the hook firing order and when setSelect should fire:
init
? probably after since setCursor fires after (as a result of setData)still need to add the ability for initial (but not permanent) scale ranges to be be defined.
the bi-directional nature leads to a chicken-egg problem if we rely only on hooks to set initial state. does the zoomed chart's setScale trigger the full chart's setSelect or vice versa? if neither and we rely on setting the values directly via opts, does that mean we don't fire the hooks at all? bad too.
Can you share the demo you have?
yes, i'll push it later tonight.
the demo is here: https://github.com/leeoniya/uPlot/blob/master/demos/zoom-ranger.html
not super elegant, but not terrible either. couple thoughts that came out of this exercise.
.setCursor()
, .setSelect()
(and their matched initial opts values .cursor
and .select
) take left/width/top/height coords, but this is mostly useless from an API perspective because 1) it would be much better to set the cursor or selection to some specific x and y scale values, or values from the data and 2) during init, there is no way to know what coords to specify before the chart & data if fully initialized with the scales and we cannot yet make use of .valToPos()
. i think these apis will change to accept scale values rather than coords.
it may make sense to pass through additional info into each api call through to the hooks so that it's possible to determine if it was triggered via UI or via API. this would make the temporary flag setting cleaner. maybe it's possible to simply tag the internally-triggered stuff automatically. not sure, and may add a lot of extra wrappers. probably not worth it.
adding a final ready
hook would allow moving the initial setSelect
call into the opts (or a plugin). might not be necessary if the changes in point 1 work out well.
Thanks for putting this together!
I agree that passing scale values rather than coords seems more useful. (but why is y and height used at all for setSelect?)
For point 2, wouldn't it be possible to pass the setSelect "opts" object to the hook? Then the caller can simply set some field in that object, if necessary.
After experimenting with this solution, I realized that for my use case, I can do something simpler; I now use a slider (Ion.RangeSlider) with two handles to set the x min/max values, and for this I just need setScale() (and the setScale hook to update the slider). This works well (I can even do "panning" with direct update of the uplot when I drag the slider). But I want to align the slider with the canvas that uplot draws. Would it be possible to pass an "id" of the canvas element that uplot creates in the options, so that I can find the element? Currently I do: $('.plot')[0].childNodes[0].getBoundingClientRect() but that is brittle and not even correct... Perhaps I should open a separate issue for this?
(but why is y and height used at all for setSelect?)
cause as of 130fc217a2d29b1f6903951f78b1b036a6a01c49 it's now possible to do selection and zooming/rescaling via a rect. it's still disabled by default. but can be enabled in cursor.drag
.
after thinking some more, i'll probably leave the current API. there are bigger snowballing questions that start to happen with regards to how sync
would work if the APIs took scale values rather than coordinates (coords exist everywhere, but scales may not).
ultimately, cursor setting and selection drawing do not affect the chart drawing. they're visual add-ons. i was trying to make it possible to declaratively set the initial cursor position and selection range in opts, but in reality, i don't think there's a good case for this since both can be set after initialization in some final ready
hook, or immediately after instantiation. there's some hook firing refinement still to do but i think the current design makes the right compromises.
Would it be possible to pass an "id" of the canvas element that uplot creates in the options
uplot can already take an id in the opts to add to the top-level wrapper, but the fastest way to grab the canvas element is u.ctx.canvas
.
Yes I know I can pass an id for the top-level wrapper, but I need the canvas. u.ctx.canvas works fine. Do you view this (u.ctx) as part of the public api?
Do you view this (u.ctx) as part of the public api?
yes. it's explicitly exposed: https://github.com/leeoniya/uPlot/blob/e7acfc2642b284699a297b5ce5303ede738e8bcc/src/Line.js#L411
ok, i think this is now good: https://github.com/leeoniya/uPlot/blob/master/demos/zoom-ranger.html
the only meh are the viaRanger
& viaZoom
flags to sync/disambiguation, but i'll live with it. pretty happy with the rest.
copied from https://github.com/leeoniya/uPlot/issues/83#issuecomment-570573947
i was planning on making a demo similar to this:
http://dygraphs.com/gallery/#g/range-selector
there's nothing built in but i think there's enough api to make this without too much effort using 2 charts with the same data but different scale ranges and some ui sprinkled on.
i might need to add the ability to disable zoom without disabling selection and make a new "select" hook. i havent really thought about it much yet.