johannilsson / android-actionbar

DEPRECATED Android Action Bar Implementation
1.3k stars 564 forks source link

Is there a way to cancel the selection from within onNavigationItemSelected? #27

Closed LorneLaliberte closed 13 years ago

LorneLaliberte commented 13 years ago

There doesn't seem to be a way to override the current selection from within onNavigationItemSelected, for example if there was an error loading the class for an intent.

I tried calling actionBar.setSelectedNavigationItem, but calling that from onNavigationItemSelected does not change what appears in the action bar...mSelectedIndex is set to the "clicked" position after the function returns:

                                //Execute call back, if exists
                                if (mListCallback != null) {
                                    mListCallback.onNavigationItemSelected(position, mListAdapter.getItemId(position));
                                }

                                if (position != mSelectedIndex) {
                                    mSelectedIndex = position;
                                    reloadDisplay();
                                }

Note also that getSelectedNavigationIndex() returns the previous selection index if called from within onNavigationItemSelected...that may or may not be by design, it depends whether onNavigationItemSelected is meant as a prefix or postfix event. You could set mSelectedIndex before calling onNavigationItemSelected, so the behaviour of getSelectedNavigationIndex() is consistent inside and outside an onNavigationItemSelected function...however the current approach allows you to get what the selected item was "before" the onClick event, and use the itemPosition parameter to get the new selection index, so it is probably preferable this way.

Suggestion: allow onNavigationItemSelected to override the selection, e.g. check the boolean return value, and only update mSelectedIndex if onNavigationItemSelected returned true.

JakeWharton commented 13 years ago

Switching those two pieces of code will fix both problems.

When you call setSelectedNavigationItem() within the callback it does update the selected item successfully but the value gets overridden once the callback completes and the original selected item index replaces it.

LorneLaliberte commented 13 years ago

Yeah, the only downside would be that you couldn't query what the selection was before the onClick. You could easily track it yourself in your OnNavigationListener if you needed to, though.

JakeWharton commented 13 years ago

I agree that tracking the previous selection(s) is an exercise best left to the implementer since it is trivial and not likely something that would be used by a lot of users.

Good catch on finding this bug, though.

johannilsson commented 13 years ago

Thank you, setting the selected index before executing the call back makes more sense.