Remove reliance on No UI Fragments with set retain instance to store state #3042
Conversation
… view model for retention across configuration changes
📝 WalkthroughWalkthroughThis set of changes refactors state retention and dialog management throughout the application. Fragment-based state holders such as Additionally, dialog construction and display methods are standardized. Dialog classes such as Sequence Diagram(s)sequenceDiagram
participant Activity
participant ViewModelProvider
participant ContainerViewModel
participant TaskConnectorViewModel
participant BreadcrumbBarViewModel
Note over Activity: On Activity creation
Activity->>ViewModelProvider: get(ContainerViewModel)
ViewModelProvider-->>Activity: ContainerViewModel instance
Activity->>ViewModelProvider: get(TaskConnectorViewModel)
ViewModelProvider-->>Activity: TaskConnectorViewModel instance
Activity->>ViewModelProvider: get(BreadcrumbBarViewModel)
ViewModelProvider-->>Activity: BreadcrumbBarViewModel instance
Note over Activity,ContainerViewModel: Activity saves/restores state via ViewModel
Note over Activity,TaskConnectorViewModel: Activity manages background tasks via ViewModel
Note over Activity,BreadcrumbBarViewModel: Activity manages breadcrumb bar via ViewModel
sequenceDiagram
participant Activity
participant DialogClass
Note over Activity: When showing a dialog
Activity->>DialogClass: create dialog instance
Activity->>DialogClass: showNonPersistentDialog(Activity)
DialogClass->>Activity: buildDialog(Activity)
DialogClass->>Activity: show()
Suggested labels
Suggested reviewers
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
app/src/org/commcare/views/dialogs/AlertDialogFragment.java (1)
49-58:⚠️ Potential issueMissing null‑guards can crash on cancel / dismiss
underlyingDialogis dereferenced inonCancel()andonDismiss()without a
null‑check.
If neither the fragment argument nor theViewModelcontained a dialog (see
edge‑case above), a rotation followed by a user‑initiated cancel will raise an
NPE.@Override public void onCancel(DialogInterface dialog) { super.onCancel(dialog); - underlyingDialog.performCancel(dialog); + if (underlyingDialog != null) { + underlyingDialog.performCancel(dialog); + } }Mirror the same guard in
onDismiss.
♻️ Duplicate comments (1)
app/src/org/commcare/activities/EntitySelectActivity.java (1)
872-873: Same leak‑risk when saving adapter
containerViewModel.setData(adapter);repeats the leak noted above. Prefer
serialising lightweight state instead.
🧹 Nitpick comments (7)
app/src/org/commcare/views/dialogs/AlertDialogFragment.java (1)
60-70: RawViewModeltype loses generic information
get(ContainerViewModel.class)returnsContainerViewModelas a raw type,
forcing an unchecked cast later. Suppress the warning or keep the type:ContainerViewModel<CommCareAlertDialog> viewModel = new ViewModelProvider(requireActivity()) .get(ContainerViewModel.class); // unchecked(Optional) Create a typed
ViewModelProvider.Factoryso you can call
get(ContainerViewModel.class)safely without suppressions.app/src/org/commcare/activities/EntitySelectActivity.java (1)
341-348: Guard against duplicate observer registrationWhen the adapter is restored from the
ViewModel,setupUIFromAdapter()is
called, but later inonResumeSessionSafe()the same adapter registers the
DataSetObserveragain. If the activity repeatedly rotates, the observer
will be registered multiple times, leading to redundant callbacks.Add a boolean flag or unregister first:
if (adapter != null) { - adapter.registerDataSetObserver(mListStateObserver); + adapter.unregisterDataSetObserver(mListStateObserver); + adapter.registerDataSetObserver(mListStateObserver); }app/src/org/commcare/fragments/TaskConnectorViewModel.java (2)
16-19: Spelling: use “detach” instead of “deattach”
Minor but worth fixing to keep Javadoc professional and searchable.- * An activity is responsible to attach and deattach itself + * An activity is responsible to attach and detach itself
88-91: Raw return type breaks generics safety
ReturnCommCareTask<?, ?, ?, R>instead of rawCommCareTask.- public CommCareTask getCurrentTask() { + public CommCareTask<?, ?, ?, R> getCurrentTask() {app/src/org/commcare/views/dialogs/CommCareAlertDialog.java (1)
80-85: Potential doublesetViewcall
buildDialog()sets the view and thenfinalizeView()sets it again.
This is harmless but redundant and slightly wasteful.app/src/org/commcare/fragments/BreadcrumbBarViewModel.java (2)
60-75: Duplicate method name causes confusion
attachBreadcrumbBar(CommCareActivity)calls a private helper with the same name (attachBreadcrumbBar(AppCompatActivity, ActionBar)), making stack traces and code search harder. Consider renaming the helper toconfigureBreadcrumbActionBar(...).
286-293: Null‑handling fallback may hide upstream errors
WhenbuildContextTilereturnsnull, the code silently falls back torequestEntityContext(). If the primary call failed due to an unexpected condition (e.g. bad session state), the problem is swallowed. Add logging to track the first failure for easier debugging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
app/src/org/commcare/activities/CallOutActivity.java(2 hunks)app/src/org/commcare/activities/CommCareActivity.java(14 hunks)app/src/org/commcare/activities/CommCareSetupActivity.java(6 hunks)app/src/org/commcare/activities/DrawActivity.java(1 hunks)app/src/org/commcare/activities/EntitySelectActivity.java(5 hunks)app/src/org/commcare/activities/FormEntryActivityUIController.java(1 hunks)app/src/org/commcare/activities/GeoPointActivity.java(1 hunks)app/src/org/commcare/activities/HomeScreenBaseActivity.java(1 hunks)app/src/org/commcare/activities/LoginActivity.java(1 hunks)app/src/org/commcare/fragments/BreadcrumbBarViewModel.java(9 hunks)app/src/org/commcare/fragments/ContainerFragment.java(0 hunks)app/src/org/commcare/fragments/ContainerViewModel.java(1 hunks)app/src/org/commcare/fragments/TaskConnectorViewModel.java(2 hunks)app/src/org/commcare/preferences/AdvancedActionsPreferences.java(1 hunks)app/src/org/commcare/preferences/MainConfigurablePreferences.java(1 hunks)app/src/org/commcare/utils/AndroidShortcuts.java(2 hunks)app/src/org/commcare/utils/GeoUtils.java(1 hunks)app/src/org/commcare/utils/Permissions.java(2 hunks)app/src/org/commcare/utils/TemplatePrinterUtils.java(2 hunks)app/src/org/commcare/views/dialogs/AlertDialogFragment.java(3 hunks)app/src/org/commcare/views/dialogs/CommCareAlertDialog.java(3 hunks)app/src/org/commcare/views/dialogs/CustomViewAlertDialog.java(1 hunks)app/src/org/commcare/views/dialogs/PaneledChoiceDialog.java(0 hunks)app/src/org/commcare/views/dialogs/StandardAlertDialog.java(0 hunks)app/src/org/commcare/views/widgets/ImageWidget.java(1 hunks)app/unit-tests/src/org/commcare/utils/RobolectricUtil.java(2 hunks)
💤 Files with no reviewable changes (3)
- app/src/org/commcare/views/dialogs/StandardAlertDialog.java
- app/src/org/commcare/views/dialogs/PaneledChoiceDialog.java
- app/src/org/commcare/fragments/ContainerFragment.java
🧰 Additional context used
🧬 Code Graph Analysis (6)
app/src/org/commcare/activities/LoginActivity.java (1)
app/src/org/commcare/views/dialogs/DialogCreationHelpers.java (1)
DialogCreationHelpers(22-77)
app/src/org/commcare/activities/EntitySelectActivity.java (1)
app/src/org/commcare/fragments/ContainerViewModel.java (1)
ContainerViewModel(5-16)
app/src/org/commcare/views/dialogs/AlertDialogFragment.java (1)
app/src/org/commcare/fragments/ContainerViewModel.java (1)
ContainerViewModel(5-16)
app/src/org/commcare/fragments/TaskConnectorViewModel.java (2)
app/src/org/commcare/activities/CommCareActivity.java (1)
CommCareActivity(83-979)app/src/org/commcare/tasks/templates/CommCareTask.java (1)
CommCareTask(13-223)
app/src/org/commcare/activities/CommCareSetupActivity.java (1)
app/src/org/commcare/fragments/ContainerViewModel.java (1)
ContainerViewModel(5-16)
app/src/org/commcare/fragments/BreadcrumbBarViewModel.java (2)
app/src/org/commcare/views/EntityViewTile.java (1)
EntityViewTile(46-491)app/src/org/commcare/utils/AndroidUtil.java (1)
AndroidUtil(19-65)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (41)
app/src/org/commcare/fragments/ContainerViewModel.java (1)
1-16: Well-designed ViewModel implementation for data retentionThis new generic
ContainerViewModelclass is a clean implementation that follows the Android ViewModel architecture pattern. It's a simple yet effective container for storing and retrieving data that needs to survive configuration changes.app/src/org/commcare/preferences/AdvancedActionsPreferences.java (1)
293-293: Updated dialog invocation with explicit context parameterThe change to pass the activity context explicitly to
showNonPersistentDialog()aligns with best practices for dialog management and is part of the wider refactoring to improve lifecycle handling.app/src/org/commcare/activities/FormEntryActivityUIController.java (1)
590-590: Updated dialog invocation with explicit context parameterThe change to pass the activity context explicitly to
showNonPersistentDialog()improves dialog lifecycle management and prevents potential memory leaks.app/src/org/commcare/preferences/MainConfigurablePreferences.java (1)
140-140: Updated dialog invocation with explicit context parameterThe change to pass the activity context explicitly via
getActivity()toshowNonPersistentDialog()follows the same pattern as other dialog invocations throughout the codebase, ensuring consistent dialog lifecycle management.app/src/org/commcare/utils/AndroidShortcuts.java (1)
58-58: Appropriate context passing in dialog display.The update to pass the current activity instance (
this) toshowNonPersistentDialog()aligns with the PR's goal of removing reliance on No UI Fragments with setRetainInstance. This explicit context passing ensures proper dialog lifecycle management.app/src/org/commcare/activities/DrawActivity.java (1)
323-323: Consistent dialog context management.Passing the activity context explicitly to
showNonPersistentDialog()ensures proper dialog lifecycle management and is consistent with changes throughout the codebase. This change supports the removal of Fragment-based state retention.app/src/org/commcare/utils/TemplatePrinterUtils.java (2)
154-154: Explicit context passing for dialog display.The addition of the activity parameter to
showNonPersistentDialog()ensures proper dialog management without relying on retained instance state in fragments.
168-168: Consistent context management pattern.This change follows the same pattern as other dialog calls in the codebase, ensuring proper lifecycle management by explicitly passing the activity context to the dialog.
app/src/org/commcare/utils/Permissions.java (1)
53-53: Improved dialog lifecycle management.Explicitly passing the activity context to
showNonPersistentDialog()aligns with the architectural change to remove reliance on retained fragment instances for state preservation. This ensures dialogs are properly associated with their parent activity's lifecycle.app/src/org/commcare/utils/GeoUtils.java (2)
119-119: Consistent API update for dialog management.The change updates the dialog method to explicitly receive the activity context, which aligns with the broader refactoring effort to improve dialog lifecycle management across the codebase.
119-119: Properly associates dialog with activity lifecycleThe change explicitly passes the activity context to the
showNonPersistentDialogmethod, which ensures that the dialog is correctly associated with the activity's lifecycle and prevents potential memory leaks.app/src/org/commcare/activities/GeoPointActivity.java (2)
238-238: Properly passing activity context to dialog method.This change ensures the dialog is properly associated with its parent activity lifecycle, which helps prevent potential memory leaks and improves state handling during configuration changes.
238-238: Improved dialog lifecycle managementExplicitly passing the activity context to
showNonPersistentDialogensures the dialog is properly tied to the activity lifecycle, which follows the broader refactor to remove retained fragments in favor of ViewModel-based state management.app/src/org/commcare/activities/CallOutActivity.java (4)
117-117: Dialog method updated to use explicit activity context.This change follows the pattern of passing the activity instance to dialog methods, maintaining consistency with the codebase-wide refactoring from fragment-based state retention to ViewModel architecture.
135-135: Updated dialog method to receive activity context.Similar to other dialog invocations, this change properly supplies the activity context to the dialog method, ensuring proper lifecycle attachment.
117-117: Proper dialog context associationExplicitly passing the activity to
showNonPersistentDialogensures the dialog is properly tied to the activity lifecycle, preventing potential memory leaks and improper view attachment.
135-135: Proper dialog context associationSimilarly to the change above, passing the activity context to the permission request dialog ensures proper lifecycle management, which aligns with the effort to eliminate retained fragments for state storage.
app/src/org/commcare/activities/LoginActivity.java (2)
433-433: Updated dialog display method to explicitly pass activity context.The change properly supplies the activity instance to the
showNonPersistentDialogmethod, which aligns with the broader refactoring to improve dialog lifecycle management throughout the application.
433-433: Improved dialog lifecycle handlingPassing the activity context to
showNonPersistentDialogis consistent with the codebase-wide pattern of explicitly providing the context for better lifecycle management. This ensures the About dialog is properly associated with the activity and can be managed correctly during configuration changes.app/src/org/commcare/views/widgets/ImageWidget.java (1)
110-110: Dialog call updated to pass context parameterThe call to
showNonPersistentDialog()has been updated to explicitly pass the context parameter, aligning with the broader refactoring throughout the codebase that improves dialog lifecycle management by using explicit context references.app/src/org/commcare/activities/HomeScreenBaseActivity.java (1)
504-504: Dialog call updated to pass activity contextThe dialog invocation has been modified to explicitly pass the activity instance (
this) to theshowNonPersistentDialogmethod, which is consistent with the broader pattern of improving dialog lifecycle management throughout the codebase.app/src/org/commcare/views/dialogs/CustomViewAlertDialog.java (2)
16-16: Simplified dialog view configurationThe constructor implementation has been refactored to delegate view configuration to the superclass method
setView(view)instead of managing an internalAlertDialog.Builderinstance, which simplifies the code and improves maintainability.
20-21: Improved button configuration patternThe
setPositiveButtonimplementation now delegates to the inherited methodssetPositiveButtonText()andsetPositiveButtonListener()instead of directly managing an internal builder. This change is part of the overall dialog refactoring to improve lifecycle management and code organization.app/unit-tests/src/org/commcare/utils/RobolectricUtil.java (3)
3-7: Updated imports for ViewModel-based architectureImport statements have been updated to support the new ViewModel-based architecture, replacing the fragment-based imports with the appropriate androidx lifecycle components.
24-26: Migrated from fragment to ViewModel for state managementThe implementation has been updated to use
ViewModelProviderto obtainTaskConnectorViewModelinstead of accessing state through fragments. This aligns with the architectural shift from fragment-based state holders to ViewModel-based lifecycle-aware components throughout the codebase.
33-33: Fixed main looper handlingThe
ShadowLooper.idleMainLooper()call has been moved outside the conditional block to ensure it's always called after attempting to get the task result, regardless of whether a task was found.app/src/org/commcare/activities/CommCareSetupActivity.java (2)
239-242: Good migration to an Activity‑scoped ViewModel
persistCommCareAppState()correctly restores theCommCareAppinstance from
ContainerViewModel, ensuring it survives rotations without the old retained
fragment.
464-465: Potential leak: heavy object in ViewModel
CommCareAppis added to theContainerViewModel. IfCommCareAppholds a
reference to theActivity(directly or indirectly), retaining it may leak the
old activity after a rotation. VerifyCommCareAppis free of such references
(or store a lightweight ID instead).app/src/org/commcare/activities/CommCareActivity.java (13)
30-35: Architecture modernization: Updated imports for ViewModel-based approach.The import statements have been updated to include the ViewModel-related libraries from AndroidX, replacing fragment-based imports with ViewModel-based counterparts. This shift aligns with Android's recommended architecture components.
39-41: Switched to ViewModel-based state management.The imports have been updated to use ViewModels instead of Fragment-based state holders:
BreadcrumbBarViewModelinstead ofBreadcrumbBarFragmentContainerViewModelinstead ofContainerFragmentTaskConnectorViewModelinstead ofTaskConnectorFragmentThis is a significant architectural improvement that better aligns with Android's recommended patterns for state management across lifecycle events.
93-93: Updated field declaration to use ViewModel.Changed the state holder from a Fragment-based implementation to a ViewModel-based implementation, which is better for handling lifecycle events and configuration changes.
131-131: Renamed fields to use ViewModels.Fragment-based fields have been replaced with their ViewModel counterparts:
containerFragment→containerViewModelbreadcrumbBarFragment→breadcrumbBarViewModelThis ensures UI state is preserved across configuration changes like screen rotations.
Also applies to: 135-135
146-148: Implemented ViewModel instantiation and attachment.The stateHolder is now retrieved using ViewModelProvider and explicitly attached to the activity. This follows the proper lifecycle-aware pattern for ViewModels, ensuring the activity can properly connect to background tasks.
172-174: Refactored breadcrumb bar initialization to use ViewModel.Instead of creating and managing a fragment transaction, the code now uses a ViewModel to handle the breadcrumb bar state. The
attachBreadcrumbBarmethod centralizes the attachment logic in the ViewModel itself.
184-190: Improved UI state persistence with ViewModels.The
persistManagedUiStatemethod now uses ViewModelProvider to create or obtain the ContainerViewModel, ensuring UI state is properly preserved across configuration changes. This provides a more reliable mechanism than fragment-based state retention.
326-326: Enhanced state saving in onPause.UI state is now saved to the ContainerViewModel's data store during onPause, ensuring that state is preserved in a lifecycle-aware manner, which is more reliable than the previous fragment-based approach.
340-340: Added proper cleanup in onDestroy.The stateHolder is now explicitly detached in onDestroy, preventing potential memory leaks by properly breaking the reference between the activity and the ViewModel when the activity is destroyed.
824-827: Updated back press handling to use ViewModel.The onBackPressed method now checks with the breadcrumbBarViewModel instead of a fragment to determine if a tile is expanded. This centralizes the breadcrumb state management in the ViewModel.
904-904: Refactored action bar refresh to use ViewModel.The refreshActionBar method now calls attachBreadcrumbBar on the breadcrumbBarViewModel instead of refreshing a fragment, centralizing the UI update logic in the ViewModel.
949-949: Modified breadcrumb bar display behavior.Changed shouldShowBreadcrumbBar to always return false by default, effectively disabling the breadcrumb bar display unless specifically overridden by subclasses. This suggests the breadcrumb bar functionality is now opt-in rather than default.
936-938: Added setter for dependency injection.Added a setStateHolder method to allow for dependency injection of the TaskConnectorViewModel, improving testability and flexibility of the activity by allowing the state holder to be mocked or injected.
| private TabbedDetailView mInternalDetailView = null; | ||
| private View tile; | ||
|
|
||
| private final static String INLINE_TILE_COLLAPSED = "collapsed"; | ||
| private final static String INLINE_TILE_EXPANDED = "expanded"; | ||
|
|
There was a problem hiding this comment.
ViewModel stores View instances – risk of Activity leaks
mInternalDetailView and tile hold strong references to UI elements created with an Activity context. When the activity is destroyed these references keep it (and its entire window hierarchy) alive, defeating the purpose of migrating away from retained fragments.
Recommendation:
- Move all view creation/expansion logic back to the activity (or a dedicated
Fragment). - Keep the ViewModel pure: only retain the data needed to rebuild the breadcrumb bar, not the views themselves.
- Provide observable state (e.g.
LiveData) for the activity to render.
There was a problem hiding this comment.
would these been causing leaks even in previous code with fragment ? Trying to verify if it was an existing issue vs a regression with view model changes ?
I am wary of changing this code too much as it's a bit legacy and I don't have a good understanding of it and would like to make minimal changes here if possible while avoiding a memory leak.
There was a problem hiding this comment.
@shubham1g5 Yes, this is potentially a regression rather than an existing issue.
In the previous Fragment implementation with setRetainInstance(true), the Android framework had special handling to manage view references during the fragment lifecycle. Even though your fragment was retained across configuration changes, the framework would properly detach views when needed.
ViewModels, however, don't have this same automatic cleanup mechanism. When a ViewModel holds direct references to Views like mInternalDetailView and tile, those views keep a reference to their Context (your Activity), preventing garbage collection.
For a minimal fix approach:
public class BreadcrumbBarViewModel extends ViewModel {
// Instead of View references, store only the data needed
private TreeReference tileReference;
private String inlineDetailId;
// Other minimal state needed to reconstruct the views
private boolean isExpanded = false;
// Remove the view field
// private TabbedDetailView mInternalDetailView = null;
// private View tile;
// Getters/setters for the state
public void setTileReference(TreeReference ref) {
this.tileReference = ref;
}
public TreeReference getTileReference() {
return tileReference;
}
// Similar methods for other state
}Then in your Activity, maintain the view instances but use the ViewModel only for state. This keeps changes minimal while avoiding the leak.
✏️ Learnings added
Learnt from: shubham1g5
PR: dimagi/commcare-android#3042
File: app/src/org/commcare/fragments/BreadcrumbBarViewModel.java:50-55
Timestamp: 2025-04-21T15:02:17.464Z
Learning: ViewModels should not store View or Activity references as this can cause memory leaks. Unlike Fragments with setRetainInstance(true), ViewModels don't have automatic view detachment mechanisms. When migrating from Fragments to ViewModels, view references should be replaced with data-only state in the ViewModel.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Would using weakreference fix it ?
There was a problem hiding this comment.
@shubham1g5 While using WeakReferences (WeakReference<View>) could technically reduce the likelihood of memory leaks, it's not the recommended approach for this issue for several reasons:
- Code complexity - You'd need to check for null every time you access the view:
if (tileRef.get() != null) { ... } - Reliability concerns - WeakReferences might still leak if there are other reference paths to the views
- Debugging difficulties - WeakReferences can make tracking object lifecycles more complicated
- Architectural mismatch - It patches the symptom rather than addressing the core design issue
The cleaner solution remains storing only data in the ViewModel and moving view creation/manipulation to the Activity or Fragment. This follows the proper architectural pattern for ViewModels.
If you're looking for a minimal change approach, WeakReferences could work as a temporary solution, but you'd need to add null checks throughout the code wherever these views are accessed.
🧠 Learnings used
Learnt from: shubham1g5
PR: dimagi/commcare-android#3042
File: app/src/org/commcare/fragments/BreadcrumbBarViewModel.java:50-55
Timestamp: 2025-04-21T15:02:17.464Z
Learning: ViewModels should not store View or Activity references as this can cause memory leaks. Unlike Fragments with setRetainInstance(true), ViewModels don't have automatic view detachment mechanisms. When migrating from Fragments to ViewModels, view references should be replaced with data-only state in the ViewModel.
|
@damagatchi retest this please |
2 similar comments
|
@damagatchi retest this please |
|
@damagatchi retest this please |
shubham1g5
force-pushed
the
getRidOfSetRetainInstance
branch
from
36be1f4 to
5c9a52c
Compare
… layout is set properly before we call it
… as they end up re-loading any way on config changes
… data state without retaining view
|
@damagatchi retest this please |
4 similar comments
|
@damagatchi retest this please |
|
@damagatchi retest this please |
|
@damagatchi retest this please |
|
@damagatchi retest this please |
shubham1g5
force-pushed
the
getRidOfSetRetainInstance
branch
2 times, most recently
from
52d0856 to
7aa103a
Compare
shubham1g5
force-pushed
the
getRidOfSetRetainInstance
branch
from
7aa103a to
1dfb8b8
Compare
shubham1g5
requested review from
OrangeAndGreen,
avazirna,
ctsims and
pm-dimagi
Product Description
https://dimagi.atlassian.net/browse/CCCT-955
We were traditinally using Fragments with
setRetainInstance(true)to store state and handle Android configuration changes. Although setRetainInstance has been deprecated and the new recommended approach is to use View Models for surving state on congiguration changes. This PR removes our ccurrent non UI fragments that we were using to handle state and replace them with view models.On the connect side of things, we were noticing that it's already not working as expected with dialog fragments not getting re-attached to re-created activity context after configuration change. We could not pinpoint it to any change made by us in connect and therefore we decided to shift to a more reliable way to handle rotation.
This PR should not cause any UI changes.
Safety Assurance
Safety story
There are various parts that this PR affects that are risk to break because of these changes -
Labels and Review
QA Note: Do rotation testing on all CommCare screens, progress dialogs and other popups and make sure we don't reload the UI causing a small lag after rotation and that the dialog buttons work correctly after rotation