Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.3k
Added optional single host MvxStartActivity#4846
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:develop
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Cheesebaron commentedMar 29, 2024
Using MvxStartActivity is already optional, you can simply not have it in your App and makre you "single host" Activity as the MainLauncher, so I am not sure if this PR is necessary. Can you elaborate exactly what it solves? |
entdark commentedMar 29, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Maybe my solution could be designed a bit better and/or named better - I am fine to tune it before the PR. |
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.
Pull Request Overview
This PR introduces an optional single host mode forMvxStartActivity to prevent double Activity navigation on startup. Instead of navigating fromMvxStartActivity to a separate Activity for the first ViewModel, it allowsMvxStartActivity to serve as both the launcher and fragment host.
- Added
SingleHostActivityproperty to control behavior (defaults to false for backward compatibility) - Changed base class from
MvxActivitytoMvxActivity<MvxStartViewModel>with proper ViewModel typing - Modified layout inflation to use binding-aware inflation and conditional window features
| } | ||
| } | ||
| publicclassMvxStartViewModel:MvxNullViewModel{} |
CopilotAIJul 20, 2025
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.
The MvxStartViewModel class is placed at the end of the file outside the namespace. This should be moved inside the namespace or to a separate file for proper organization and to avoid namespace pollution.
| publicclassMvxStartViewModel:MvxNullViewModel{} | |
| publicclassMvxStartViewModel:MvxNullViewModel{} | |
| } |
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.
Please move it to a separate file
| // Set our view from the "splash" layout resource | ||
| // Be careful to use non-binding inflation | ||
| varcontent=LayoutInflater.Inflate(_resourceId,null); | ||
| varcontent=this.BindingInflate(_resourceId,null); |
CopilotAIJul 20, 2025
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.
The change from non-binding to binding inflation may have unintended consequences when SingleHostActivity is false. Consider making this conditional based on SingleHostActivity property to maintain existing behavior:var content = SingleHostActivity ? this.BindingInflate(_resourceId, null) : LayoutInflater.Inflate(_resourceId, null);
| varcontent=this.BindingInflate(_resourceId,null); | |
| varcontent=SingleHostActivity | |
| ?this.BindingInflate(_resourceId,null) | |
| :LayoutInflater.Inflate(_resourceId,null); |
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.
This could be a breaking change.
Cheesebaron left a comment
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.
Sorry to leave this one hanging, can you pelase address the linting issues and review comments
| } | ||
| } | ||
| publicclassMvxStartViewModel:MvxNullViewModel{} |
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.
Please move it to a separate file
| // Set our view from the "splash" layout resource | ||
| // Be careful to use non-binding inflation | ||
| varcontent=LayoutInflater.Inflate(_resourceId,null); | ||
| varcontent=this.BindingInflate(_resourceId,null); |
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.
This could be a breaking change.
entdark commentedSep 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This PR still bothers me a bit. Thinking of it I found another solution: mark the main launcher MvxAndroidApplication<TMvxAndroidSetup,TApplication,TActivity>:MvxAndroidApplication<TMvxAndroidSetup,TApplication>whereTActivity:Activity{publicoverridevoidOnCreate(){base.OnCreate();RegisterActivityLifecycleCallbacks(newActivityLifecycleCallbacks(this));}classActivityLifecycleCallbacks(global::Android.App.Applicationapplication): Java.Lang.Object,IActivityLifecycleCallbacks{bool_isResumed;Bundle_bundle;Setup Setup=>MvxAndroidSetupSingleton.EnsureSingletonAvailable(application).PlatformSetup<Setup>();IsMainLauncherActivity(Activityactivity)=>activityisTActivity;publicvoidOnActivityPaused(Activityactivity){if(!IsMainLauncherActivity(activity))return;_isResumed=false;CancelMonitor();}publicvoidOnActivityResumed(Activityactivity){if(!IsMainLauncherActivity(activity))return;_isResumed=true;Monitor();}publicvoidOnActivityCreated(Activityactivity,BundlesavedInstanceState){if(!IsMainLauncherActivity(activity))return;_bundle=savedInstanceState;Monitor();}publicvoidOnActivityDestroyed(Activityactivity){if(!IsMainLauncherActivity(activity))return;CancelMonitor();}publicvoidOnActivitySaveInstanceState(Activityactivity,BundleoutState){}publicvoidOnActivityStarted(Activityactivity){}publicvoidOnActivityStopped(Activityactivity){}voidInitializationComplete(){if(!_isResumed)return;if(Mvx.IoCProvider.TryResolve(outIMvxAppStartstartup)){if(!startup.IsStarted){Task.Run(async()=>awaitstartup.StartAsync(_bundle));}}}voidMonitor(){Setup.StateChanged-=SetupStateChanged;if(Setup.State==MvxSetupState.Initialized){InitializationComplete();}else{Setup.StateChanged+=SetupStateChanged;}}voidCancelMonitor(){Setup.StateChanged-=SetupStateChanged;}voidSetupStateChanged(objectsender,MvxSetupStateEventArgsev){if(ev.SetupState==MvxSetupState.Initialized){InitializationComplete();}}}} So basically we move responsibility to start the initial navigation to |
✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)
Prevent double
Acitivitynavigation on startup (MvxStartActivitythen first VMActivity) and navigate directly to the main launcherActivitythat isMvxStartActivityand the host for fragments.Currently
MvxStartActivitystarts the app and navigates to the first VM that view is expected to be anActivity.🆕 What is the new behavior (if this is a feature change)?
Made
MvxStartActivityto be used simultaneously as the main launcher and the host for fragments.SingleHostActivityproperty controls the old and new behaviour.In the new behaviour it never
Finish()theMvxStartActivitywhen it already launched with first VM.💥 Does this PR introduce a breaking change?
Should not. I made things optional and defaults to original behaviour. (Maybe should default to the new one and break?)
🐛 Recommendations for testing
Test any first VM that navigates to a
Fragmentrather than to anActivity.📝 Links to relevant issues/docs
🤔 Checklist before submitting