Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Conversation

@WonderCsabo
Copy link
Member

Implements #1302.

This is a breaking change, because @EFragments now always use getFragmentManager() instead of getActivity().getFragmentManager() (or getChildFragmentManager() but only if that is set).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MyFragment extends Fragment? ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. :)

@WonderCsabo WonderCsabo force-pushed the 1302_childFragmentManager branch 3 times, most recently from 9d191fc to 02e8d75 Compare June 7, 2015 07:38
@WonderCsabo
Copy link
Member Author

@wintersandroid @mgod can you check if this is working for you?

@mgod
Copy link

mgod commented Jun 10, 2015

@WonderCsabo this fixes it for me. It's a bit of a pain as I expect I'll still be supporting APIs 15 and 16 for a while yet, but I can't tell if making this use the regular fragmentManager on API < 17 is useful.

@WonderCsabo
Copy link
Member Author

Sorry I do not really think understand what do you mean. We only support child Fragments from API 17, since that is when the method become avaialbe. And of course you can use the support lib.

@mgod
Copy link

mgod commented Jun 10, 2015

I can't use childFragment=true and have it work for an app that is compiled for min SDK version 15 and target SDK version 21, for example. It will correctly tell me that "The 'childFragmentManager' parameter only can be used if the getChildFragmentManager() method is available in Fragment (from API 17)".

Ideally (for me), it would use the getFragmentManager() method when using API < 17 instead of failing to compile. It seems like this would be fairly safe as if I'm using the childFragmentManager in the code as well, it should give me appropriate warnings that I need to handle that separately in API < 17.

@WonderCsabo
Copy link
Member Author

So if the device is below API 17, the following Fragment layout:

<LinearLayout >
    <fragment
        android:id="@+id/myChildFragment"
        android:name="com.foo.views.ChildMyFragment_"  />
</LinearLayout>

will add myChildFragment into the standard FragmentManager? If we want to support that case, we have to create a branch:

if (Build.SDK.VERSION_INT < 17) {
  myChildFragment = getFragmentManager().findFragmentById(R.id.myChildFragment);
} else {
  myChildFragment = getChildFragmentManager().findFragmentById(R.id.myChildFragment);
}

This is a fairly reasonable request.
However i do not think this should be the default behavior, it is better to use Support Fragments to provide a unified API in this case. Also, if we generate this silently, it could lead to potential bugs. So my suggestion is making this an explicit decision of the developer and adding a processing parameter useChildFragmentCallback, which is false by default, and only generates the structure above if set by the client. Otherwise AA will produce an error in case of API < 17 like the current solution does.

@yDelouis @dodgex @mgod WDYT?

@mgod
Copy link

mgod commented Jun 11, 2015

So it took me about 10 minutes to get this working with API 15 and support Fragments, so I think it probably makes more sense to keep the implementation and API for AA as is and make the error alert/docs say something more along the lines of:

"The 'childFragmentManager' parameter only can be used if the getChildFragmentManager() method is available in Fragment (from API 17). If you want to support older API versions, you should use android.support.v4.app.Fragment"

@WonderCsabo
Copy link
Member Author

Great! I will update the error message, then.

@WonderCsabo WonderCsabo force-pushed the 1302_childFragmentManager branch from 02e8d75 to 6d5ad41 Compare June 16, 2015 14:14
@WonderCsabo
Copy link
Member Author

PR updated.

@yDelouis
Copy link
Contributor

It's okay. Rebase and you can merge this PR.

@WonderCsabo WonderCsabo force-pushed the 1302_childFragmentManager branch from 8602ecf to 6f7ddbb Compare September 21, 2015 10:10
WonderCsabo added a commit that referenced this pull request Sep 21, 2015
@WonderCsabo WonderCsabo merged commit e4400a2 into androidannotations:develop Sep 21, 2015
@WonderCsabo WonderCsabo deleted the 1302_childFragmentManager branch September 21, 2015 10:52
@WonderCsabo WonderCsabo added this to the 4.0 milestone Sep 21, 2015
@WonderCsabo
Copy link
Member Author

Wiki edited.

@WonderCsabo
Copy link
Member Author

It turned out it may be not a good idea...

You cannot inflate a layout into a fragment when that layout includes a . Nested fragments are only supported when added to a fragment dynamically.

@yDelouis @dodgex

@mgod
Copy link

mgod commented Oct 15, 2015

Huh. I've been using nested fragments in XML layouts for a while now. It looks like apparently the android.support.v4.app.FragmentActivity does handle this ok or that the AA implementation somehow avoids the issues (I didn't have nested fragments before using AA). I'd certainly appreciate it if this feature stays around as otherwise I'm going to be stuck reimplementing some screens, but I agree that it doesn't look like it should be encouraged.

@WonderCsabo
Copy link
Member Author

@mgod I faced the problem when the parent Fragment is recreated, I got duplicate ID error. http://stackoverflow.com/a/19815266/747412

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants