hoang8f / android-flat-button

FButton - a flat button library for Android
Apache License 2.0
1.4k stars 327 forks source link

Various about the code and the behavior #9

Open rogomantik opened 10 years ago

rogomantik commented 10 years ago

There are two warnings that eclipse shows:

  1. onTouch should call View#performClick when a click is detected
  2. constructors has setOnTouchListener called on it but does not override performClick

So for (2) I added @Override public boolean performClick() { // TODO Auto-generated method stub return super.performClick(); } ////////////////////////////////////////

For (1) I modified the method onTouch in this way: case MotionEvent.ACTION_MOVE: Rect r = new Rect(); view.getLocalVisibleRect(r); if (!r.contains((int) motionEvent.getX(), (int) motionEvent.getY())) { updateBackground(unpressedDrawable); this.setPadding(mPaddingLeft, mPaddingTop + mShadowHeight, mPaddingRight, mPaddingBottom + mShadowHeight); } break; //Changed updateBackground(pressedDrawable); to updateBackground(unpressedDrawable); //When the touch leaves the button, the button gets up /////////////////////////////////////// case MotionEvent.ACTION_UP: updateBackground(unpressedDrawable); this.setPadding(mPaddingLeft, mPaddingTop + mShadowHeight, mPaddingRight, mPaddingBottom + mShadowHeight); Rect r1 = new Rect(); view.getLocalVisibleRect(r1); if (r1.contains((int) motionEvent.getX(), (int) motionEvent.getY())) { view.performClick(); return true; } break;

//If I raise my finger when it is out of the button, the click is not executed otherwise (r1.contains == true) is performed

//I also changed the value that the method onTouch returns to false . I put a return true after view.performClick();

/////////////////////// I really do not have great knowledge but this way seems to resolve the fact that the button and the text are not synchronized (the text got up a little before the button). If, after pressing the button, you move your finger outside, the text moves up while the button no Then check if everything I've done is consistent :)

hoang8f commented 10 years ago

Hi @rogomantik First of all thank you for your very nice comment :+1:

//Changed updateBackground(pressedDrawable); to updateBackground(unpressedDrawable); //When the touch leaves the button, the button gets up

Yes, you are right ! I should use unpressedDrawable instead of pressedDrawable here.

//If I raise my finger when it is out of the button, the click is not executed otherwise (r1.contains == true) is performed //I also changed the value that the method onTouch returns to false . I put a return true after view.performClick();

I'm sorry, but I can't see the different when view.performClick() is put in. On my code, I used onTouch method only for changing the drawable resource purpose. Button's onClick event is handled by default Button method. So if ACTION_UP is called and the finger in Rect, the onClick is called as normal.

The problem is some cases the drawable is changed to "unpressed" state but onClick still called. for example, when you move the finger up to the top border of button, the button state change to unpressed. However, If the finger is raised at this time, the onClick event is called. I think I should correct the timing of changing drawable in order to make It is synchronized with "real" onClick event.

I really do not have great knowledge but this way seems to resolve the fact that the button and the text are not synchronized (the text got up a little before the button). If, after pressing the button, you move your finger outside, the text moves up while the button no Then check if everything I've done is consistent :)

If I understand correctly, the the fact that the button and the text are not synchronized will be resolved when we change updateBackground(pressedDrawable); to updateBackground(unpressedDrawable); . It is not related to performClick()'s stuff. Please correct me If i am wrong !

Thanks again :)

rogomantik commented 10 years ago

Thanks for your interest to my proposals!

I added this little part just for test and I noticed one thing (I also left the part added before) ///// case MotionEvent.ACTION_MOVE: Rect r = new Rect(); view.getLocalVisibleRect(r); if (!r.contains((int) motionEvent.getX(), (int) motionEvent.getY())) { updateBackground(unpressedDrawable); // era pressedDrawable this.setPadding(mPaddingLeft, mPaddingTop + mShadowHeight, mPaddingRight, mPaddingBottom + mShadowHeight); }else{ //THIS updateBackground(pressedDrawable); this.setPadding(mPaddingLeft, mPaddingTop + mShadowHeight, mPaddingRight, mPaddingBottom); } break; /////

I did some testing by performing this actions (emulator api19 - inside custom dialog with LinearLayout, not scrollable) the button start an intent action_view with an internet link, not using your sample application

1.touch the button 2.Move outside 3.return on button 4.unclick the mouse (raise finger)

with your code included pressedDrawable to unpressedDrawable

  1. button go down
  2. button go up
  3. button is still up
  4. no click event, no browser opened

whit my code

  1. button go down
  2. button go up
  3. button go down (else clause in this message)
  4. click event, open browser (I think it's due to the previous changes)

I can not explain the behavior of point 4 but may be due to view.performClick (); or to return true I did it just to eliminate warnings in eclipse, I don't know how it works exactly :) You are extremely free to ignore my suggestions

i tested in a srollview (big enough to not fit on the screen) too and if i press the button and I move even a pixel the focus go to the scroll, the button goes up and no click is performed (just because I saw that you updated the demo app with a ScrollView)

hoang8f commented 10 years ago

Hi @rogomantik Thank you for your detailed comments !

  1. button go down
  2. button go up
  3. button is still up
  4. no click event, no browser opened

I believed that "button is still up" is default behavior of original Button (a least on my phone). Thus, we should implement follow this behavior.

I can not explain the behavior of point 4 but may be due to view.performClick (); or to return true I did it just to eliminate warnings in eclipse, I don't know how it works exactly :) You are extremely free to ignore my suggestions

I think that I need more time to decide using view.performClick() or not. Anyway, the changing drawable code was push to master branch. Thank you so much :)

i tested in a srollview (big enough to not fit on the screen) too and if i press the button and I move even a pixel the focus go to the scroll, the button goes up and no click is performed (just because I saw that you updated the demo app with a ScrollView)

I will take a look at this. This might be a potential bug. I am going to compare with the behavior of original button.