Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

@FragmentByXXX inject child Fragments#1452

Merged
WonderCsabo merged 5 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
WonderCsabo:1302_childFragmentManagerWonderCsabo/androidannotations:1302_childFragmentManagerCopy head branch name to clipboard
Sep 21, 2015
Merged

@FragmentByXXX inject child Fragments#1452
WonderCsabo merged 5 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
WonderCsabo:1302_childFragmentManagerWonderCsabo/androidannotations:1302_childFragmentManagerCopy head branch name to clipboard

Conversation

@WonderCsabo

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

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

@mgod

mgod commented Jun 10, 2015

Copy link
Copy Markdown

@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
Copy Markdown
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

mgod commented Jun 10, 2015

Copy link
Copy Markdown

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
Copy Markdown
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

mgod commented Jun 11, 2015

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Member Author

PR updated.

@yDelouis

Copy link
Copy Markdown
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
Copy Markdown
Member Author

Wiki edited.

@WonderCsabo

Copy link
Copy Markdown
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

mgod commented Oct 15, 2015

Copy link
Copy Markdown

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
Copy Markdown
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.