From 58a626dedb16df61c431710058e5e95798bcfbf9 Mon Sep 17 00:00:00 2001 From: Mauricio Colli Date: Sat, 19 Oct 2019 21:31:15 -0300 Subject: [PATCH] Fix broken view pager tabs implementation - Fragments were being recreated from scratch (losing their state) every time some configuration change occurred (e.g. screen rotation). - Use `FragmentStatePagerAdapter` instead, as it is built to work with them and manage their states. --- .../newpipe/fragments/MainFragment.java | 69 ++++++++----------- .../org/schabi/newpipe/settings/tabs/Tab.java | 19 +++++ 2 files changed, 48 insertions(+), 40 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java index 085056302..79199a14d 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java @@ -1,5 +1,6 @@ package org.schabi.newpipe.fragments; +import android.content.Context; import android.os.Bundle; import android.util.Log; import android.view.LayoutInflater; @@ -15,7 +16,7 @@ import androidx.appcompat.app.ActionBar; import androidx.appcompat.app.AppCompatActivity; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentManager; -import androidx.fragment.app.FragmentPagerAdapter; +import androidx.fragment.app.FragmentStatePagerAdapter; import androidx.viewpager.widget.ViewPager; import com.google.android.material.tabs.TabLayout; @@ -52,32 +53,19 @@ public class MainFragment extends BaseFragment implements TabLayout.OnTabSelecte super.onCreate(savedInstanceState); setHasOptionsMenu(true); - destroyOldFragments(); - tabsManager = TabsManager.getManager(activity); tabsManager.setSavedTabsListener(() -> { if (DEBUG) { Log.d(TAG, "TabsManager.SavedTabsChangeListener: onTabsChanged called, isResumed = " + isResumed()); } if (isResumed()) { - updateTabs(); + setupTabs(); } else { hasTabsChanged = true; } }); } - private void destroyOldFragments() { - for (Fragment fragment : getChildFragmentManager().getFragments()) { - if (fragment != null) { - getChildFragmentManager() - .beginTransaction() - .remove(fragment) - .commitNowAllowingStateLoss(); - } - } - } - @Override public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) { return inflater.inflate(R.layout.fragment_main, container, false); @@ -90,23 +78,17 @@ public class MainFragment extends BaseFragment implements TabLayout.OnTabSelecte tabLayout = rootView.findViewById(R.id.main_tab_layout); viewPager = rootView.findViewById(R.id.pager); - /* Nested fragment, use child fragment here to maintain backstack in view pager. */ - pagerAdapter = new SelectedTabsPagerAdapter(getChildFragmentManager()); - viewPager.setAdapter(pagerAdapter); - tabLayout.setupWithViewPager(viewPager); tabLayout.addOnTabSelectedListener(this); - updateTabs(); + + setupTabs(); } @Override public void onResume() { super.onResume(); - if (hasTabsChanged) { - hasTabsChanged = false; - updateTabs(); - } + if (hasTabsChanged) setupTabs(); } @Override @@ -153,14 +135,21 @@ public class MainFragment extends BaseFragment implements TabLayout.OnTabSelecte // Tabs //////////////////////////////////////////////////////////////////////////*/ - public void updateTabs() { + public void setupTabs() { tabsList.clear(); tabsList.addAll(tabsManager.getTabs()); - pagerAdapter.notifyDataSetChanged(); - viewPager.setOffscreenPageLimit(pagerAdapter.getCount()); + if (pagerAdapter == null || !pagerAdapter.sameTabs(tabsList)) { + pagerAdapter = new SelectedTabsPagerAdapter(requireContext(), getChildFragmentManager(), tabsList); + } + // Clear previous tabs/fragments and set new adapter + viewPager.setAdapter(pagerAdapter); + viewPager.setOffscreenPageLimit(tabsList.size()); + updateTabsIconAndDescription(); updateCurrentTitle(); + + hasTabsChanged = false; } private void updateTabsIconAndDescription() { @@ -194,26 +183,30 @@ public class MainFragment extends BaseFragment implements TabLayout.OnTabSelecte updateCurrentTitle(); } - private class SelectedTabsPagerAdapter extends FragmentPagerAdapter { + private static class SelectedTabsPagerAdapter extends FragmentStatePagerAdapter { + private final Context context; + private final List internalTabsList; - private SelectedTabsPagerAdapter(FragmentManager fragmentManager) { - super(fragmentManager); + private SelectedTabsPagerAdapter(Context context, FragmentManager fragmentManager, List tabsList) { + super(fragmentManager, BEHAVIOR_RESUME_ONLY_CURRENT_FRAGMENT); + this.context = context; + this.internalTabsList = new ArrayList<>(tabsList); } @Override public Fragment getItem(int position) { - final Tab tab = tabsList.get(position); + final Tab tab = internalTabsList.get(position); Throwable throwable = null; Fragment fragment = null; try { - fragment = tab.getFragment(requireContext()); + fragment = tab.getFragment(context); } catch (ExtractionException e) { throwable = e; } if (throwable != null) { - ErrorActivity.reportError(activity, throwable, activity.getClass(), null, + ErrorActivity.reportError(context, throwable, null, null, ErrorActivity.ErrorInfo.make(UserAction.UI_ERROR, "none", "", R.string.app_ui_crash)); return new BlankFragment(); } @@ -234,15 +227,11 @@ public class MainFragment extends BaseFragment implements TabLayout.OnTabSelecte @Override public int getCount() { - return tabsList.size(); + return internalTabsList.size(); } - @Override - public void destroyItem(ViewGroup container, int position, Object object) { - getChildFragmentManager() - .beginTransaction() - .remove((Fragment) object) - .commitNowAllowingStateLoss(); + public boolean sameTabs(List tabsToCompare) { + return internalTabsList.equals(tabsToCompare); } } } diff --git a/app/src/main/java/org/schabi/newpipe/settings/tabs/Tab.java b/app/src/main/java/org/schabi/newpipe/settings/tabs/Tab.java index f80a8bb7f..cba3c4534 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/tabs/Tab.java +++ b/app/src/main/java/org/schabi/newpipe/settings/tabs/Tab.java @@ -28,6 +28,8 @@ import org.schabi.newpipe.util.KioskTranslator; import org.schabi.newpipe.util.ServiceHelper; import org.schabi.newpipe.util.ThemeHelper; +import java.util.Objects; + public abstract class Tab { Tab() { } @@ -47,6 +49,8 @@ public abstract class Tab { @Override public boolean equals(Object obj) { + if (obj == this) return true; + return obj instanceof Tab && obj.getClass().equals(this.getClass()) && ((Tab) obj).getTabId() == this.getTabId(); } @@ -340,6 +344,13 @@ public abstract class Tab { kioskId = jsonObject.getString(JSON_KIOSK_ID_KEY, ""); } + @Override + public boolean equals(Object obj) { + return super.equals(obj) && + kioskServiceId == ((KioskTab) obj).kioskServiceId + && Objects.equals(kioskId, ((KioskTab) obj).kioskId); + } + public int getKioskServiceId() { return kioskServiceId; } @@ -409,6 +420,14 @@ public abstract class Tab { channelName = jsonObject.getString(JSON_CHANNEL_NAME_KEY, ""); } + @Override + public boolean equals(Object obj) { + return super.equals(obj) && + channelServiceId == ((ChannelTab) obj).channelServiceId + && Objects.equals(channelUrl, ((ChannelTab) obj).channelUrl) + && Objects.equals(channelName, ((ChannelTab) obj).channelName); + } + public int getChannelServiceId() { return channelServiceId; }