#15486 closed feature (fixed)
Provide threshold elements for charting
Reported by: | cjolif | Owned by: | cjolif |
---|---|---|---|
Priority: | undecided | Milestone: | 1.9 |
Component: | Charting | Version: | 1.7.2 |
Keywords: | Cc: | Mathevet julien | |
Blocked By: | Blocking: |
Description (last modified by )
The thresholds elements would draw vertical/horizontal lines at data position and provide optional label.
Optionally this can be a:
- dual threshold (low/high)
- the area can be filled
- on can listen to interaction events on the threshold
Attachments (3)
Change History (19)
comment:1 Changed 9 years ago by
Owner: | changed from Eugene Lazutkin to cjolif |
---|---|
Status: | new → assigned |
comment:2 Changed 9 years ago by
Description: | modified (diff) |
---|---|
Type: | defect → feature |
comment:3 Changed 9 years ago by
Description: | modified (diff) |
---|
Changed 8 years ago by
Attachment: | 15486.patch added |
---|
draft patch (no doc yet, look at examples), this includes #15589 as well
comment:5 Changed 8 years ago by
Cc: | Mathevet julien added |
---|
Changed 8 years ago by
Attachment: | 15486_2.patch added |
---|
updated patch with API doc, animation support & some code factorization on CartesianBase?
comment:6 Changed 8 years ago by
The latest patch is breaking alternate axis & widget markup interpretation, I'll be working on fixing this before committing.
comment:8 Changed 8 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
this breaks existing code that subclasses CartesianBase? and calls this.cleanGroup()
without passing the params it now relies upon - creator, dim, offsets, noClip
. is there an alternative implementation that remains backward compatible or do i need to update my subclasses?
comment:9 Changed 8 years ago by
this goes further than i first realized. it also breaks subclasses of action2d/_IndicatorElement subclasses which are indirectly a subclass of CartesianBase?. i hadn't noticed these ones until i started to interact more with some of my custom actions. some of my actions just don't have access to dims
and offset
when calling cleanGroup
because they are used in event handlers that may track mouse movement or touches etc which may call cleanGroup
in response to those events - ie render
is not the only place that cleanGroup
may be called from.
while it's been possible to keep all dojox/charting code working with this change, it is really hurting my custom code. while i know dojox/charting is part of dojox and is technically allowed to break backward compatibility, this change has caught me by surprise.
comment:10 Changed 8 years ago by
Hi Ben, I'll look at CartesianBase incompatibility, I agree we should do our best to keep compatibility here. I just wanted to avoid querying several times those values and so I passed them as arguments. But I can certainly get them directly in the cleanGroup method.
For _IndicatorElement, this was/is a private class. So I except you might have to change some code to get it back running on your side. In particular there is a noClip optional argument that is added to CartesianBase cleanGroup. It is set to true in the new Indicator code (because you don't want the indicator to be clipped to the plot area) which the new !_IndicatorElement inherits from. If you set it to true you should not need the dims & offsets. So cleanGroup should be back working even if called at some point where you don't have the dims/offsets.
Changed 8 years ago by
Attachment: | fixes#15486.patch added |
---|
patch to fix API regression on CartesianBase?.cleanGroup
comment:11 Changed 8 years ago by
i didn't think of _IndicatorElement as a private class because it looks like it provides a base class to build other classes. If I want to build something like MouseIndicator?, it's useful to add a plot that is either an _IndicatorElement (as done by MouseIndicator?) or a subclass of it if i have slightly different requirements. the '_' in a class name often indicates something is a mixin or base class and this was what i thought in this case since it contains code that would be reusable.
anyhow, discussing '_' is just semantics, i could just as easily have said above that i was using plot2d/Indicator (the base class for _IndicatorElement) and i would be having the same problem because my problem doesn't actually come from _IndicatorElement itself. i was just using it so i didn't have to repeat code that's in it.
i tried the noClip option and it doesn't work for me. i have to call cleanGroup from within _trackMove in order for my indicator to work properly and if i use noClip i get some issues. as a side note, i think there's a bug as to why i need to call cleanGroup but i haven't had time to distill it into a test case to figure out how to build a test case. again, that's beside the point now with the patch you've suggested.
the patch should work for me and since it requires no processing to get this.chart.dim
and this.chart.offsets
i'd say it's not too bad to use those since they should be more definitive than what a user may pass in accidentally (or not even pass in at all). if for some reason you wanted to allow a user to pass in dims
and offsets
then you could fall back to this.chart.dim
and this.chart.offsets
if they weren't provided. however, it probably only makes sense to allow for that if you think there's some useful case where the values wouldn't already be the ones from the chart.
comment:12 Changed 8 years ago by
Yes I know your specific issue can happen with more than just _IndicatorElement. However I wanted to emphasize the fact I did not consider this module as a public supported API as of today because you might encounter other issues as I did not tried to keep compatibility on it during my refactoring (note that in addition to the underscore the doc "api" as a "private" tag). Anyway as the patch seems to solve your issue I will commit it. Let me know if you still experience a problem.
Additionally the Mouse/TouchIndicator actions should be re-written on top of that element.