24

I have MenuItem defined this way:

<?xml version="1.0" encoding="utf-8"?>
<menu xmlns:android="http://schemas.android.com/apk/res/android">
    <item android:id="@+id/menu_starred"
        android:icon="@drawable/btn_star"
        android:title="@string/description_star"
        android:checkable="true"
        android:checked="true"
        android:orderInCategory="1"
        android:showAsAction="always" />
</menu>

and btn_star.xml defined this way:

<selector xmlns:android="http://schemas.android.com/apk/res/android">
    <item 
        android:state_checked="false" 
        android:drawable="@drawable/btn_star_off_normal" />
    <item 
        android:state_checked="true"
        android:drawable="@drawable/btn_star_on_normal" />
</selector>

When I create an options menu using this, however, the icon is never shown in its checked state, even if the MenuItem's isChecked() property is true.

I'm using the ActionBarSherlock control, however, I'm getting the same result if I simply create a normal options menu and call setChecked(true). It still displays the btn_star_off drawable regardless of the checked state of the item.

The onOptionsItemSelected() method is being called correctly, and I can successfully change the checked property:

@Override
    public boolean onOptionsItemSelected(MenuItem item) {
        if(item.isCheckable()) {
            item.setChecked(!item.isChecked());
        }
        return super.onOptionsItemSelected(item);
}

Setting a breakpoint here shows the isChecked property being changed, but the icon itself is not updated to reflect the correct state.

Is there something I'm missing here? Am I doing this incorrectly? I can't figure out why this wouldn't be working correctly.

Carl Manaster
  • 39,912
  • 17
  • 102
  • 155
Joe Krill
  • 1,734
  • 1
  • 18
  • 19

5 Answers5

41

According to the official document at http://developer.android.com/guide/topics/ui/menus.html

Note: Menu items in the Icon Menu (from the Options Menu) cannot display a checkbox or radio button. If you choose to make items in the Icon Menu checkable, you must manually indicate the checked state by swapping the icon and/or text each time the state changes.

Hope it helps.

gvaish
  • 9,374
  • 3
  • 38
  • 43
  • Ah, I don't know how I missed that -- I read through that documentation a few times. Thanks! – Joe Krill Aug 19 '11 at 12:37
  • 1
    The confusing part for me is that adding `checkable="true"` does add a checkbox to the menu item, and that this caveat only seems to apply to menu items in the icon menu rather than all menus. – Flimm Aug 19 '16 at 16:32
  • 4
    Google WHY? Why can't you just use state_checked of the drawable? – DrBreakalot May 28 '19 at 09:52
12

If you still want to have the behavior (checked, not checked) defined in a xml drawable, this is one way you could accomplish this:

if (item.getItemId()==R.id.menu_item){
    item.setChecked(!item.isChecked());
    StateListDrawable stateListDrawable = (StateListDrawable) getResources().getDrawable(R.drawable.selector_drawable);
    int[] state = {item.isChecked()?android.R.attr.state_checked:android.R.attr.state_empty};
    stateListDrawable.setState(state);
    item.setIcon(stateListDrawable.getCurrent());
}
Andrei Tudor Diaconu
  • 2,157
  • 21
  • 26
6

A bit simpler way (without xml-states file):

configChecked = !configChecked;
item.setChecked(configChecked);
item.setIcon(configChecked ? R.drawable.check_on : R.drawable.check_off);
kolyaseg
  • 535
  • 5
  • 13
3

Question is a bit old but I stumbled upon this problem recently.

After some analyzing it turns out that checked state of menu item is not being properly propagated down to the drawable. Here's the solution I came up with (in Kotlin).

Ensure your menuItem is using a state list drawable (like btn_star.xml from question), then create a drawable wrapper class:

/** Fixes checked state being ignored by injecting checked state directly into drawable */
class CheckDrawableWrapper(val menuItem: MenuItem) : DrawableWrapper(menuItem.icon) {
    // inject checked state into drawable state set
    override fun setState(stateSet: IntArray) = super.setState(
        if (menuItem.isChecked) stateSet + android.R.attr.state_checked else stateSet
    )
}

/** Wrap icon drawable with [CheckDrawableWrapper]. */
fun MenuItem.fixCheckStateOnIcon() = apply { icon = CheckDrawableWrapper(this) }

Last step is replacing menu items drawable with wrapped drawable:

override fun onCreateOptionsMenu(menu: Menu) : Boolean{
    menuInflater.inflate(R.menu.menu_main, menu)
    menu.findItem(R.id.menu_starred).fixCheckStateOnIcon()
    /** ...  */
    return true
}

After that you don't need to do anything when changing menu items checked state, icon should be self aware and react whenever checked state changes.

Pawel
  • 15,548
  • 3
  • 36
  • 36
  • DrawableWrapper Class requires API level 23 – A. Petrov Oct 20 '20 at 07:45
  • @A.Petrov I think you can use `androidx.appcompat.graphics.drawable.DrawableWrapper` instead, just add `@SuppressLint("RestrictedApi")` on top of the class. – Pawel Oct 20 '20 at 10:28
  • `@SuppressLint` is always a bad approach. Currently, I am ended up with the solution from @Andrei Tudor Diaconu with the major note: state-list drawable may contain many states (> 2) but only two first item-states are affected and can be used in a deal with `Toolbar.MenuItem` – A. Petrov Oct 20 '20 at 10:55
  • I'd argue if it's always a bad approach. I guess they put restricted annotation on top of that class because it's just a "crude helper class" and not an exposed api, but I don't see a problem ignoring it since it's just a simple delegate to wrapped object (you can check the source). – Pawel Oct 20 '20 at 11:53
  • >(you can check the source) - this is the key. Source on the moment. There is no guarantee to survive this API on evolution of Jetpack. – A. Petrov Oct 20 '20 at 12:32
  • Yes suppression is a "risk" but in my opinion it's non existent in this case since that class is very simple and was not modified since its creation (aside from androidx migration). But if you don't want to take it there's always an option of copying the class over to your project to have a stable version. – Pawel Oct 20 '20 at 13:18
1
<menu xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto">
    <item android:id="@+id/fav"
        android:title=""
        app:showAsAction="ifRoom"
        android:orderInCategory="1"
        android:icon="@drawable/ic_favorite_black_unselectd"
        android:checked="false" />
    <item android:id="@+id/share"
        android:title=""
        app:showAsAction="ifRoom"
        android:orderInCategory="2"
        android:icon="@drawable/ic_share_black" />
</menu>

//and in java...

@Override
public boolean onOptionsItemSelected(MenuItem item) {
    switch (item.getItemId()) {
        case R.id.fav:
            boolean mState = !item.isChecked();
            item.setChecked(mState);
            item.setIcon(mState ? getResources().getDrawable(R.drawable.ic_favorite_black_selected) : getResources().getDrawable(R.drawable.ic_favorite_black_unselectd));
            Toast.makeText(this, "" + item.isChecked(), Toast.LENGTH_SHORT).show();
            return true;
        case R.id.share:
            return true;
    }
    return super.onOptionsItemSelected(item);
}
Akhtar
  • 91
  • 1
  • 2