Allow selectZoom to be adaptive #67
Conversation
|
Are you planning to include this feature in an upcoming version? |
|
Thanks and sorry for the delay. I will try this out this weekend. |
src/plugins_extra/selectZoom.ts
Outdated
| const y = Math.min(this.start.p.y, p.y); | ||
| const h = Math.abs(this.start.p.y - p.y); | ||
|
|
||
| if (this.options.enableX && ((w >= h) || (w >= this.options.thresholdX))) { |
There was a problem hiding this comment.
| if (this.options.enableX && ((w >= h) || (w >= this.options.thresholdX))) { | |
| if (this.options.enableX && (w >= this.options.thresholdX)) { |
I think this condition w >= h is confusing. The preview is shown, but the zoom will not be executed.
There was a problem hiding this comment.
You are right, there was a huge oversight on my part since I only tested the feature with very small thresholds...
The application of the zoom did not match the visual indicator if the height and/or width were below their respective thresholds. But the problem was not in the onMouseMove() event as you suggested. Instead I adapted the onMouseUp() event to work exactly like the zoom indicator suggests.
There was a problem hiding this comment.
And regarding the documentation. I could add a section that introduces the SelectZoomPlugin just after the TimeChartZoomPlugin here?
There was a problem hiding this comment.
Instead I adapted the onMouseUp() event to work exactly like the zoom indicator suggests.
I personally do not like this. As you can see, the if statement becomes rather complex, and can be hard to reason about. But fine if you insist and it is properly documented.
And regarding the documentation. I could add a section that introduces the SelectZoomPlugin just after the TimeChartZoomPlugin here?
You may add a link there. But I believe the main documentation should live at https://github.com/huww98/TimeChart/blob/master/src/plugins_extra/README.md#select-zoom
There was a problem hiding this comment.
I tried to increase the readability of the if statement and I introduced private properties for the boolean flags that decide whether an axis should be selected or not. This ensures that the same rules are applied in the events onMouseMove and onMouseUp.
I also added two inputs for the threshold in the select-zoom demo. I have never worked with forms so I just tried to copy the way you handled the checkboxes.
Do you want me to add any inline comments to explain the thresholds? And do you want me to also add documentation to the link you posted in your last comment? Or is it sufficient to have the threshold inputs in the demo?
When both axes are enabled for the selectZoom, it's quite inconvenient to zoom along only one axis. Making the zoom adaptive allows only one axis to be zoomed when the selection along the other axis is below a certain threshold. As an example, see the plot "X or Y (adaptive + omni)" here https://leeoniya.github.io/uPlot/demos/zoom-variations.html.
This merge request implements an adaptive select zoom that can be configured with the options
thresholdXandthresholdY. By default both thresholds are zero which recovers the regular select zoom. When both thresholds are set to a finite value, the zoom selection will ignore the axis with the smaller selection if it is below its respective threshold.