Bug P2
Status Update
No update yet.
Comments
[Deleted User] <[Deleted User]>
I am facing the same issue. In my case I am not re-ordering the items in the FragmentStatePagerAdapter, I only ever add an item to the end of the adapter or remove an item from the end of the adapter. Therefore, I decided to use a sledgehammer approach.
I created a complete copy of the FragmentStatePagerAdapter from the v4 support library and added this method
public void destroyItemState(int position) {
mFragments.remove(position);
mSavedState.remove(position);
}
Then in my subclass I override destroyItem like this:
@Override
public void destroyItem(ViewGroup container, int position, Object object) {
super.destroyItem(container, position, object);
if (getItemPosition(object) == POSITION_NONE) {
destroyItemState(position);
}
}
I created a complete copy of the FragmentStatePagerAdapter from the v4 support library and added this method
public void destroyItemState(int position) {
mFragments.remove(position);
mSavedState.remove(position);
}
Then in my subclass I override destroyItem like this:
@Override
public void destroyItem(ViewGroup container, int position, Object object) {
super.destroyItem(container, position, object);
if (getItemPosition(object) == POSITION_NONE) {
destroyItemState(position);
}
}
ad...@2fours.com
Great find. Something to be aware of is that this change now calls getCount() in the constructor so if you subclass and have a constructor and get count similar to this:
public ChatPagerAdapter(FragmentManager fm) {
super(fm);
mChatNames = new ArrayList<String>();
}
@Override
public int getCount() {
return mChatNames.size();
}
you will get a NPE on instantiation. Obviously the fix is easy but NewFragmentStatePagerAdapter is not a drop in replacement.
public ChatPagerAdapter(FragmentManager fm) {
super(fm);
mChatNames = new ArrayList<String>();
}
@Override
public int getCount() {
return mChatNames.size();
}
you will get a NPE on instantiation. Obviously the fix is easy but NewFragmentStatePagerAdapter is not a drop in replacement.
[Deleted User] <[Deleted User]>
I made the destroyItem() slightly different to be more in line with the original code:
public void destroyItemState(int position) {
mFragments.set(position, null);
if (position < mSavedState.size()) {
mSavedState.set(position, null);
}
}
public void destroyItemState(int position) {
mFragments.set(position, null);
if (position < mSavedState.size()) {
mSavedState.set(position, null);
}
}
bc...@google.com
How is this not getting more attention from the Android team, after being reported almost a year ago? Are there simply too few of us using FragmentStatePagerAdapter with the need to reorder? Or is this working as intended, with all of us somehow using it wrong?
vk...@gmail.com
For those looking at the fix in NewFragmentStatePagerAdapter, I do not think line 197:
position = getItemPosition(object);
in destroyItem(ViewGroup container, int position, Object object), should be there, and one should use the supplied position instead.
This is because the position supplied may in fact correspond to the old position of the object and not the new position as would be returned by getItemPosition.
Hope this helps.
position = getItemPosition(object);
in destroyItem(ViewGroup container, int position, Object object), should be there, and one should use the supplied position instead.
This is because the position supplied may in fact correspond to the old position of the object and not the new position as would be returned by getItemPosition.
Hope this helps.
au...@gmail.com
Response to comment 2:
There should be a null check in the `getCount()` method.
@Override
public int getCount() {
if (mData == null) return 0;
return mData.size();
}
Also, 40 stars and no comment from Google? This is getting ridiculous.
There should be a null check in the `getCount()` method.
@Override
public int getCount() {
if (mData == null) return 0;
return mData.size();
}
Also, 40 stars and no comment from Google? This is getting ridiculous.
ro...@google.com
wn...@gmail.com
The best way to use ViewPager is to use a custom implemented adapter instead of existing one.
ob...@gmail.com
I took the modifications in #1 and ported them to the latest code from the support library. I moved away from position to stable ids within the adapter subclass itself. You can find the code at https://gist.github.com/unverbraucht/a046369382a2f86f0a0f
zb...@gmail.com
#9 is a great solution! It doesn't handle resizing adapters though. Am trying to reuse to toggle showing a few pages in the adapter and then switching to show all.
ca...@gmail.com
#9 pointed me to a new version in GitHub, I forked and modified it a bit to allow my use case.
I wanted fragments {A, B, C} to become {B, C, D}.
https://github.com/Galvanize/sortable-fragment-pager
The code is probably not the best but I hope it can help someone.
I wanted fragments {A, B, C} to become {B, C, D}.
The code is probably not the best but I hope it can help someone.
ku...@gmail.com
#11 - This worked perfectly for my use case, which is simply adding new items at the front of the pager adapter, so that the item formerly at position 0 is shifted to position 1, etc.
I call notifyDataSetChanged() when new items are added, and I did have to implement getItemPosition(Object object).
Many thanks to contributors of #9 and #11. I spent hours trying various other solutions with no luck before finding this page.
I call notifyDataSetChanged() when new items are added, and I did have to implement getItemPosition(Object object).
Many thanks to contributors of #9 and #11. I spent hours trying various other solutions with no luck before finding this page.
re...@gmail.com
This 2.5 year old issue still exists in 2015 and will presumably never be fixed.
(I remember when Android development was generally productive fun... for some time it's seemed little more than an endless battle against idiotic/buggy fragment-related APIs.)
(I remember when Android development was generally productive fun... for some time it's seemed little more than an endless battle against idiotic/buggy fragment-related APIs.)
jo...@sirqul.com
Agreed #13. This issue makes working with fragments in a dynamic way impossible out of the box...
ra...@gmail.com
Wow still no love from google
zb...@gmail.com
I've had a lot more success with having two PagerAdapters and swapping one out and putting a new one in. It handles all of the stale ViewPager views for you.
The only thing to keep in mind is that you need a way to transfer the state of your fragments from one PagerAdapter to another. I do this by having an underlying model of the state of each page and when changing the order, I create a new PagerAdapter which has the exact state of the other page, just in a different order.
For me, I needed to strip out certain pages e.g. {A, B, C, D} -> {A, D}. Swapping out PagerAdapters worked for me :). No more handle re-orderings, etc.
The only thing to keep in mind is that you need a way to transfer the state of your fragments from one PagerAdapter to another. I do this by having an underlying model of the state of each page and when changing the order, I create a new PagerAdapter which has the exact state of the other page, just in a different order.
For me, I needed to strip out certain pages e.g. {A, B, C, D} -> {A, D}. Swapping out PagerAdapters worked for me :). No more handle re-orderings, etc.
a....@gmail.com
This is still an existing issue. Infact, if a new fragment is added to the front of the list, the current fragment suddenly becomes blank.
a....@gmail.com
Also,you guys seem to have fixed it here:
https://android.googlesource.com/platform/packages/apps/UnifiedEmail/+/master/src/com/android/mail/utils/FragmentStatePagerAdapter2.java .
" * <li>add support to handle data set changes that cause item positions to change</li>"
You could have back ported this change to the main library :/
" * <li>add support to handle data set changes that cause item positions to change</li>"
You could have back ported this change to the main library :/
bg...@gmail.com
Are we getting this in the latest build? Need this fix badly to release a product I have worked from past one month. I do not want to refresh whole grid and become costly on the memory and CPU. Thanks... replies appreciated.
yu...@gmail.com
FragmentStatePagerAdapter2.java. does not handle reordering of mSavedState.
I implemented fixed version based on recent FragmentStatePagerAdapter.java and it will be included to our production app.
I'll submit patch after ensuring that there is no problem.
https://gist.github.com/ypresto/8c13cb88a0973d071a64
I implemented fixed version based on recent FragmentStatePagerAdapter.java and it will be included to our production app.
I'll submit patch after ensuring that there is no problem.
yu...@gmail.com
My version handles reordering using unique integer from getItemId() (inspired by FragmentPagerAdapter).
So you should override getItemId() to make it work correctly.
So you should override getItemId() to make it work correctly.
zb...@gmail.com
Try making a new instance of PagerAdapter with the reordered data in. Saved me so much time and reordering code.
e.g.
public void updateUi() {
pagerAdapter = new MyPagerAdapter();
mPager.setAdapter(pagerAdapter);
}
e.g.
public void updateUi() {
pagerAdapter = new MyPagerAdapter();
mPager.setAdapter(pagerAdapter);
}
gu...@gmail.com
Guys, could you please suggest me the better solution for my case ?
My viewPager can hold maximum of 10 fragments. App will have a default fragment, and from there user can keep adding new fragments at the end of the list. Also he can delete any one from any position - zero position, last position or from middle. The existing FragmentStatePagerAdapter works fine when user deletes the page from the last position and adding at the end again. But when user deletes any page from middle and then if he tries to add page at the end, it shows blank page. I could find that the fragments were not getting deleted properly in the current solution. So, I have added below two lines in destroyItem() and it worked.
this.mFragments.remove(position);
this.mSavedState.remove(position);
But it shows ArrayIndexOutOfBoundException shoetimes at this.mFragments.set(position, null);. Below is the complete method with my two lines added. Can you please suggest me the best solution ? My app delivery to customer is in trouble due to this.
public void destroyItem(ViewGroup container, int position, Object object) {
Fragment fragment = (Fragment)object;
if(this.mCurTransaction == null) {
this.mCurTransaction = this.mFragmentManager.beginTransaction();
}
while(this.mSavedState.size() <= position) {
this.mSavedState.add(null);
}
this.mSavedState.set(position, this.mFragmentManager.saveFragmentInstanceState(fragment));
this.mFragments.set(position, null);
//////// These two lines added in addition to the original source from Android. ////////
this.mFragments.remove(position);
this.mSavedState.remove(position);
this.mCurTransaction.remove(fragment);
}
My viewPager can hold maximum of 10 fragments. App will have a default fragment, and from there user can keep adding new fragments at the end of the list. Also he can delete any one from any position - zero position, last position or from middle. The existing FragmentStatePagerAdapter works fine when user deletes the page from the last position and adding at the end again. But when user deletes any page from middle and then if he tries to add page at the end, it shows blank page. I could find that the fragments were not getting deleted properly in the current solution. So, I have added below two lines in destroyItem() and it worked.
this.mFragments.remove(position);
this.mSavedState.remove(position);
But it shows ArrayIndexOutOfBoundException shoetimes at this.mFragments.set(position, null);. Below is the complete method with my two lines added. Can you please suggest me the best solution ? My app delivery to customer is in trouble due to this.
public void destroyItem(ViewGroup container, int position, Object object) {
Fragment fragment = (Fragment)object;
if(this.mCurTransaction == null) {
this.mCurTransaction = this.mFragmentManager.beginTransaction();
}
while(this.mSavedState.size() <= position) {
this.mSavedState.add(null);
}
this.mSavedState.set(position, this.mFragmentManager.saveFragmentInstanceState(fragment));
this.mFragments.set(position, null);
//////// These two lines added in addition to the original source from Android. ////////
this.mFragments.remove(position);
this.mSavedState.remove(position);
this.mCurTransaction.remove(fragment);
}
bg...@gmail.com
Kill your old place holder and create new. That's they only solution
ra...@gmail.com
I want to add fragments to the beginning of pagerView. Is there a way to do this without visual image swapping.
sa...@gmail.com
four years and no fix. Governments change in that amount of time.
bg...@gmail.com
This has a work around. Needs no fix.
ಗುರುವಾರ, ಆಗಸ್ಟ್ 25, 2016 ದಿನಾಂಕದಂದು, <android@googlecode.com> ಅವರು ಹೀಗೆ
ಬರೆದಿದ್ದಾರೆ:
ಗುರುವಾರ, ಆಗಸ್ಟ್ 25, 2016 ದಿನಾಂಕದಂದು, <android@googlecode.com> ಅವರು ಹೀಗೆ
ಬರೆದಿದ್ದಾರೆ:
zb...@gmail.com
And what is that workaround? A new FragmentStatePagerAdapter implementation?
j....@gmail.com
Android Development: For the developer that loves implementing a feature that doesn't work as expected, scrambles to debug, and finally stumbles on the years-old reported bug with the 3rd-party fix that Google never absorbs into their support libraries.
Sure, this doesn't "need" a fix, only because others have done the work externally, but sheesh... give your platform devs the run-around more please (sarcasm).
Sure, this doesn't "need" a fix, only because others have done the work externally, but sheesh... give your platform devs the run-around more please (sarcasm).
wo...@gmail.com
How comes nobody cares about this? Its a crucial thing to be able to replace fragments in the view pager.
re...@gmail.com
I kinda like that it still exists to bite people in the ass and warn them off ever attempting to do anything non-trivial with Fragments. Like trying to use fragments within other fragments.
Fragments have always been a horrible broken mess and I greatly look forward to their eventual deprecation.
Fragments have always been a horrible broken mess and I greatly look forward to their eventual deprecation.
ni...@gmail.com
Having the same issue on the latest revision. it has been 4 years!!!
bg...@gmail.com
getItemPosition() will fail to refresh the first element in the array. I
faced this issue while doing this Multi-language calculator - IndiCal.
https://play.google.com/store/apps/details?id=com.cosmitude.calculator&hl=en
My Problem: On change of language, the Calculator first button in the array
would not get updated while all other buttons were updated to the new
language characters.
Work-around: Every time there is a change, re-draw the entire thing from
the beginning. This took a lot of time to figure out and fix but finally
took the re-draw approach.
faced this issue while doing this Multi-language calculator - IndiCal.
My Problem: On change of language, the Calculator first button in the array
would not get updated while all other buttons were updated to the new
language characters.
Work-around: Every time there is a change, re-draw the entire thing from
the beginning. This took a lot of time to figure out and fix but finally
took the re-draw approach.
pi...@googlemail.com
In the blog of Billy Ng http://billynyh.github.io/blog/2014/03/02/fragment-state-pager-adapter/ the error is described in detail and accurate. In my opinion, someone in Android Team should take the bug and fix the error. Such a bug which occurs in standard componentd and prevents many developers to develop stable apps should get higher priority.
ze...@gmail.com
Same issue here. I ended up copying the source code out and make the changes which works for me:
@Override
public void destroyItem(ViewGroup container, int position, Object object) {
....
// Fix the issue in instantiateItem get the wrong fragment.
//mFragments.set(position, null);
mFragments.remove(position);
}
I don't know why this is not yet fixed as a such important component.
@Override
public void destroyItem(ViewGroup container, int position, Object object) {
....
// Fix the issue in instantiateItem get the wrong fragment.
//mFragments.set(position, null);
mFragments.remove(position);
}
I don't know why this is not yet fixed as a such important component.
to...@gmail.com
I come here 4 years later and the bug is still not fixed? Haha what a joke! :)
ra...@shuklarahul.co.in
+1 to the developers being affected with this issue :)
ci...@gmail.com
+1 to the developers being affected with this issue
al...@gmail.com
+1 here
ki...@gmail.com
5 years, no solution.
po...@gmail.com
+1 here
er...@gmail.com
Half a day into this issue and finally found this bug report :D What a great great day.
tr...@gmail.com
Facing the same problem still !
pi...@googlemail.com
+1 Already not fixes.
ah...@gmail.com
Do bugs die?
ri...@racitup.com
Can someone CC this to Larry and Sergey?
This bug makes Apple look good..
This bug makes Apple look good..
al...@gmail.com
I've reimplemented the existing solution in Kotlin such that it allows you to return a String instead of a long for the item id. You can find it here: https://gist.github.com/SUPERCILEX/1e7fd5f50d38bfccaeffdf0eccda426a .
va...@starlingbank.com
The issue still reproducible. Shame on you!
pi...@googlemail.com
What is the current standing?
[Deleted User] <[Deleted User]>
This still appears to be happening. :(
ac...@googlemail.com
+1 Wasted so many hours until finally ending up here.
It'd be much appreciated if known issues were at least mentioned in the documentation.https://developer.android.com/reference/android/support/v4/app/FragmentStatePagerAdapter
Thank you!
It'd be much appreciated if known issues were at least mentioned in the documentation.
Thank you!
ha...@digiturk.com.tr
+1 to the developers being affected with this issue.
ca...@gmail.com
+1 to the developers being affected with this issue.
il...@google.com
jg...@google.com
Hi, apologies for not attending to this earlier. We are aware of the issue, and it has been one of the motivators to rewrite ViewPager - enter ViewPager2.
Can you try with ViewPager2?
Standalone sample:https://github.com/googlesamples/android-viewpager2
MutableCollectionFragmentActivity is the one showing, well, a collection which changes under the hood, so what you want there.
Release info:https://developer.android.com/jetpack/androidx/releases/viewpager2
At the time of writing we are in alpha03. Addressing a number of teething issues before alpha04 (early May).
Can you try with ViewPager2?
Standalone sample:
MutableCollectionFragmentActivity is the one showing, well, a collection which changes under the hood, so what you want there.
Release info:
At the time of writing we are in alpha03. Addressing a number of teething issues before alpha04 (early May).
Description
Attached is a simple example adapter (TestPagerAdapter.java) extending FragmentStatePagerAdapter. In the initial state, the order of the pages is {A, B, C}. Upon calling toggleState(), the order of the pages changes to {A, C, B}. By overriding getItemPosition(Object object), the adapter should ensure that the current page being viewed (A, B, or C) does not change after a toggle.
The example does not work. At least two types of incorrect behaviour may be observed.
1. An IndexOutOfBoundsException may occur. This can be reproduced by immediately calling toggleState() (while viewing page A, before swiping to any other page).
2. Empty pages may occur. This can be reproduced by swiping to B, swiping back to A, calling toggleState(), and swiping right to C. Instead of fragment C, we get an empty page.
Looking at the source of FragmentStatePagerAdapter, I figured out exactly what is going wrong. The FragmentStatePagerAdapter caches the fragments and saved states in ArrayLists: mFragments and mSavedState. But when the fragments are reordered, there's no mechanism for reordering the elements of mFragments and mSavedState. Therefore, the adapter will provide the wrong fragments to the pager.
I've attached a fixed implementation (NewFragmentStatePagerAdapter.java). I added a getItemId() function to FragmentStatePagerAdapter. (This mirrors the reordering implementation in FragmentPagerAdapter.) An array of the itemIds by adapter position is stored at all times. Then, in notifyDataSetChanged(), the adapter checks if the itemIds array has changed. If it has, then mFragments and mSavedState are reordered accordingly. Further modifications can be found in destroyItem(), saveState() and restoreState().
To use this class, getItemPosition() and getItemId() must be implemented consistently with getItem().
I don't claim that my NewFragmentStatePagerAdapter is a nice implementation. It's simply an implementation which reuses as much of the existing code as possible. In a more elegant implementation, I don't think that the fragments and saved states should be indexed by position.