-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fragment] Add onNewInstance callback. #323
Conversation
val end = SystemClock.uptimeMillis() | ||
|
||
firstRender = false | ||
fragmentDelegate.onFirstModelRendered( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this anymore because we can use onNewInstance
with setOutput
to fire the first model render
JaCoCo Code Coverage 79.66% ✅
Generated by 🚫 Danger |
* Creates a unique identifier the first time fragment is attached that | ||
* is persisted across configuration changes. | ||
*/ | ||
private fun initializeFragmentInstanceIdIfNeeded(f: Fragment) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved into getFormulaFragmentId
2449d07
to
ab6dd4c
Compare
return if (this is BaseFormulaFragment<*>) { | ||
val arguments = arguments ?: run { | ||
Bundle().apply { | ||
arguments = this | ||
} | ||
} | ||
val id = arguments.getString(FormulaFragment.ARG_FORMULA_ID, "") | ||
if (id.isNullOrBlank()) { | ||
val initializedId = UUID.randomUUID().toString() | ||
arguments.putString(FormulaFragment.ARG_FORMULA_ID, initializedId) | ||
initializedId | ||
} else { | ||
id | ||
} | ||
} else { | ||
"" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we cover this logic with tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question - we already cover this logic via tests in the formula-android
and formula-android-tests
modules. Specifically, the most important tests are the following:
formula/formula-android-tests/src/test/java/com/instacart/formula/FragmentFlowRenderViewTest.kt
Line 146 in 877827f
@Test fun `activity restart`() { formula/formula-android-tests/src/test/java/com/instacart/formula/FragmentFlowRenderViewTest.kt
Line 162 in 877827f
@Test fun `process death imitation`() {
What
Removing
onFirstModel
callback in favor of exposingonNewInstance
and moving the first model logic out of the core library.