-
Notifications
You must be signed in to change notification settings - Fork 3.5k
onCreateOptionsMenu is called for every Fragment inside the ViewPager #48
Comments
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.
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. |
@ChrisSmith can you paste/email/pull request your solution so I can take a look at it and possibly implement it? |
Sure, I hadn't posted it because its a hack and I'm not really happy with it. At the end of
It would be better if this could be handled by the |
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. |
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 |
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 |
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 |
That's actually pretty close to what I did. I modified the
Then in my base Fragment class
So the
This applies to #56 because inside |
Thanks for the patch. It inspired me to find a solution which did not require modifying 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. |
The menu items appear to be working correctly with your implementation. However I feel like modifying the internals of |
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 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. |
…booleans that come with participating in a ViewPager. Refs #48.
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 What do you think about that? |
…e technique that will rely on user implementation to fix the actual bugs. Samples to follow.
That seems fine to me. Because of where I had placed the call to notify the listener ( inside |
The reason I moved it to Also Dianne weighed in on the situation on the mailing list: https://groups.google.com/d/msg/android-developers/_opT-Lr_hgE/f4dLIpHbK-4J |
Nice to see the team taking a look at this. |
Released with 3.2.0 |
Can Anybody please help me, i am trying to use both Vertical and Horizontal Swipes in one Pager.? Please help to solve this.! |
When using Fragments inside the ViewPager, every Fragment that is partially visible (left page, right page and visible page) and has called setHasOptionsMenu(true) will have onCreateOptionsMenu called, causing the items to appear in the action bar.
The correct (implied?) behavior would be to only have the currently displayed Fragment show its menu items.
The issue comes down to how ViewPager operates, it holds up to three Fragments in its ViewGroup at a time, and they are all added to the FragmentManager. So when FragmentManager.dispatchCreateOptionsMenu is called, all of the menu items are created as-if all the Fragments were visible (because they technically are).
So far I've been unable to successfully prevent this behavior (using the public class methods) because of the order ViewPager.onPageSelected is called in. Since it is called before the next Fragment is added to the ViewGroup, I can't disable the fragments menu. 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.
I'm not sure how you want to handle this one, the issue is outside ActionBarSherlock but within the ACL. I guess filing a bug to source.android.com?
The text was updated successfully, but these errors were encountered: