Use FragmentFactory to construct Fragments for NavHostFragment [119054429]

Fixed

Feature Request

Status Update

No update yet.

Description

ru...@gmail.com

created issue #1

Nov 6, 2018 01:50AM

Component used: AndroidX Fragment, Navigation
Version used: Fragment 1.1.0-alpha01, Nav 1.0.0-alpha07
Devices/Android versions reproduced on: All

When using FragmentFactory attached to a fragment manager, FragmentNavigator$Destination.createFragment() throws an "InstantiationException: <Fragment> has no zero argument constructor". It appears that upon creating the nav graph, NavHotsFragment instantiates fragments itself instead of delegating to the fragment manager.

Comments

il...@google.com <il...@google.com> #2Nov 16, 2018 07:12AM

Assigned to il...@google.com.

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> Nov 26, 2018 05:53AM

Accepted by il...@google.com.

ap...@google.com <ap...@google.com> #3Nov 26, 2018 06:15PM

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

commit 1b215f0d93828286782776b9a12d2387097947f9
Author: Ian Lake <ilake@google.com>
Date: Sun Nov 25 21:45:56 2018

Move Fragment creation to FragmentNavigator

Provide an instantiateFragment() method on
FragmentNavigator which serves as the
centralized place to instantiate new
Fragment instances for FragmentNavigator.

Besides moving this business logic out of
the dumb POJO FragmentNavigator.Destination,
this also provides a future extension point
for integrating in FragmentFactory.

Test: existing tests passes, testapp still works
BUG: 119054429
Change-Id: If60471a704ef77bc35b30838e152a31cde8bc235

M navigation/fragment/ktx/src/androidTest/java/androidx/navigation/fragment/FragmentNavigatorDestinationBuilderTest.kt
M navigation/fragment/ktx/src/main/java/androidx/navigation/fragment/FragmentNavigatorDestinationBuilder.kt
M navigation/fragment/src/androidTest/java/androidx/navigation/fragment/FragmentNavigatorTest.kt
M navigation/fragment/src/main/java/androidx/navigation/fragment/FragmentNavigator.java

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

https://goto.google.com/android-sha1/1b215f0d93828286782776b9a12d2387097947f9

il...@google.com <il...@google.com> #4Nov 26, 2018 06:33PM

Navigation still depends on Support Library and not AndroidX, so we can't directly use FragmentFactory in FragmentNavigator.

However, in alpha08, you'll be able to subclass FragmentNavigator and override instantiateFragment(), calling into FragmentFactory if you're using AndroidX Fragment 1.1.0+. You'd also need to subclass NavHostFragment and override createFragmentNavigator() to return your custom FragmentNavigator subclass (or, if you're not using app:navGraph, you can set it via navHostFragment.navController.navigatorProvider += YourFragmentNavigator(context, navHostFragment.childFragmentManager)).

We'll keep this open until Navigation has moved to AndroidX and can directly use FragmentFactory as its default implementation of instantiateFragment().

ru...@gmail.com <ru...@gmail.com> #5Nov 27, 2018 04:44PM

Thanks for the workaround in alpha08!

[Deleted User] <[Deleted User]> #6Mar 5, 2019 05:21PM

Can't wait to use this library after the move to AndroidX.
For us is a major blocking feature and we don't want to proceed overriding all the necessary classes.

Thank you Ian and the rest of the navigation team.

il...@google.com <il...@google.com> Mar 8, 2019 09:58PM

Assigned to il...@google.com.

il...@google.com <il...@google.com> Mar 13, 2019 10:19PM

Accepted by il...@google.com.

ap...@google.com <ap...@google.com> #7Mar 14, 2019 05:15PM

Marked as fixed.

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

commit 74c3760e428648d00249dc1d38865b7802f48a9a
Author: Ian Lake <ilake@google.com>
Date: Wed Mar 13 13:11:57 2019

Use FragmentFactory in FragmentNavigator

Rely on the FragmentManager's FragmentFactory
to instantiate Fragments created for
FragmentNavigator.

Test: new FragmentNavigatorTest
Fixes: 119054429
Change-Id: Icbeb2771e29cafdc52e4c4ea770ce89f739921d1

M navigation/fragment/src/androidTest/java/androidx/navigation/fragment/FragmentNavigatorTest.kt
M navigation/fragment/src/main/java/androidx/navigation/fragment/FragmentNavigator.java

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

https://goto.google.com/android-sha1/74c3760e428648d00249dc1d38865b7802f48a9a

il...@google.com <il...@google.com> #8Mar 14, 2019 05:16PM

This is fixed internally - FragmentNavigator will use the FragmentFactory to create Fragments in the upcoming 2.1.0-alpha01 release.

[Deleted User] <[Deleted User]> #9Mar 15, 2019 10:31AM

Thank you Ian :)

ap...@google.com <ap...@google.com> #10Mar 18, 2019 06:05PM

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

commit 3dd6d1fcdb3ffa7d4bd265a023f0ac7c94e14d19
Author: Ian Lake <ilake@google.com>
Date: Mon Mar 18 09:36:37 2019

Deprecate FragmentNavigator.instantiateFragment

Instead of providing a custom subclass of
FragmentNavigator, instantiation of Fragments
should be controlled by setting a custom
FragmentFactory, which is the default behavior
of instantiateFragment() now.

By deprecating the class, we ensure that
developers who were overriding this method
are informed of the changes and can change
their code to use FragmentFactory.

Test: ./gradlew checkApi
BUG: 119054429
Change-Id: Iff46919face98baa6dc62835ca3fc105d987d141

M navigation/fragment/api/2.1.0-alpha01.txt
M navigation/fragment/api/current.txt
M navigation/fragment/src/main/java/androidx/navigation/fragment/FragmentNavigator.java

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

https://goto.google.com/android-sha1/3dd6d1fcdb3ffa7d4bd265a023f0ac7c94e14d19