Request: FragmentTransaction.xxx overloads that take Fragment class rather than instance [126124987] - Issue Tracker

Android Public Tracker >

App Development >

Jetpack (androidx) >

Request: FragmentTransaction.xxx overloads that take Fragment class rather than instance

Fixed

Feature Request

P3

Status Update

No update yet.

Description

lu...@ozrunways.com

created issue #1

Feb 25, 2019 06:00AM

I've been using the new FragmentFactory, added in androidx.fragment 1.1.0-alpha01, to use Dagger constructor injection for creating Fragment instances. It seems to be working well so far and is a big improvement over doing field injection.

It would be nice to have a overloads of FragmentTransaction.add() and replace() that take a Fragment class (instead of a Fragment instance).

This would improve the symmetry between how fragments are instantiated during restore, where the user code isn't involved in how FragmentFactory.instantiate() is called; and how they're instantiated and added in code, as typically done on onCreate/onViewCreated.

A motivating example in Kotlin - a Fragment that adds two child fragments.
The Activity has set a FragmentFactory. I'd like to use the same FragmentFactory during initial creation as will get used for restoration. I can't just call the default constructor on the Fragments, because there is none -- dagger is creating a provider that injects some dependencies. Doing this manually means grabbing the FragmentFactory from the FragmentManager and calling instantiate().
////
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
if (savedInstanceState == null) {
val fragmentFactory = childFragmentManager.fragmentFactory
childFragmentManager.beginTransaction()
.replace(R.id.map_container, fragmentFactory.instantiate(this.javaClass.classLoader!!, MapFragment::

class.java.name, null))
.replace(R.id.details_container, fragmentFactory.instantiate(this.javaClass.classLoader!!, DetailsFragment::

class.java.name, null))
.commitNow()
}
}
////

This would be much nicer if I didn't have to create the Fragment instance directly here (ie if the FragmentTrasaction handled creating the fragment using the FragmentFactory). This might look like this:
////
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
if (savedInstanceState == null) {
childFragmentManager.beginTransaction()
.replace(R.id.map_container, MapFragment::class.java))
.replace(R.id.details_container, DetailsFragment::class.java))
.commitNow()
}
}
////
The benefit being that I don't have to write the code to create the Fragments.

In some ways, this 2nd code snippet is similar to passing the information needed by androidx.fragment.app.FragmentState, so the same information can be used to initially create the Fragment as would get used to restore it. This means that the creation and restoration code paths are similar, which helps to make sure the app is doing this consistently.

Suggested new overloads:
FragmentTransaction.add(@NonNull Class<? extends Fragment> fragmentClass, @Nullable Bundle args, @Nullable String tag)
FragmentTransaction.add(@IdRes int containerViewId, @NonNull Class<? extends Fragment> fragmentClass, @Nullable
Bundle args, @Nullable String tag)
FragmentTransaction.add(@IdRes int containerViewId, @NonNull Class<? extends Fragment> fragmentClass, @Nullable Bundle args)
FragmentTransaction.add(@IdRes int containerViewId, @NonNull Class<? extends Fragment> fragmentClass) // null args
FragmentTransaction.replace(@IdRes int containerViewId, @NonNull Class<? extends Fragment> fragmentClass, @Nullable Bundle args, @Nullable String tag)
FragmentTransaction.replace(@IdRes int containerViewId, @NonNull Class<? extends Fragment> fragmentClass, @Nullable Bundle args)
FragmentTransaction.replace(@IdRes int containerViewId, @NonNull Class<? extends Fragment> fragmentClass) // null args

The reason for passing the classes (rather than string class names) is to make misspellings a build error and aid the IDE with refactorings.

Component used: androidx.fragment
Version used: 1.1.0-alpha04

Comments

il...@google.com <il...@google.com> Mar 8, 2019 06:23PM

Assigned to il...@google.com.

il...@google.com <il...@google.com> Jul 9, 2019 09:32PM

Accepted by il...@google.com.

ap...@google.com <ap...@google.com> #2Jul 9, 2019 10:02PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

il...@google.com <il...@google.com> #3Jul 9, 2019 11:41PM

Marked as fixed.

We've added overloads for add and replace that take a Class and use FragmentFactory to instantiate a Fragment instance from the class name as well as Kotlin reified methods to do the same without requiring you type YourFragment::class.java. This will be available in Fragments 1.2.0-alpha02.

lu...@ozrunways.com <lu...@ozrunways.com> #4Jul 10, 2019 04:06AM

Thanks for the explanation and link to the changeset, that's better than my original suggestion. Looking forward to trying it.

ap...@google.com <ap...@google.com> #5Jul 17, 2019 02:58PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 9bfc29b0becb32aa98d135f59be47db13d82a2a0
Author: Ian Lake <ilake@google.com>
Date: Tue Jul 16 16:48:32 2019

Add optional Bundle to FragmentTransaction Class APIs

When adding or replacing a Fragment with a
Class<? extends Fragment> (added in

), allow
developers to pass in an optional Bundle of arguments.

In FragmentTransaction itself, the methods take
a @Nullable Bundle, ensuring that developers are
aware that they are expected to pass arguments through
that method if they want them set.

In the Kotlin extensions, the Bundle is the last
parameter, with a default value of null.

Test: updated tests
Fixes: 137684600
Change-Id: Ic492556e92031d2116d394b561002f338e5b1824

M fragment/fragment-ktx/api/1.2.0-alpha02.txt
M fragment/fragment-ktx/api/current.txt
M fragment/fragment-ktx/api/restricted_1.2.0-alpha02.txt
M fragment/fragment-ktx/api/restricted_current.txt
M fragment/fragment-ktx/src/androidTest/java/androidx/fragment/app/FragmentTransactionTest.kt
M fragment/fragment-ktx/src/main/java/androidx/fragment/app/FragmentTransaction.kt
M fragment/fragment/api/1.2.0-alpha02.txt
M fragment/fragment/api/current.txt
M fragment/fragment/api/restricted_1.2.0-alpha02.txt
M fragment/fragment/api/restricted_current.txt
M fragment/fragment/src/androidTest/java/androidx/fragment/app/FragmentTransactionTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/FragmentTransaction.java

https://android-review.googlesource.com/1053563

https://goto.google.com/android-sha1/9bfc29b0becb32aa98d135f59be47db13d82a2a0

Reporter

lu...@ozrunways.com

Type

Priority

Severity

Status

Access

Default access

View

Assignee

il...@google.com

Verifier

--

Collaborators

CC

an...@google.com

lu...@ozrunways.com

Code Changes

--

Pending Code Changes

--

Staffing

--

AOSP ID

--

Estimate

--

Found In

--

Targeted To

--

Verified In

--

In Prod

ReportedBy