MenuItem's checked state is not shown correctly by its icon

Question

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.

Solution 1

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.

Solution 2

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());
}

Solution 3

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

configChecked = !configChecked;
item.setChecked(configChecked);
item.setIcon(configChecked ? R.drawable.check_on : R.drawable.check_off);

Solution 4

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.

Solution 5

<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);
}

View more solutions

Comments

[Joe Krill]

Ah, I don't know how I missed that -- I read through that documentation a few times. Thanks!

[Flimm]

What is R.drawable.selector_drawable?

[Flimm]

Why is the variable configChecked necessary?

[Andrei Tudor Diaconu]

The xml drawable that contains multiple states (focused, pressed, etc). In the example above, it would be btn_star.xml

[Flimm]

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.

[kolyaseg]

Flimm, because you have to change it to opposite value every time you click the checkbox

[DrBreakalot]

Google WHY? Why can't you just use state_checked of the drawable?

[A. Petrov]

DrawableWrapper Class requires API level 23

[Pawel]

@A.Petrov I think you can use androidx.appcompat.graphics.drawable.DrawableWrapper instead, just add @SuppressLint("RestrictedApi") on top of the class.

[A. Petrov]

@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

[Pawel]

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).

[A. Petrov]

>(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.

[Pawel]

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.