mikepenz/Android-ActionItemBadge

onOptionsItemSelected not called on Fragment

Closed this issue · 12 comments

The library binds the onclick event to Activity.onOptionsItemSelected. This results in only the Activity onOptionsItemSelected is called, the Fragments are not.

This event should be dispatched to the Activity.onMenuItemSelected method which properly calls onOptionsItemSelected on both the Activity and its Fragments.

for you there is now an OnOptionsItemSelected listener you can provide with the update method.

@mikepenz thanks!

However i think my proposed solution would be better (calling Activity.onMenuItemSelected). This way we would not have to create the redundant listener. And the selected item would have been dispatched the same way as normal action items. What do you think about it?

@WonderCsabo as the interface has the same method signature as the method in the Activity or the Fragment you should be able that your Activity or Fragment implements that interface and then you pass it with this so it uses the same method.

Also i added the separate method so it is more flexible than providing another method with a fragment too, as the Activity is required for showing the Toast when doing a LongClick

I still not understand why you do not have to want to use my suggestion. :S

@WonderCsabo i already call Activity.onOptionsItemSelected() which is the default method for those events as far as i know.

https://github.com/mikepenz/Android-ActionItemBadge/blob/develop/library/src/main/java/com/mikepenz/actionitembadge/library/ActionItemBadge.java#L195

@mikepenz i know! i asked you to actually replace that call with a call to onMenuItemSelected. That method correctly dispatches the event to the Activity and the Fragments. onOptionsItemSelected trivially only the Activity. Check out this excerpt from onMenuItemSelected:

if (onOptionsItemSelected(item)) {
    return true;
}
if (mFragments.dispatchOptionsItemSelected(item)) {
    return true;
}

But i can open a PR as well.

@WonderCsabo oh sorry i totally overread this difference. didn't know that there's a difference. i will probably just modify it to this method. and keep the possibility of the listener. thanks for your time and for pointing that out

@WonderCsabo one thing which you probably should make sure before i add this change:
http://stackoverflow.com/a/18133494/325479 Just to make sure that you also call the super methods in your activity (which will then forward it to the fragment)

The first few result from the mentioned method onMenuItemSelected just define that this is the more generic method, used for context menus and option menus

Actually i do not even override onCreateOptionsMenu and onOptionsItemSelected in my Activity, only in the Fragment, so this is not the problem.

Yeah, onMenuItemSelected is a more generic method, that is why you have to pass Window.FEATURE_OPTIONS_PANEL as the first parameter so it can know that an options menu event occurred. But this method is called when an item is clicked in the options menu, actually you can check it in the stacktrace. And this is the method which dispatches the event to the Activity and its Fragment. onOptionsItemSelected is only a listener method, it does not dispatches the event further. It can just return a boolean to indicate wether the event was consumed or not. The return value is actually used in onMenuItemSelected.

@WonderCsabo i now use your suggestion in v3.1.9

Great, thanks very much!

Finally i could test this, works as intended. :)