ncapdevi / FragNav

An Android library for managing multiple stacks of fragments
1.5k stars 220 forks source link

switch tab without detach fragments #11

Closed xxxifan closed 7 years ago

xxxifan commented 8 years ago

I found that it will detach fragment once switch tabs, if so, fragments will recreate view while attach and some operation like data load will be performed every time. can we have a option to not detach/attach just hide/show fragment?

two1stnamz commented 8 years ago

I had this issue too. A way around this is to save the View that gets inflated as in field of your Fragment class. Then in your fragment's onCreateView() method you can check if your View field is null - if it is, inflate it, else return your already inflated View. Similarly, for data load, only load data if your View field is null, else do nothing.

JadenGu commented 8 years ago

Same issue..

hauthorn commented 8 years ago

Use the answer from @two1stnamz, or just cache data, and not the entire view. The idea from the Material Design spec is that the view should reset state. Caching data and/or some of the state might be a good idea for performance reasons.

codeteo commented 7 years ago

Can you elaborate I bit more on : A way around this is to save the View that gets inflated as in field of your Fragment class. ?? Is it different from saving/restoring fragment's state ?

ncapdevi commented 7 years ago

Considering adding this as an enhancement moving forward. Reasonably quick to add, but shown/hidden state isn't maintained and would need to be added to the savedInstanceState. If anyone wants to make a PR for this, it would be helpful.

two1stnamz commented 7 years ago

@codeteo Fragment.onCreate() returns a View. When you inflate your View, save it off as a field of your fragment class. So something like this:

public class MyFragment extends Fragment {

   private View cachedView;

   public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
    if (cachedView == null) {
      // Inflate and populate
      cachedView = inflater.inflate(R.layout.fragment_layout, container, false);
   }

   return cachedView;
  }

}
ncapdevi commented 7 years ago

@codeteo Yes, different then the fragment's state. http://daniel-codes.blogspot.com/2012/06/fragment-transactions-reference.html here's a quick read up on the difference between show/hide and detach/reattach. What is suggested above is to save a field variable of the View, so that you don't have to reinflate the view. When the view is detached, it's view is destroyed, but all of its field variables are maintained. The problem with using "Hide" on a bunch of views is that it can be fairly taxing on memory (why it destroys the view on detach instead of maintaining it) so the developer has to be careful. That said, it shouldn't be up to the library to determine that, and should be up to the developer to decide to detach vs hide.

hauthorn commented 7 years ago

I agree @ncapdevi. It is a pain to debug memory issues - and this library shouldn't be the cause of any :)

ncapdevi commented 7 years ago

@hauthorn @two1stnamz Any suggestions on how this should be implemented. It can either be

  1. A global final setting defined in the constructor
  2. A setting that can be toggled via a setter
  3. A parameter used in every function call. (e.g, push(boolean bDetach))

Not sure what the most common use case is here.

hauthorn commented 7 years ago

I don't think you should change your code to accomodate this, @ncapdevi.

The extra setting might confuse future developers, and it promotes heavy memory usage (which might trigger an OOM error for the developer later on).

ncapdevi commented 7 years ago

@hauthorn Ah! I thought you were requesting it be added as well. Yeah, that's my current thinking as well. Allowing it encourages bad behavior without devs fully realizing what's happening. I'm going to continue to leave it out for now unless a strong case is put forth to add it. Thanks for the feedback.

francescodagostino commented 7 years ago

For example, SharedElements ReturnTransition does not work for this reason, but there are so many other situations where i'd like to keep previous fragments attacched to my activity. More than othes, i'd like to have a method param like this pushFragment(Fragment frag, boolean keepAttached) or similar.