How to properly handle screen rotation with a ViewPager and nested fragments?

Question

I've got this activity, which holds a fragment. This fragment layout consists of a view pager with several fragments (two, actually).

When the view pager is created, its adapter is created, getItem gets called and my sub fragments are created. Great.

Now when I rotate the screen, the framework handles the fragment re-creation, the adapter is created again in my onCreate from the main fragment, but getItem never gets called, so my adapter holds wrong references (actually nulls) instead of the two fragments.

What I have found is that the fragment manager (that is, the child fragment manager) contains an array of fragments called mActive, which is of course not accessible from code. However there's this getFragment method:

@Override
public Fragment getFragment(Bundle bundle, String key) {
    int index = bundle.getInt(key, -1);
    if (index == -1) {
        return null;
    }
    if (index >= mActive.size()) {
        throwException(new IllegalStateException("Fragement no longer exists for key "
                + key + ": index " + index));
    }
    Fragment f = mActive.get(index);
    if (f == null) {
        throwException(new IllegalStateException("Fragement no longer exists for key "
                + key + ": index " + index));
    }
    return f;
}

I won't comment the typo :)
This is the hack I have implemented in order to update the references to my fragments, in my adapter constructor:

// fm holds a reference to a FragmentManager
Bundle hack = new Bundle();
try {
    for (int i = 0; i < mFragments.length; i++) {
        hack.putInt("hack", i);
        mFragments[i] = fm.getFragment(hack, "hack");
    }
} catch (Exception e) {
    // No need to fail here, likely because it's the first creation and mActive is empty
}

I am not proud. This works, but it's ugly. What's the actual way of having a valid adapter after a screen rotation?

PS: here's the full code

Siusiubhan

same issue here : #16300126 Have you find something? – Unbreathed

Nope, still using my hack. I had already checkd the issue you're mentioning but this didn't solve my problem. – Siusiubhan

I am sorry but I am still learning Android and I cannot understand your hack. Can you explain it a bit more for me please? You put that code in your constructor, the fm is the FragmentManager given as argument, but what is mFragment? Why do you put all int to 0 ? thanks – Unbreathed

You're right there's a mistake that I forgot to fix here (it's fixed on my code). I will edit my answer and publish all the code. – Siusiubhan

nice! I tried just that code and it crashed instantly ^^ – Unbreathed

Check the updated question. I've uploaded a gist. Tell me how this works for you! – Siusiubhan

Thanks ! I correct the 0-->i. It still does not call getItem. can you explain me what's the purpose of all the bundle? I put my code below, if you see a mistake (all working good except rotation or any equivalent configuration change) – Unbreathed

I saw you corrected the typo in your gist in your 3rd revision, but it should be MyTabsAdapter constructor, not ViewPagerFragment, no? You need a constructor. I did not get it in the end. Not even how this hack should make getItem() to be called. But many thanks for your time :) – Unbreathed

Indeed. I have corrected it again :) Have you seen what I explained in the chat discussion? – Siusiubhan

sorry, first time i tried chat and i close the window ._. Find the room. – Unbreathed

Have a look at this: chat.stackoverflow.com/rooms/40748/… – Siusiubhan

Maybe this's useful for you: #7952230 – Leaven

Answer 1

I had the same issue - I assume you're subclassing FragmentPagerAdapter for your pager adapter (as getItem() is specific to FragmentPagerAdapter).

My solution was to instead subclass PagerAdapter and handle the fragment creation/deletion yourself (reimplementing some of the FragmentPagerAdapter code):

public class ListPagerAdapter extends PagerAdapter {
    FragmentManager fragmentManager;
    Fragment[] fragments;

    public ListPagerAdapter(FragmentManager fm){
        fragmentManager = fm;
        fragments = new Fragment[5];
    }

    @Override
    public void destroyItem(ViewGroup container, int position, Object object) {
        assert(0 <= position && position < fragments.length);
        FragmentTransaction trans = fragmentManager.beginTransaction();
        trans.remove(fragments[position]);
        trans.commit();
        fragments[position] = null;
}

    @Override
    public Fragment instantiateItem(ViewGroup container, int position){
        Fragment fragment = getItem(position);
        FragmentTransaction trans = fragmentManager.beginTransaction();
        trans.add(container.getId(),fragment,"fragment:"+position);
        trans.commit();
        return fragment;
    }

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

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

    public Fragment getItem(int position){
        assert(0 <= position && position < fragments.length);
        if(fragments[position] == null){
            fragments[position] = ; //make your fragment here
        }
        return fragments[position];
    }
}

Hope this helps.

Fusain

Yeah it took me 3 years to accept your solution. Thanks! :D – Siusiubhan

That's a solution, but the default behavior is unexpected, or maybe I'm missing something? Any idea what's the reason behind it? – Albertinaalbertine

Nice answer, but for a slightly simpler solution, check out https://mcmap.net/q/57829/-viewpager-and-fragments-what-39-s-the-right-way-to-store-fragment-39-s-state – Nadeen

Answer 2

Why solutions above so complex? Looks like overkill. I solve it just with replacing old reference to new in class extended from FragmentPagerAdapter

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

All adapter's code looks like this

public class RelationsFragmentsAdapter extends FragmentPagerAdapter {

private final String titles[] = new String[3];
private final Fragment frags[] = new Fragment[titles.length];

public RelationsFragmentsAdapter(FragmentManager fm) {
    super(fm);
    frags[0] = new FriendsFragment();
    frags[1] = new FriendsRequestFragment();
    frags[2] = new FriendsDeclinedFragment();

    Resources resources = AppController.getAppContext().getResources();

    titles[0] = resources.getString(R.string.my_friends);
    titles[1] = resources.getString(R.string.my_new_friends);
    titles[2] = resources.getString(R.string.followers);
}

@Override
public CharSequence getPageTitle(int position) {
    return titles[position];
}

@Override
public Fragment getItem(int position) {
    return frags[position];
}

@Override
public int getCount() {
    return frags.length;
}

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

}

Bermuda

Thanks, this is a great answer. Lightweight, fast, easy to implement, no static (leaky) references. And pretty easy to understand why it works. – Figured

but this will create new fragment instance everytime you rotate screen over and over again – Tidal

this answer is amazing – Reputed

Answer 3

My answer is kind of similar to Joshua Hunt's one, but by commiting the transaction in finishUpdate method you get much better performance. One transaction instead of two per update. Here is the code:

private class SuchPagerAdapter extends PagerAdapter{

    private final FragmentManager mFragmentManager;
    private SparseArray<Fragment> mFragments;
    private FragmentTransaction mCurTransaction;

    private SuchPagerAdapter(FragmentManager fragmentManager) {
        mFragmentManager = fragmentManager;
        mFragments = new SparseArray<>();
    }

    @Override
    public Object instantiateItem(ViewGroup container, int position) {
        Fragment fragment = getItem(position);
        if (mCurTransaction == null) {
            mCurTransaction = mFragmentManager.beginTransaction();
        }
        mCurTransaction.add(container.getId(),fragment,"fragment:"+position);
        return fragment;
    }

    @Override
    public void destroyItem(ViewGroup container, int position, Object object) {
        if (mCurTransaction == null) {
            mCurTransaction = mFragmentManager.beginTransaction();
        }
        mCurTransaction.detach(mFragments.get(position));
        mFragments.remove(position);
    }

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

    public Fragment getItem(int position) {         
        return YoursVeryFragment.instantiate();
    }

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


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

}

Siobhansion

Comment from mitenko: mFragments isn't being populated in instantiateItem and needs mFragments.put(position, fragment); inside or you'll end up with this error: [ Trying to remove fragment from view gives me NullPointerException on mNextAnim.](#22490203). – Gib

Answer 4

The problem is that getItem() in FragmentPageAdapter has a wrong name. It should have been named createItem(). Because the way it works the getItem() is for create fragments and it's not safe to call it to query/find a fragment.

My recomendation is to make a copy of current FragmentPagerAdapter and change it that way:

Add:

    public abstract Fragment createFragment(int position);

And change getItem to:

public Fragment getItem(int position) {
    if(containerId!=null) {
        final long itemId = getItemId(position);
        String name = makeFragmentName(containerId, itemId);
        return mFragmentManager.findFragmentByTag(name);
    } else {
        return null;
    }
}

Finally add that to instantiateItem:

    if(containerId==null)
        containerId = container.getId();
    else if(containerId!=container.getId())
        throw new RuntimeException("Container id not expected to change");

Complete code at this gist

I think that this implementation is safer and easier to use and also has same performance from the original adapter from Google engineers.

Amarillis

Answer 5

My code :

    public class SampleAdapter extends FragmentStatePagerAdapter {

    private Fragment mFragmentAtPos2;
    private FragmentManager mFragmentManager;
    private Fragment[] mFragments = new Fragment[3];


    public SampleAdapter(FragmentManager mgr) {
        super(mgr);
        mFragmentManager = mgr;
        Bundle hack = new Bundle();
        try {
            for (int i = 0; i < mFragments.length; i++) {
                hack.putInt("hack", i);
                mFragments[i] = mFragmentManager.getFragment(hack, "hack");
            }
        } catch (Exception e) {
            // No need to fail here, likely because it's the first creation and mActive is empty
        }

    }

    public void switchFrag(Fragment frag) {

        if (frag == null) {
            Dbg.e(TAG, "- switch(frag) frag is NULL");
            return;
        } else Dbg.v(TAG, "- switch(frag) - frag is " + frag.getClass());
        // We have to check for mFragmentAtPos2 null in case of first time (only Mytrips fragment being instatiante).
        if (mFragmentAtPos2!= null) 
            mFragmentManager.beginTransaction()  
                .remove(mFragmentAtPos2)
                .commit();
        mFragmentAtPos2 = frag;
        notifyDataSetChanged();
    }

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

    @Override
    public int getItemPosition(Object object) {
        Dbg.v(TAG,"getItemPosition : "+object.getClass());
        if (object instanceof MyTripsFragment
                || object instanceof FindingDriverFragment
                || object instanceof BookingAcceptedFragment
                || object instanceof RideStartedFragment
                || object instanceof RideEndedFragment
                || object instanceof ContactUsFragment
                )
            return POSITION_NONE;
        else return POSITION_UNCHANGED;

    }

    @Override
    public Fragment getItem(int position) {
        Dbg.v("SampleAdapter", "getItem called on: "+position);
        switch (position) {
        case 0: 
            if (snapbookFrag==null) {
                snapbookFrag = new SnapBookingFragment();
                Dbg.e(TAG, "snapbookFrag created");
            }
            return snapbookFrag;
        case 1: 
            if(bookingFormFrag==null) {
                bookingFormFrag = new BookingFormFragment();
                Dbg.e(TAG, "bookingFormFrag created");
            }
            return bookingFormFrag;
        case 2: 
            if (mFragmentAtPos2 == null) {
                myTripsFrag = new MyTripsFragment();
                mFragmentAtPos2 = myTripsFrag;
                return mFragmentAtPos2;
            }
            return mFragmentAtPos2;
        default:
            return(new SnapBookingFragment());
        }
    }
}

Unbreathed

Answer 6

With reference to simekadam's solution, mFragments isn't being populated in instantiateItem and needs mFragments.put(position, fragment); inside or you'll end up with this error: Trying to remove fragment from view gives me NullPointerException on mNextAnim.

Collis

I've added your comment to the answer. You can also suggest edits to post as long as it actually makes sense and doesn't change the meaning. – Gib