JakeWharton / ActionBarSherlock

[DEPRECATED] Action bar implementation which uses the native action bar on Android 4.0+ and a custom implementation on pre-4.0 through a single API and theme.
http://actionbarsherlock.com
Apache License 2.0
7.1k stars 3.53k forks source link

Fragments added to ChildFragmentManager do not receive calls for OptionsMenus #828

Open purdyk opened 11 years ago

purdyk commented 11 years ago

The latest version of the support library brings in the ChildFragmentManager to all fragments. However, none of the fragments added through childFragmentManager will receive calls to any of the OptionsMenu functions.

Relevant code from the fragment class:

    boolean performCreateOptionsMenu(Menu menu, MenuInflater inflater) {
        boolean show = false;
        if (!mHidden) {
            if (mHasMenu && mMenuVisible) {
                show = true;
                onCreateOptionsMenu(menu, inflater);
            }
            if (mChildFragmentManager != null) {
                show |= mChildFragmentManager.dispatchCreateOptionsMenu(menu, inflater);
            }
        }
        return show;
    }
JakeWharton commented 11 years ago

Just a heads up. I'm not even going to look at this until r11 or newer is in Maven central.

purdyk commented 11 years ago

Resolution for the issue can be found on my fork: https://github.com/purdyk/ActionBarSherlock/commit/30750def631aa4cdd224d4c4550b23e27c245ac4 I will issue a pull request if you'd like when you move to r11.

Cheers.

SimonVT commented 11 years ago

That PR only works for 1 level of nested fragments tho. Our users might have more, you never know.

purdyk commented 11 years ago

Yes, I noticed this earlier today and resolved it in a later commit. Thanks for looking over it.
Properly recursive implementation is here: https://github.com/purdyk/ActionBarSherlock/commit/cab97d6a33685963b402b61db1343b3fea802598

SimonVT commented 11 years ago

Protip: Make your commits pass checkstyle checks

purdyk commented 11 years ago

Looks like a handy tool. I've gone over my changes and resolved all the offending lines created by my changes.

RawPhunky commented 11 years ago

purdyk I have tried your implementation and found another issue with ViewPager and FragmentPagerAdapter.

FragmentPagerAdapter calls setMenuVisibility... but it does not call it for child fragments.

Lets imagine that our Pager has 3 fragments. And one has a child fragment. All of them have menus. F1 F2 F3 F1.1

Now let's activate 2nd page with F2. FragmentPagerAdapter (on setPrimaryItem) will call F1.setMenuVisibility(false), but will not do it for F1.1

So our menu will be F2 + F1.1, not that we expected.

For a temp solution i have modified your code: I do not dispatch calls to child if parent fragments menu is invisible (you need to do that in all your recursive functions):

private boolean recurseOnPreparePanel(Fragment f, Menu menu)
{
    boolean show = false;
    if (f != null && !f.mHidden && f.mHasMenu && f.mMenuVisible && f instanceof OnPrepareOptionsMenuListener) {
        show = true;
        ((OnPrepareOptionsMenuListener)f).onPrepareOptionsMenu(menu);
    }

    if (!f.mMenuVisible)
        return show;

    // Dispatch calls to any child fragments
    if (f != null && f.mChildFragmentManager != null && f.mChildFragmentManager.mAdded != null)
    {
        for (int j = 0; j < f.mChildFragmentManager.mAdded.size(); j++)
        {
            Fragment f2 = f.mChildFragmentManager.mAdded.get(j);
            show = recurseOnPreparePanel(f2, menu);
        }
    }
    return show;
}
SimonVT commented 11 years ago

Whoops.

SimonVT commented 11 years ago

Make sure any changes your make matches the behavior of the native implementation.

Nevro commented 11 years ago

https://github.com/Nevro/ActionBarSherlock/commit/341490dbaedcbd5781cbf75f8833710380ab4505

tkeunebr commented 11 years ago

Hey guys! Do you have any idea on when this issue might be addressed?

Nevro commented 11 years ago

Probably never! All maven library stuck on android 4.1.1.4 and support library r7 forever... :(

tkeunebr commented 11 years ago

Well that's bad news... This issue is actually preventing me from shipping. I modified Watson.java using the above implementation but I'm still getting duplicated MenuItems when I use child fragments along together with a ViewPager.

Nevro commented 11 years ago

Try with latest abs version and with my watson class. If not work, commit some simple sample to your git and post url here...

mykola-dev commented 11 years ago

thanks Nevro, great fix!

m190 commented 11 years ago

Hi Nevro,

don't you know why inner fragments menu is not destroyed? :) I tried to use last abs and your fix, but once created, menu is shown all the time, even if you switch the tab. I debugged the code (Watson class), and it look like the menu is destroyed, but on the screen I see other things. Should I override onDestroyOptionsMenu or to do something other?

Thanks.

tkeunebr commented 11 years ago

I'm actually having the same issue, would be cool if someone had a fix :)

Nevro commented 11 years ago

Copy ViewPager from r12 and wait for official ActionBarCompat....

ekux44 commented 11 years ago

I can't believe this issue isn't more public. Took me an entire afternoon to isolate this issue in a codebase I'm backporting with ABS

geekkoz commented 11 years ago

thanks @purdyk your solution did work for me. :+1:

Shusshu commented 11 years ago

Same problem as @m190 with @purdyk or even @Nevro's code I also changed the support-v4 dependency to 13.0.0

Edit: I did menu.clear() in onCreateOptionsMenu before inflating the fragment's menu to solve the problem

rddimon commented 11 years ago

I have same problem as @m190 for solve this i did: @Override public void onPause() { setHasOptionsMenu(false); super.onPause(); } maybe another way to fix this in Watson class?

BurntBrunch commented 11 years ago

Does the fact that the SDK now ships with a maven repository for the support library (which, frankly, I find to be an awful hack) change the project's stance on this bug?

In case you didn't know, there's a repository (that the Gradle builds get automagically) in $ANDROID_HOME/extras/android/m2repository/

rddimon commented 11 years ago

I haven't read about Maven and Gradle). Thanks, I'll try use Marven.

SimonVT commented 11 years ago

Until either the support library is in maven central or android-maven-plugin picks up the repositories in the SDK automatically, nothing's going to happen on this issue.

LOG-TAG commented 11 years ago

can anybody explain in detail how to solve this bug? http://stackoverflow.com/questions/14137230/actionbar-menu-items-disappear-in-nestedfragments