Delete Item challenge shows FragmentStatePagerAdapter bug?


#1

All -

In working through the challenge to provide a delete option within CrimeFragment, I initially solved this successfully by adding the same crime_list_item_context resource to the options menu:

@Override public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { super.onCreateOptionsMenu(menu, inflater); inflater.inflate(R.menu.crime_list_item_context, menu); }

Then handling the resultant delete item being selected from the options menu by removing the Crime object from CrimeLab, and forcing the user back to the CrimeListFragment using the same NavUtils technique

    @Override
    public boolean onOptionsItemSelected(MenuItem item) {
        switch (item.getItemId()) {
            case R.id.menu_item_delete_crime:
                // Now have the Activity remove this fragment
                CrimeLab.get(getActivity()).deleteCrime(mCrime);
                // Fall through to the navigation
            
            case android.R.id.home:
                if (NavUtils.getParentActivityName(getActivity()) != null) {
                    NavUtils.navigateUpFromSameTask(getActivity());
                }
                return true;
            default:
                return super.onOptionsItemSelected(item);
        }
    }

This works fine, but it brings the user back to the list. I decided I wanted a different behavior after I delete the current Crime:

If CrimeLab is now empty, only then go back to the list (where it will show our prompt for the user to add a new Crime)
If CrimeLab is not empty and the user deleted the last crime in the list, have the ViewPager automatically move to the first item in the list without the user leaving the CrimeFragment view.
If CrimeLab is not empty and the user did not delete the last Crime, have the ViewPager move to the next item in the list without the user leaving the CrimeFragment view.

This proved to be extraordinarily difficult!

The Question
I’m going to ask this question twice, just in case a reader sees this post and can answer without having to wade though all my other input :wink:

Did anyone solve this challenge while staying within the ViewPager and not exiting back out to the list after the delete WITHOUT having to essentially re-write FragmentStatePagerAdapter? How did you do it?

The Problems
I ran into a myriad of problems, before I finally found a way to get this to work - but the way basically included finding and fixing a behavior in FragmentStatePagerAdapter by implementing a new class that essentially replicates its behavior but slightly changes the way the caching is done within the adapter.

First of all, I noticed that when I deleted the currently viewed Crime, even if I issued a notifyDataSetChanged() on the adapter housed in CrimePagerActivity I still saw the same fragment on the screen. This was so even when I specifically called mVewPager.setCurrentItem() using the same position as I had before (I felt that since I had indicated that the data set had changed, the adapter would reload itself appropriately using the updated data model then show me the position I had requested, which although was the same position as before, would now be populated with the data from the next Crime which would logically have been moved up - it did not).

Then I tried deleting an item in the middle of the list, and having the setCurrentItem() call always move to the first item in the list (instead of requesting the same position as before). This time I saw the page move to the first item in the list. However, when I swiped through the list, I realized that the item I deleted was still being shown, and the last item in the list had been deleted. I verified stepping through the debugger that I had in fact deleted the item from CrimeLab, yet here it was, showing up in the ViewPager! What the heck?

Googling some more, I found others had experienced this behavior. Then I found a couple of posts that explained that the culprit is how the FragmentStatePagerAdapter actually caches not only the fragment you are viewing and the number of fragments on either side of that fragment as controlled by ViewPager.setOffscreenPageLimit(), but actually SAVES FRAGMENT STATES even as it destroys the individual fragments themselves. It then RESTORES FRAGMENT STATES when you move across the same position in the list where those fragment states had been originally used.

Stated another way - assume a list of 4 Crimes:

Crime 1
Crime 2
Crime 3
Crime 4

Assume the ViewPager has me on Crime 3. Assume I delete that Crime. What I expect now is that I should have a list of Crimes 1, 2 and 4, and in fact this is what is contained in my CrimeLab. However, there is a Cache within the adapter that has actually kept the data from Crime 3, presumably to be able to restore it should I move from 3 to 4 and back to 3. This cached data is what is loaded into the fragment by the adapter NOT the data from the CrimeLab model!

See also this excellent explanation: http://speakman.net.nz/blog/2014/02/20/a-bug-in-and-a-fix-for-the-way-fragmentstatepageradapter-handles-fragment-restoration/

The Solution
If your goal is to stay within the ViewPager after you delete and be able to continue paging around, and you are using FragmentStatePagerAdapter, I don’t know of any other solution other than one similar to the one linked above that relies on comparing fragment tags within the adapter before assuming it should restore fragment state.

The Question
Did anyone solve this challenge while staying within the ViewPager WITHOUT having to essentially re-write FragmentStatePagerAdapter? How did you do it?

The Code
Finally, I decided to handle ALL the delete logic in the CrimePagerActivity itself, not in the CrimeFragment. This is because I found I needed to tell the adapter what it should do, possibly move back up the CrimeListFragment, etc. So I wrote an interface “CrimeSuicide” which was implemented by CrimePagerActivity and becomes the communication point between CrimeFragment and CrimePagerActivity. Why did I do this? I felt that rather than have CrimeFragment have to know it was being hosted by an activity that implemented FragmentStatePagerAdapter, it would support better reuse if it only had to assume its hosting activity implemented this interface.

So here is how I ended up with the code in CrimeFragment:

@Override public boolean onOptionsItemSelected(MenuItem item) { switch (item.getItemId()) { case R.id.menu_item_delete_crime: // Now have the Activity remove this fragment ((DeleteCurrentPageData)getActivity()).handleDeleteCurrentPageData(); return true; // Home icon which we enabled as a button got pressed? case android.R.id.home: if (NavUtils.getParentActivityName(getActivity()) != null) { NavUtils.navigateUpFromSameTask(getActivity()); } return true; default: return super.onOptionsItemSelected(item); } }

And in CrimePagerActivity:

[code] public void handleDeleteCurrentPageData() {
// Remove from the model
int position = mViewPager.getCurrentItem();
boolean onLastItem = ((position+1)==mViewPager.getAdapter().getCount());
mCrimes.remove(position);
mViewPager.getAdapter().notifyDataSetChanged();
if ( mCrimes.size() == 0 ) {
if (NavUtils.getParentActivityName(this) != null) {
NavUtils.navigateUpFromSameTask(this);
}

    } else {
        if ( onLastItem ) {
            // flip to first item
            mViewPager.setCurrentItem(0);
        } else {
            // Reload current item
            mViewPager.setCurrentItem(position);
        }
    }
}

[/code]

And finally, the newly minted FragementStatePagerAdapter:

[code] class CrimeFragmentStatePagerAdapter extends PagerAdapter {
private static final String TAG = “FragmentStatePagerAdapter”;
private static final boolean DEBUG = false;

    private final FragmentManager mFragmentManager;
    private FragmentTransaction mCurTransaction = null;

    private ArrayList<Fragment.SavedState> mSavedState = new ArrayList<Fragment.SavedState>();
    private ArrayList<Fragment> mFragments = new ArrayList<Fragment>();
    private ArrayList<String> mSavedFragmentTags = new ArrayList<String>();
    private Fragment mCurrentPrimaryItem = null;

    public CrimeFragmentStatePagerAdapter(FragmentManager fm) {
        mFragmentManager = fm;
    }

    public String getTag(int position) {

        if ( position >= 0 && position < mCrimes.size() ) {
            return mCrimes.get(position).getId().toString();
        }

        return null;
    }

    @Override
    public int getItemPosition(Object object) {
        return PagerAdapter.POSITION_NONE;
    }

    public Fragment getItem(int position) {
        Crime crime = mCrimes.get(position);

        // Utilize a mechanism to send an argument that contains the ID, rather than
        // use the activity's intent.
        return CrimeFragment.newInstance(crime.getId());
    }

    public int getCount() {
        return mCrimes.size();
    }


    @Override
    public void startUpdate(ViewGroup container) {
    }

    @Override
    public Object instantiateItem(ViewGroup container, int position) {
        // If we already have this item instantiated, there is nothing
        // to do.  This can happen when we are restoring the entire pager
        // from its saved state, where the fragment manager has already
        // taken care of restoring the fragments we previously had instantiated.
        if (mFragments.size() > position) {
            Fragment f = mFragments.get(position);
            if (f != null) {
                return f;
            }
        }

        if (mCurTransaction == null) {
            mCurTransaction = mFragmentManager.beginTransaction();
        }

        Fragment fragment = getItem(position);
        String fragmentTag = getTag(position);
        if (DEBUG) Log.v(TAG, "Adding item #" + position + ": f=" + fragment + " t=" + fragmentTag);
        if (mSavedState.size() > position) {
            String savedTag = mSavedFragmentTags.get(position);
            if (fragmentTag != null &&
                savedTag != null &&
                TextUtils.equals(fragmentTag,savedTag)) {

                Fragment.SavedState fss = mSavedState.get(position);
                if (fss != null) {
                    fragment.setInitialSavedState(fss);
                }

            }
        }
        while (mFragments.size() <= position) {
            mFragments.add(null);
        }
        fragment.setMenuVisibility(false);
        fragment.setUserVisibleHint(false);
        mFragments.set(position, fragment);
        mCurTransaction.add(container.getId(), fragment, fragmentTag);

        return fragment;
    }

    @Override
    public void destroyItem(ViewGroup container, int position, Object object) {
        Fragment fragment = (Fragment)object;

        if (mCurTransaction == null) {
            mCurTransaction = mFragmentManager.beginTransaction();
        }
        if (DEBUG) Log.v(TAG, "Removing item #" + position + ": f=" + object
                + " v=" + ((Fragment)object).getView() + " t=" + fragment.getTag());
        while (mSavedState.size() <= position) {
            mSavedState.add(null);
            mSavedFragmentTags.add(null);
        }
        mSavedState.set(position, mFragmentManager.saveFragmentInstanceState(fragment));
        mSavedFragmentTags.set(position, fragment.getTag());
        mFragments.set(position, null);

        mCurTransaction.remove(fragment);
    }

    @Override
    public void setPrimaryItem(ViewGroup container, int position, Object object) {
        Fragment fragment = (Fragment)object;
        if (fragment != mCurrentPrimaryItem) {
            if (mCurrentPrimaryItem != null) {
                mCurrentPrimaryItem.setMenuVisibility(false);
                mCurrentPrimaryItem.setUserVisibleHint(false);
            }
            if (fragment != null) {
                fragment.setMenuVisibility(true);
                fragment.setUserVisibleHint(true);
            }
            mCurrentPrimaryItem = fragment;
        }
    }

    @Override
    public void finishUpdate(ViewGroup container) {
        if (mCurTransaction != null) {
            mCurTransaction.commitAllowingStateLoss();
            mCurTransaction = null;
            mFragmentManager.executePendingTransactions();
        }
    }

    @Override
    public boolean isViewFromObject(View view, Object object) {
        return ((Fragment)object).getView() == view;
    }

    @Override
    public Parcelable saveState() {
        Bundle state = null;
        if (mSavedState.size() > 0) {
            state = new Bundle();
            Fragment.SavedState[] fss = new Fragment.SavedState[mSavedState.size()];
            mSavedState.toArray(fss);
            state.putParcelableArray("states", fss);
            state.putStringArrayList("tags", mSavedFragmentTags);
        }
        for (int i=0; i<mFragments.size(); i++) {
            Fragment f = mFragments.get(i);
            if (f != null) {
                if (state == null) {
                    state = new Bundle();
                }
                String key = "f" + i;
                mFragmentManager.putFragment(state, key, f);
            }
        }
        return state;
    }

    @Override
    public void restoreState(Parcelable state, ClassLoader loader) {
        if (state != null) {
            Bundle bundle = (Bundle)state;
            bundle.setClassLoader(loader);
            Parcelable[] fss = bundle.getParcelableArray("states");
            mSavedFragmentTags = bundle.getStringArrayList("tags");
            mSavedState.clear();
            mFragments.clear();
            if (fss != null) {
                for (int i=0; i<fss.length; i++) {
                    mSavedState.add((Fragment.SavedState)fss[i]);
                }
            }
            Iterable<String> keys = bundle.keySet();
            for (String key: keys) {
                if (key.startsWith("f")) {
                    int index = Integer.parseInt(key.substring(1));
                    Fragment f = mFragmentManager.getFragment(bundle, key);
                    if (f != null) {
                        while (mFragments.size() <= index) {
                            mFragments.add(null);
                        }
                        f.setMenuVisibility(false);
                        mFragments.set(index, f);
                    } else {
                        Log.w(TAG, "Bad fragment at key " + key);
                    }
                }
            }
        }
    }
}

[/code]


#2

Hi!

To get around this bug I just reset the adapter each time the delete button is pressed.

In the CrimeFragment I call a method of the hosting activity:

	@Override
	public boolean onOptionsItemSelected(MenuItem item) {
		switch(item.getItemId()) {
			case android.R.id.home:
				if(NavUtils.getParentActivityName(getActivity()) != null)
					NavUtils.navigateUpFromSameTask(getActivity());
				return true;
			case R.id.item_delete_task:
				((ActivityViewPagerTask)getActivity()).deletePage();
				return true;
			default:
				return super.onOptionsItemSelected(item);
		}
	}

Here is the deletPage method in the CrimePagerActivity:

	public void deletePage() {
		int index = mViewPager.getCurrentItem();
		TaskList.getTaskList(this).deleteTask(mTasks.get(index));
		mTasks = TaskList.getTaskList(this).getTasks();
		if(mTasks.size() == 0) {
			if(NavUtils.getParentActivityName(this) != null)
				NavUtils.navigateUpFromSameTask(this);
		}
		else {
			setPager();
			if(index == mPagerAdapter.getCount())
				mViewPager.setCurrentItem(index - 1);
			else
				mViewPager.setCurrentItem(index);
		}
	}

And here is the setPager(), which I also call in onCreate:

	private void setPager() {
		mPagerAdapter = new FragmentStatePagerAdapter(getSupportFragmentManager()) {
			@Override
			public Fragment getItem(int position) {
				Task task = mTasks.get(position);
				return FragmentTask.newInstance(task.getId());
			}

			@Override
			public int getCount() {
				return mTasks.size();
			}
		};
		mViewPager.setAdapter(mPagerAdapter);
	}

Works great.

There is another bug with ViewPager which causes action bar buttons to disappear. Reply 20 helped me fix that: code.google.com/p/android/issue … l?id=29472


#3

@clippership: thanks…ur article is great…it works everything fine…there is also one thing u just forgot otherwise all are great…

inside CrimeFragmentStatePagerAdapter, u never say how get mCrime values…also u dont mention how use this class tooo…this is my own code how get works for me

CrimePagerActivity.java

. . . . mViewPager.setAdapter(new CrimeFragmentStatePagerAdapter(fm,mCrimes)); . . . . .

CrimeFragmentStatePagerAdapter.java

. . . ArrayList\ mCrimes;

public CrimeFragmentStatePagerAdapter(FragmentManager fm,ArrayList<Crime> mCrimes)
{
mFragmentManager = fm;
this.mCrimes = mCrimes
}
.
.
.
.

any way thank once again