Moved from Google Code issue #605.
Reported by graham.mueller315, Nov 4, 2013
Most touch/mouse events occur on the Up event, instead of the down (short of dragging). However, all of the delegate methods for the various graphs seem to be using the Down event.
This causes problems with graphs that are embedded in scrollable views, as you try to scroll and the events are unexpectedly fired.
Would you consider changing this behavior in the next version?
Thanks,
Graham
Nov 4, 2013 comment #1 graham.mueller315
This will additionally cause issue when "accidentally clicking" (i.e, user accidentally grabs one bar, and tries to drag away to prevent actually selecting it). The expected behavior in this case is that the event would not be fired at all (or at least not for that bar), but currently it would be fired immediately on the accidental tap-down.
Nov 14, 2013 comment #2 [email protected]
Should we change the behavior of the existing selection methods or add new ones with the new behavior to give the user a choice? If we change the existing behavior, that will need to go into the release 2.0 branch along with the other major changes in progress over there.
Nov 14, 2013 comment #3 graham.mueller315
I think that allowing the user to choose would make sense, but the default should be (in my opinion) on Up.
Based on that, I believe this will need to go into the 2.0 Branch. Is that nearing release, or is that likely not going to happen yet this year?
Nov 15, 2013 comment #4 [email protected]
We don't have a timeline for the 2.0 release yet. There are still some major architecture and performance changes that need to happen that are not ready to check in. The 2.0 branch is functional—I know some people are already using it.
Nov 15, 2013 comment #5 [email protected]
(No comment was entered for this change.)
Dec 12, 2013 comment #6 graham.mueller315
Hi Eric,
Here are two patches that should resolve this issue for the 2.0 release. I took two different approaches, let me know if either would work for you.
The first was using a boolean to determine when to fire the event. It didn't change the API at all, but adds a property to all of the items that handle events on the pointingDeviceUp/Down. If the boolean is set to false, it only fires on the down event, if set to true (the default), it will fire on the up event. This seems a little kludgy, so I came up with a second change, that fully preserves (though deprecating) backwards compatibility.
The second approach is to have all of those items that handle the pointingDeviceUp/Down events fire both events, depending on what the delegate wants. These methods all have a new API looks like, for example, -(void)axis:(CPTAxis *)axis labelTouchDown:(CPTAxisLabel *)label withEvent:(CPTNativeEvent *)event;
.
This is nice because it allows the user to choose which events they want (it also enables them to respond to both.
Anyway, let me know what you think.
Graham
Dec 12, 2013 comment #7 [email protected]
Thanks. I will take a look at the patches this weekend.
Dec 13, 2013 comment #8 [email protected]
The boolean property makes for an awkward API. I prefer the solution that adds additional delegate methods. As you say, this gives the application a choice of which one to use.
Rather than deprecating and renaming all of the xxxWasSelected: methods, what if we instead add the "up" methods and leave the original methods as they are. We could call the "up" methods something like xxxWasReleased:. Since adding new APIs doesn't break old apps, this change wouldn't need to wait for the 2.0 release.
Dec 13, 2013 comment #9 graham.mueller315
If that's what you would prefer, it could certainly be done. However, to me "wasSelected" and "wasReleased" are not corresponding events.
Selection is really up/down agnostic, and seems like more of a "this item is now in focus" event.
The wasPressed/Released method would better indicate what is actually happen -- mouse/finger down/Up.
I realize you're suggesting this in favor of getting it into 1.x, but to me the breaking API change seems like a better fit, which would make it a better candidate for 2.0. That's just my opinion, though, so I can certainly modify my patch to make it fit your preference.
Dec 14, 2013 comment #10 [email protected]
I agree with the approach, I'm just looking for the right name for the event. Core Plot is platform agnostic; the names should make sense on both Mac and iOS. That's why we used "pointingDeviceXXX" for the CPTResponder
events and "xxxWasSelected:" for the selection events.
However, none of this addresses the issue you raised in the initial issue report or the first comment. Rather than deprecate "xxxWasSelected:", why not use those to indicate when the down and up events occur on the same plot point? For example, if you touch a bar on a bar plot, slide your finger to a different bar, and then lift the finger, you'll get the down event for the first bar and the up event for the second bar, but not the selected event. If the up event occurs on the same bar as the down event, you'll get the selected event, too.
I'm not sure if this change in behavior will fix the issue when embedded in a scroll view or if more needs to be done to handle that case, too. The solution might be different on Mac and iOS.
Dec 14, 2013 comment #11 graham.mueller315
I know what you mean about choosing the right name. I recognize the cross platform aspect of the framework, and had actually looked at a couple of other libs that are as well when I decided on the touch name.
Check out LibGDX's (a gaming lib) InputProcessor
class at http://libgdx.badlogicgames.com/nightlies/docs/api/ (sorry, I can't directly link to the class, JavaDocs kind of suck :/)
What you mention with touchUp/Down on different, non-matched points is actually an issue I had ran into when I was writing a Lua UI framework. It does actually get kind of tricky; what should the behavior be? If you started (down) on one, and ended (up) on another, are you actually trying to 'select' the latter, or are you canceling your selection of the first?
In that sense, rather than exposing either of those new events (up/down), maybe a change in the behavior of wasSelected would make sense -- it could only be fired when both up and down are triggered on the same point.
Regarding the scroll behavior, I believe this will work, though I guess I'm not 100% sure -- we put a separate work around in place to deal with it for this release, and I didn't try pulling it out to test. I'll give it a shot once we've decided on a strategy here, since presumably that's going to require some rework depending on the direction it goes.
Dec 15, 2013 comment #12 [email protected]
It does actually get kind of tricky; what should the behavior be? If you started (down) on one, and ended (up) on another, are you actually trying to 'select' the latter, or are you canceling your selection of the first?
That's why having both the up/down events and xxxWasSelected: would be useful. The application could choose which one is appropriate for what it is doing. xxxWasSelected: would only fire if the down and up events occur on the same data point.
I like your suggestion in comment #9 of using xxxWasPressed:
for the down events and xxxWasReleased:
for the up events.