onCreateOptionsMenu is called for every Fragment inside the ViewPager about actionbarsherlock HOT 17 CLOSED

Yes, file a bug on b.android.com and add the link to this issue. You can also post to the official android developers mailing list where Dianne or another engineer might post a workaround.

My temporary solution was to modify ViewPager, but I doubt you want to go this route since ViewPager will likely change in future releases of the ACL.

On the contrary, I have already fixed a few bugs in the library and would gladly accept more. My release cycle is a lot quicker than Google's compat lib so it allows any changes to be distributed more rapidly.

from actionbarsherlock.

@ChrisSmith can you paste/email/pull request your solution so I can take a look at it and possibly implement it?

from actionbarsherlock.

Sure, I hadn't posted it because its a hack and I'm not really happy with it.

At the end of setCurrentItemInternal inside ViewPager, I just iterate over the fragments and disable their options menu.

    for(ItemInfo info : mItems){
            if(info.object instanceof Fragment){
                Fragment f = ((Fragment)info.object);
                f.setHasOptionsMenu(info.position==item);
            }
        }

It would be better if this could be handled by the OnPageChangeListener but the ArrayList<ItemInfo> mItems is private, and the ItemInfo class is too.

from actionbarsherlock.

You're right, that is a hack :) I filed this on b.android.com and will take a look at implementing a fix hopefully before the 3.1.3 release this weekend.

I'm fine with changing the behind-the-scenes code of the compat-lib sources as long as the exposed API doesn't change and it still functions as a drop-in replacement to the official jar.

from actionbarsherlock.

I've been plagued by this bug for ages. The fix above doesn't seem to work on the first item in a pager - I've found that adding it to the end of the populate() function works best.

from actionbarsherlock.

I think I ended up just manually setting it for the first one (but like I said this is a hack). I've got a better implementation now that also address Issue 56 that I'll post up soon

from actionbarsherlock.

Excellent I look forward to it. I was thinking of including a boolean in the info for each fragment as well to remember its original mHasMenu value. This way we handle a case where a fragment would (for who knows what reason) expose its menu in one situation but not another.

from actionbarsherlock.

That's actually pretty close to what I did.

I modified the OnPageChangeListener interface to include two more methods, and I adjusted the method I posted earlier

public void onPageDeselected(Object obj);
public void onPageSelected(Object obj);


public void updateSelectedPage(int item){
        if(mOnPageChangeListener==null) return;

        for(ItemInfo info : mItems){
            if(info.position==item){
                mOnPageChangeListener.onPageSelected(info.object);
            }else{
                mOnPageChangeListener.onPageDeselected(info.object);
            }
        }
    }

Then in my base Fragment class

    private boolean hasOptions = false;
    private boolean isHidden = false;

    public boolean isFragmentHidden(){
        return isHidden;
    }


    @Override
    public void setHasOptionsMenu(boolean hasOptions){
        this.hasOptions = hasOptions;
        super.setHasOptionsMenu(hasOptions);
    }

    @Override
    public void onFragmentShown() {
        isHidden = false;
        if(hasOptions) super.setHasOptionsMenu(true);
    }


    @Override
    public void onFragmentHidden() {
        isHidden = true;
        super.setHasOptionsMenu(false); 
    }

So the OnPageChangeListener is responsible for calling the two new methods as needed

        @Override
    public void onPageDeselected(Object obj) {
        ((MyFragment) obj).onFragmentHidden();
    }

    @Override
    public void onPageSelected(Object obj) {
        ((MyFragment) obj).onFragmentShown();
    }

This applies to #56 because inside public boolean onContextItemSelected(MenuItem item) we can first make a call to isFragmentHidden and return false, allowing other fragments to continue processing the context item.

from actionbarsherlock.

Thanks for the patch. It inspired me to find a solution which did not require modifying OnPageChangeListener (in order to maintain compliance with the official compat lib).

Give it a try (a60d03a) and see what you think. I didn't get a change to test it against #56 so if you have a test case for that please give it a try as well.

from actionbarsherlock.

The menu items appear to be working correctly with your implementation.

However I feel like modifying the internals of FragmentManager has more consequences than modifying OnPageChangeListener, ie completely different behavior on the 3.0 SDK and the ACL. That said, I'd like to see Google acknowledge this issue and let us know if they are going to address this or not. Besides anyone using ABS will likely stick with it for a substancial period of time just for the ActionBar pre-3.0 support.

from actionbarsherlock.

I prefer something along the lines of your method where determining whether or not to contribute a menu is left to the fragment implementation rather than handled by the library directly. On the other hand, I don't like changing APIs that I didn't create, even if they seem like something that should have existed in the first place.

The ViewPager is only available with the compat lib and thus modifying Fragment and FragmentManager would only affect users of this library. Since you are forced to use the library-implementations of those two classes as well, it shouldn't make a difference that we are tweaking the functionality. If future versions of the SDK do implement ViewPager natively then I would without a doubt opt to mirror their implementation.

I posted to the Android developers group so we'll see if anyone has an opinion there. It would be nice to catch Dianne's attention and get her input as she seems to be the steward of the compat lib.

I might try to hop in the weekly Hangout on G+ with Reto Meier and the other Android devs and/or Romain Guy on IRC to see if they have any thoughts as to what would be an appropriate direction to steer this.

I will hold off releasing until I can think about it more.

from actionbarsherlock.

So the more that I think about it the more that I think I am favoring a form of your solution. I was thinking of this method declaration:

void onPageSelectionStateChanged(int position, Object object, boolean selected);

Your use of two methods is a bit misleading because they may be called more than once for each state depending on the calls to populate(). I've just combined them into a single method which will allow you to react appropriately based on the selected argument. It also provides both the page position as the other callbacks do as well as the object to allow us to get our fragment instance out.

What do you think about that?

from actionbarsherlock.

That seems fine to me. Because of where I had placed the call to notify the listener ( inside setCurrentItemInternal) the methods were only called once and not repeatedly like populate() but it doesn't get called initially (as blork mentioned). The extra information will be useful to someone I'm sure.

from actionbarsherlock.

The reason I moved it to populate() was because its implementation removes items from the mItems array if you are jumping quickly or by more than one page via the API. This could probably be overcome by storing additional references to the ItemInfo instance for the last selected page and then placing the call only in setCurrentItemInternal. I'll have to experiment.

Also Dianne weighed in on the situation on the mailing list: https://groups.google.com/d/msg/android-developers/_opT-Lr_hgE/f4dLIpHbK-4J

from actionbarsherlock.

Nice to see the team taking a look at this.

from actionbarsherlock.

Released with 3.2.0

from actionbarsherlock.

Can Anybody please help me, i am trying to use both Vertical and Horizontal Swipes in one Pager.? Please help to solve this.!

from actionbarsherlock.