addisonElliott / SegmentedButton

Segmented Control/Button with animation for Android API 16+
Apache License 2.0
148 stars 39 forks source link

Fix IllegalStateException when dragging (#10) #11

Closed tkashkin closed 5 years ago

tkashkin commented 5 years ago

Do not throw IllegalStateException when x is out of bounds and return last button instead

addisonElliott commented 5 years ago

Thanks for the PR. I'll try to test this sometime this week.

I'm not sure this is the best course of action to take if the X position is out of bounds. My concern is that what if the touch event bounds given is not the last button and the button jumps a bit because the touch event causes it to move.

Rather, would it be better to return -1 (or NaN for float) and ignore the touch event?

What are your thoughts?

tkashkin commented 5 years ago

I'm not sure either, but this should happen only if user starts dragging and continues to drag to right until x becomes out of bounds. It seems appropriate to return last button in this case.

If user drags to left it will always return first button because x <= button.getRight() will always be true.

I haven't noticed any side effects of this fix.

addisonElliott commented 5 years ago

Okay, maybe this is the best course of action then.

It's odd because in theory, the touch events should not be generated if out of bounds. When I had this problem before, the error was because the bounds were not relative to the segmented button group but rather they were coming from a parent ScrollView which was messing up the X coordinate.

If that's not what is happening here then we should be good.

You're welcome to do this if you like, but I'll probably take your test case given in #10 and put it in the sample library. Then I'll make sure I can reproduce the bug and that it gets fixed with your fix. This will be good for future work to make sure we don't introduce the issue again.