FragmentPagerAdapter is not removing items (Fragments) correctly

Question

I have implemented the FragmentPagerAdapter and and using a List<Fragment> to hold all fragments for my ViewPager to display. On addItem() I simply add an instantiated Fragment and then call notifyDataSetChanged(). I am not sure if this is necessary or not.

My problem simply... start with fragment 1

[Fragment 1] 

add new Fragment 2

[Fragment 1] [Fragment 2]

remove Fragment 2

[Fragment 1]

add new Fragment 3

[Fragment 1] [Fragment 2]

When adding new fragments everything seems great. Once I remove a fragment and then add a newly instantiated fragment the old fragment is still displayed. When i go a .getClass.getName() it is giving me Fragment 3's name however I still see Fragment 2.

I believe this might be an issue with instantiateItem() or such but I thought the adapter was to handle that for us. Any suggestions would be great.

adapter code...

public class MyPagerAdapter extends FragmentPagerAdapter {
public final ArrayList<Fragment> screens2 = new ArrayList<Fragment>();


private Context context;

public MyPagerAdapter(FragmentManager fm, Context context) {
    super(fm);
    this.context = context;
}

public void removeType(String name){
    for(Fragment f: screens2){
        if(f.getClass().getName().contains(name)){ screens2.remove(f); return; }
    }
    this.notifyDataSetChanged();
}

public boolean addSt(String tag, Class<?> clss, Bundle args){
    if(clss==null) return false;
    if(!clss.getName().contains("St")) return false; 
    if(!args.containsKey("cId")) return false;
    boolean has = false;
    boolean hasAlready = false;
    for(Fragment tab: screens2){
        if(tab.getClass().getName().contains("St")){
            has = true;
            if(tab.getArguments().containsKey("cId"))
                if(tab.getArguments().getLong("cId") == args.getLong("cId")){
                    hasAlready = true;
                }
            if(!hasAlready){
                // exists but is different so replace
                screens2.remove(tab);
                this.addScreen(tag, clss, args, C.PAGE_ST);
                // if returned true then it notifies dataset
                return true;
            }
        }
        hasAlready = false;
    }

    if(!has){ 
        // no st yet exist in adapter
        this.addScreen(tag, clss, args, C.PAGE_ST);
        return true;
    }

    return false;
}

public boolean removeCCreate(){
    this.removeType("Create");  
    return false;
}

@Override
public int getItemPosition(Object object) {

   return POSITION_NONE; //To make notifyDataSetChanged() do something
  }

public void addCCreate(){
    this.removeCCreate();
    Log.w("addding c", " ");
    this.addScreen("Create C",  CreateCFragment.class, null, C.PAGE_CREATE_C);
}

public void addScreen(String tag, Class<?> clss, Bundle args, int type){
    if(clss!=null){
        screens2.add(Fragment.instantiate(context, clss.getName(), args));
    }
}

@Override
public int getCount() {
    return screens2.size();
}


@Override
public Fragment getItem(int position) {
    return screens2.get(position); 
}

}

I realize the code uses some "ghetto" means of determining the fragment type however I wrote this code strictly for testing functionality. Any help or ideas would be great as it seems that not many people ventured into the world of FragmentPagerAdapters.

Answer 1

I got same problem,and my solution was overring the method "destroyItem" as following.

@Override
public void destroyItem(ViewGroup container, int position, Object object) {
    FragmentManager manager = ((Fragment)object).getFragmentManager();
    FragmentTransaction trans = manager.beginTransaction();
    trans.remove((Fragment)object);
    trans.commit();
}

It's work for me,does anybody have another solutions?

Updated:

I found those code made Fragment removed unnecessary,so I added a condition to avoid it.

@Override
public void destroyItem(ViewGroup container, int position, Object object) {
    if (position >= getCount()) {
        FragmentManager manager = ((Fragment) object).getFragmentManager();
        FragmentTransaction trans = manager.beginTransaction();
        trans.remove((Fragment) object);
        trans.commit();
    }
}

Answer 2

Updated this post and included my solution (if someone can improve let me know)

Ok i've now solved my problem in a hackish way, but yeah it's working ;). If someone can improve my solution please let me know. For my new solution i now use a CustomFragmentStatePagerAdapter but it doesn't save the state like it should and stores all the Fragments in a list. This can cause a memory problem if the user has more than 50 fragments, like the normal FragmentPagerAdapter does. It would be great if someone can add the State-thing back to my solution without removing my fixes. Thanks.

So here's my CustomFragmentStatePagerAdapter.java

package com.tundem.webLab.Adapter;

import java.util.ArrayList;

import android.os.Bundle;
import android.os.Parcelable;
import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentManager;
import android.support.v4.app.FragmentTransaction;
import android.support.v4.view.PagerAdapter;
import android.util.Log;
import android.view.View;
import android.view.ViewGroup;

public abstract class CustomFragmentStatePagerAdapter extends PagerAdapter {
    private static final String TAG = "FragmentStatePagerAdapter";
    private static final boolean DEBUG = false;

    private final FragmentManager mFragmentManager;
    private FragmentTransaction mCurTransaction = null;

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

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

    /**
     * Return the Fragment associated with a specified position.
     */
    public abstract Fragment getItem(int position);

    @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.

        // DONE Remove of the add process of the old stuff
        /* if (mFragments.size() > position) { Fragment f = mFragments.get(position); if (f != null) { return f; } } */

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

        Fragment fragment = getItem(position);
        if (DEBUG)
            Log.v(TAG, "Adding item #" + position + ": f=" + fragment);
        if (mSavedState.size() > position) {
            Fragment.SavedState fss = mSavedState.get(position);
            if (fss != null) {
                try // DONE: Try Catch
                {
                    fragment.setInitialSavedState(fss);
                } catch (Exception ex) {
                    // Schon aktiv (kA was das heißt xD)
                }
            }
        }
        while (mFragments.size() <= position) {
            mFragments.add(null);
        }
        fragment.setMenuVisibility(false);
        mFragments.set(position, fragment);
        mCurTransaction.add(container.getId(), fragment);

        return fragment;
    }

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

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

        /*if (mCurTransaction == null) { mCurTransaction = mFragmentManager.beginTransaction(); } if (DEBUG) Log.v(TAG, "Removing item #" + position + ": f=" + object + " v=" + ((Fragment)
         * object).getView()); while (mSavedState.size() <= position) { mSavedState.add(null); } mSavedState.set(position, mFragmentManager.saveFragmentInstanceState(fragment));
         * 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);
            }
            if (fragment != null) {
                fragment.setMenuVisibility(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);
        }
        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");
            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);
                    }
                }
            }
        }
    }
}

Here's my normal FragmentAdapter.java

package com.tundem.webLab.Adapter;

import java.util.LinkedList;
import java.util.List;

import android.support.v4.app.FragmentManager;

import com.tundem.webLab.fragments.BaseFragment;
import com.viewpagerindicator.TitleProvider;

public class FragmentAdapter extends CustomFragmentStatePagerAdapter implements TitleProvider {
    public List<BaseFragment> fragments = new LinkedList<BaseFragment>();

    private int actPage;

    public FragmentAdapter(FragmentManager fm) {
        super(fm);
    }

    public void setActPage(int actPage) {
        this.actPage = actPage;
    }

    public void addItem(BaseFragment fragment) {
        // TODO if exists don't open / change to that tab
        fragments.add(fragment);
    }

    public BaseFragment getActFragment() {
        return getItem(getActPage());
    }

    public int getActPage() {
        return actPage;
    }

    @Override
    public BaseFragment getItem(int position) {
        if (position < getCount()) {
            return fragments.get(position);
        } else
            return null;
    }

    @Override
    public int getCount() {
        return fragments.size();
    }

    @Override
    public String getTitle(int position) {
        return fragments.get(position).getTitle();
    }

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

And this is the way i delete a Fragment. (I know it's a bit more than only .remove() ). Be free to improve my solution, you can also add this code somewhere in the adapter so yeah. It's up to the user who tries to implement this. I use this in my TabHelper.java (A class which handles all tab operations like delete, add, ...)

    int act = Cfg.mPager.getCurrentItem();
    Cfg.mPager.removeAllViews();
    Cfg.mAdapter.mFragments.remove(act);
    try {
        Cfg.mAdapter.mSavedState.remove(act);
    } catch (Exception ex) {/* Already removed */}
    try {
        Cfg.mAdapter.fragments.remove(act);
    } catch (Exception ex) {/* Already removed */}

    Cfg.mAdapter.notifyDataSetChanged();
    Cfg.mIndicator.notifyDataSetChanged();

Description of the Cfg. thing. I save the reference to those objects in a cfg, class so i can always use them without the need of a special Factory.java ...

Yeah. i hope i was able to help. Feel free to improve this, but let me know so i can improve my code too.

Thanks.

If i missed any code let me know.

---

My old answer also works but only if you have different Fragments. FileFragment, WebFragment, ... Not if you use one of those fragmenttypes twice.

I got it pseudo working for now. It's a really dirty solution and i'm still searching for a better one. Please help.

I changed the code, where i delete a tab to this one:

   public static void deleteActTab()
        {   
            //We set this on the indicator, NOT the pager
            int act = Cfg.mPager.getCurrentItem();
            Cfg.mAdapter.removeItem(act);
            List<BaseFragment> frags = new LinkedList<BaseFragment>();
            frags = Cfg.mAdapter.fragments;

            Cfg.mPager = (ViewPager)Cfg.act.findViewById(R.id.pager);
            Cfg.mPager.setAdapter(Cfg.mAdapter);
            Cfg.mIndicator.setViewPager(Cfg.mPager);

            Cfg.mAdapter.fragments = frags;

            if(act > 0)
            {
                Cfg.mPager.setCurrentItem(act-1);
                Cfg.mIndicator.setCurrentItem(act-1);
            }

            Cfg.mIndicator.notifyDataSetChanged();
        }

If someone can improve this code let me know. If someone can tell us the real answer for that problem. please add it here. There are many many people who experience this issue. I added a reputation of 50 for the one who solve it. I can also give a donation for the one who solve it.

Thanks

Answer 3

Maybe this answer helps you.

Use FragmentStatePagerAdapter instead of FragmentPagerAdapter.

Because FragmentPagerAdapter does not destory the views. For more information read this answer.

Answer 4

Taking "the best of both worlds" (I mean the answers by @Tericky Shih and @mikepenz) we have it short and simple:

public class MyPagerAdapter extends FragmentPagerAdapter {

    public ArrayList<Fragment> fragments = new ArrayList<Fragment>();    

    ...

    @Override
    public void destroyItem(ViewGroup container, int position, Object object) {
        super.destroyItem(container, position, object);
        if (position >= getCount()) fm.beginTransaction().remove((Fragment) object).commit();
    }

    @Override
    public int getItemPosition(Object object) {
        if (fragments.contains(object)) return fragments.indexOf(object);
        else return POSITION_NONE;
    }
}

The main difference is that if some fragment is not changed you don't have to destroy its view and don't have to return POSITION_NONE for it. At the same time, I've encountered a situation when ViewPager was holding a reference to an item which was already destroyed, therefore the check if (fragments.contains(object)) helps to determine if that item is not needed anymore.

Answer 5

I had a situation similar to yours. I recently needed to add and remove Fragments from the ViewPager. In the first mode, I have Fragments 0, 1, and 2 and in the second mode I have Fragments 0 and 3. I want Fragment 0 to be same for both modes and hold over information.

All I needed to do was override FragmentPagerAdapter.getItemId to make sure that I returned a unique number for each different Fragment - the default is to return "position". I also had to set the adapter in the ViewPager again - a new instance will work but I set it back to the same instance. Setting the adapter results in the ViewPager removing all the views and trying to add them again.

However, the trick is that the adapter only calls getItem when it wants to instantiate the Fragment - not every time it shows it. This is because they are cached and looks them up by the "position" returned by getItemId.

Imagine you have three Fragments (0, 1 and 2) and you want to remove "1". If you return "position" for getItemId then removing Fragment 1 will not work because when you try to show Fragment 2 after deleting Fragment 1 the pager/adapter will think it's already got the Fragment for that "position" and will continue to display Fragment 1.

FYI: I tried notifyDataSetChanged instead of setting the adapter but it didn't work for me.

First, the getItemId override example and what I did for my getItem:

public class SectionsPagerAdapter extends FragmentPagerAdapter
{
    ...

    @Override
    public long getItemId(int position)
    {
        // Mode 1 uses Fragments 0, 1 and 2. Mode 2 uses Fragments 0 and 3
        if ( mode == 2 && position == 1 )
            return 3;
        return position;
    }

    @Override
    public Fragment getItem(int position)
    {
        if ( mode == 1 )
        {
            switch (position)
            {
                case 0:
                    return <<fragment 0>>;
                case 1:
                    return <<fragment 1>>;
                case 2:
                    return <<fragment 2>>;
            }
        }
        else    // Mode 2
        {
            switch (position)
            {
                case 0:
                    return <<fragment 0>>;
                case 1:
                    return <<fragment 3>>;
            }
        }
        return null;
    }
}

Now the change of mode:

private void modeChanged(int newMode)
{
    if ( newMode == mode )
        return;

    mode = newMode;

    // Calling mSectionsPagerAdapter.notifyDataSetChanged() is not enough here
    mViewPager.setAdapter(mSectionsPagerAdapter);
}

Answer 6

Didn't work out for me. My solution was put FragmentStatePagerAdapter.java in my project, renamed to FragmentStatePagerAdapter2.java. In destroyItem(), I changed a little based on error logs. From

// mFragments.set(position, null);

to

if (position < mFragments.size())mFragments.remove(position);

Maybe you don't have the same problem, just check the log.Hope this helps someone!

Answer 7

After a lot of trying, i got it to work so that it removes or attaches a third fragment at the end position correctly.

Object fragments[] = new Object[3];
int mItems = 2;
MyAdapter mAdapter;
ViewPager mPager;


public void addFragment(boolean bool) {

    mAdapter.startUpdate(mPager);

    if (!bool) {
        mAdapter.destroyItem(mPager, 2, fragments[2]);
        mItems = 2;
        fNach = false;
    }
    else if (bool && !fNach){
        mItems = 3;
        mAdapter.instantiateItem(mPager,2);
        fNach = true;
    }
    mAdapter.finishUpdate(mPager);
    mAdapter.notifyDataSetChanged();

}

public class MyAdapter extends FragmentPagerAdapter {
    MyAdapter(FragmentManager fm) {
        super(fm);
    }

    @Override
    public int getCount() {
        return mItems;
    }

    @Override
    public CharSequence getPageTitle(int position) {
    ... (code for the PagerTitleStrip)
    }

    @Override
    public Fragment getItem(int position) {
        Fragment f = null;
        switch (position) {
            case 0:
                f = new Fragment1();
                break;
            case 1:
                f = new Fragment2();
                break;
            case 2:
                f = new Fragment3();
                break;
        }
        return f;
    }

    @Override
    public Object instantiateItem(ViewGroup container, int position) {
        Object o = super.instantiateItem(container,position);
        fragments[position] = o;
        return o;
    }

    @Override
    public void destroyItem(ViewGroup container, int position, Object object) {
        super.destroyItem(container, position, object);
        System.out.println("Destroy item " + position);
        if (position >= getCount()) {
                FragmentManager manager = ((Fragment) object).getFragmentManager();
                FragmentTransaction ft = manager.beginTransaction();
                ft.remove((Fragment) object);
                ft.commit();
        }

    }
}

Some clarification: to get the object reference to call destroyItem, i stored the objects returned from instantiateItem in an Array. When you are adding or removing fragments, you have to announce it with startUpdate, finishUpdate and notifyDataSetChanged. The number of items have to be changed manually, for adding you increase it and instantiate it, getItem creates it then. For deletion, you call destroyItem, and in this code, it is essential the position >= mItems, because destroyItem is also called if a fragment goes out of the cache. You don't want to delete it then. The only thing which doesn't work is the swiping animation. After removing the last page, the "cannot swipe left" animation is not restored correctly on the new last page. If you swipe it, a blank page is shown and it bounces back.

Answer 8

The real problem is that FragmentPagerAdapter uses the position of the fragment in your list as ID. So if you add a new List or simply remove items "instantiateItem" item will find different fragments for new items in list...

@Override
public Object instantiateItem(ViewGroup container, int position) {
    if (mCurTransaction == null) {
        mCurTransaction = mFragmentManager.beginTransaction();
    }

    final long itemId = getItemId(position);

    // Do we already have this fragment?
    String name = makeFragmentName(container.getId(), itemId);
    Fragment fragment = mFragmentManager.findFragmentByTag(name);
    if (fragment != null) {
        if (DEBUG) Log.v(TAG, "Attaching item #" + itemId + ": f=" + fragment);
        mCurTransaction.attach(fragment);
    } else {
        fragment = getItem(position);
        if (DEBUG) Log.v(TAG, "Adding item #" + itemId + ": f=" + fragment);
        mCurTransaction.add(container.getId(), fragment,
                makeFragmentName(container.getId(), itemId));
    }
    if (fragment != mCurrentPrimaryItem) {
        fragment.setMenuVisibility(false);
        fragment.setUserVisibleHint(false);
    }

    return fragment;
}

and

  private static String makeFragmentName(int viewId, long id) {
    return "android:switcher:" + viewId + ":" + id;
}

and

     * Return a unique identifier for the item at the given position.
 * <p>
 * <p>The default implementation returns the given position.
 * Subclasses should override this method if the positions of items can change.</p>
 *
 * @param position Position within this adapter
 * @return Unique identifier for the item at position
 */
public long getItemId(int position) {
    return position;
}

Answer 9

I have been having this same issue until I dawned on me, I was creating my PagerView from within another Fragment and not the main activity.

My solution was to pass the ChildFragment Manager into the constructor of the Fragment(State)PagerAdapter and not the Fragment Manager of the parent Fragment.

Using the ChildFragmentManager all the Fragments created by the ViewPagerAdapter are cleaned up automatically when the parent Fragment is destroyed.