-
Notifications
You must be signed in to change notification settings - Fork 2.3k
@FocusChange and @CheckedChange #459
Conversation
Add generation test and propper validation for @focuschange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zeroOrOneMenuItemParameters should be renamed to zeroOrOneMenuItemParameter
|
@mathieuboniface please, check :) |
|
Ok, i'm starting a full review. Github is saying "This pull request cannot be automatically merged." can you fix this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you test that this method is called when the checked state is changed (CompundButton.setChecked(boolean)) ?
You could checkout ClicksHandledActivity from the functional-test-1-5 module and ClicksHandledActivityTest from the functional-test-1-5-tests module as example.
|
👏 Really great work ! We just need more tests on Thank you @naixx :) |
|
Who's supposed to write the missing tests ? By the way, I guess master has moved too far: |
|
Hi! I can take a look in about a week |
|
Thanks. Sorry I haven't been involved in this pull request, so I'd rather let @mathieuboniface finish this with you :) |
|
Hi @naixx, I will work on merging that PR next week. Have you worked on the tests ? If not, I can wrote those tests. |
…istener_processor Conflicts: AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/AndroidAnnotationProcessor.java AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/helper/ValidatorHelper.java AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/ClickProcessor.java AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/LongClickProcessor.java AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/processing/TouchProcessor.java
|
Hello! I've just merged develop and resolved some conflicts. It will be easier now. But it fails some tests when I run |
|
Hi Rodolph, thanks for trying to help on merging :) I just merged your work into develop except the commit representing the merge of develop into your branch (#b8bf822) because it's really huge and so hard to understand. |
|
Woow, the this merge was not so nice... I fixed this on 7a83417 |
|
@mathieuboniface Thanks, great! |
Here is a not finished yet PR for
@FocusChange.I've made several changes: extracted abstract processor, which generates the same code for
@Click,@LongClick,@Touch. Everything that has same functionality as(T extends View).setOn***Listener()can use this, as well as@ChechedChangeand other annotations.I've also extracted everything that validates parameters to the separate class(it was i bit difficult to navigate, now everything is in one place). Concerning this, was it good idea to use public final field?
This PR also should eliminate stackoverflow problem, that
@Touchhad, whenonTouch()was annotated.There is no javadoc yet. Any feedback is welcome :)
@CheckedChangeadded