Skip to content
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 might lose the presenter id #83

Closed
inv3rse opened this issue Apr 13, 2016 · 14 comments
Closed

Fragment might lose the presenter id #83

inv3rse opened this issue Apr 13, 2016 · 14 comments

Comments

@inv3rse
Copy link

inv3rse commented Apr 13, 2016

If a fragments (PresenterLifecycleDelegates) onSaveInstanceState method gets called before the PresenterFactory is set, it will not save its presenter id in the result bundle.

I have a NucleusSupportFragment for which I set the PresenterFactory in onActivityCreated.
When I detach that fragment (without destroying the presenter) and rotate the screen once, the presenter id will be saved in the bundle . After another rotate however the presenter id will not be saved anymore because getPresenter() will not restore the Presenter without a set Factory.

@konmik
Copy link
Owner

konmik commented Apr 13, 2016

Do you mean, that onSaveInstanceState can be called before onResume?

Do you have an example?

I know about a fragment bug - they sometimes do not save state while being in back stack and you need to flip screen twice. I don't know if it was fixed yet.

@inv3rse
Copy link
Author

inv3rse commented Apr 13, 2016

I do not attach the fragment back to the screen after the orientation change. Therefore onResume is not getting called at all.

I might be doing something stupid with the FragmentManager:
https://gist.github.com/inv3rse/51d89728cc5f8c3c06687c307096232a

Basically I add two fragments to a container and detach the one I do not want to show.
On the detached fragment only onCreate and onSaveInstanceState are getting called.

Edit: As a workaround i am currently setting a factory which returns null in onCreate if i already have a presenter set.

@konmik
Copy link
Owner

konmik commented Apr 13, 2016

Presenter normally becomes created during the first onResume call. But if it doesn't it is created during onSaveInstanceState as well.

https://github.com/konmik/nucleus/blob/master/nucleus/src/main/java/nucleus/view/PresenterLifecycleDelegate.java#L92

https://github.com/konmik/nucleus/blob/master/nucleus/src/main/java/nucleus/view/PresenterLifecycleDelegate.java#L69

I would use a debugger and find out what is the real reason for this.

@inv3rse
Copy link
Author

inv3rse commented Apr 13, 2016

getPresenter() will not work without a factory, which is not set in my case.

@konmik
Copy link
Owner

konmik commented Apr 13, 2016

You need to supply a presenter factory in another way then.

@inv3rse
Copy link
Author

inv3rse commented Apr 13, 2016

I have worked around it currently, just thougth that there is no reason not to keep the presenter id in the bundle.

@konmik
Copy link
Owner

konmik commented Apr 13, 2016

How could we connect views with presenters without saving presenter's id?

@inv3rse
Copy link
Author

inv3rse commented Apr 13, 2016

I ment that the presenter id should not get lost. That the information is there and should not be discarded, regardless whether there is a factory set or not.

@konmik
Copy link
Owner

konmik commented Apr 13, 2016

Why did you override the default presenter factory at all? I think that the default way with just one annotation is awesome. Dagger can be used with this way as well.

@inv3rse
Copy link
Author

inv3rse commented Apr 13, 2016

Can you link me more information to that? All i found was that gist while checking if there already was an issue like mine https://gist.github.com/konmik/6ac725fa7134402539c4.

I am setting a custom factory because dagger (2) is constructing my presenter.

@konmik
Copy link
Owner

konmik commented Apr 13, 2016

#32

@inv3rse
Copy link
Author

inv3rse commented Apr 13, 2016

Well that looks somewhat similar to what I am doing.

in onActivityCreated because i need the context:
setPresenterFactory(new HomePresenter.Factory(App.get(getContext())));

and the factory:

public static class Factory implements PresenterFactory<HomePresenter> {
        private App mApp;

        public Factory(App app) {
            mApp = app;
        }

        @Override
        public HomePresenter createPresenter() {
            return DaggerHomeComponent.builder().appComponent(mApp.component()).build().getPresenter();
        }
    }

@konmik
Copy link
Owner

konmik commented Apr 13, 2016

You have activity right in fragment's onCreate - just call getActivity()

There is also no need in activity context for presenters at all (if you don't want to have leaks). If you need Application context you can access it using singleton pattern i.e. using a static variable and a static getter function.

@inv3rse
Copy link
Author

inv3rse commented Apr 14, 2016

Yeah, I was only using the context to get the Application context. Did not think about using a static getter, will do that, thanks.

I guess this ticket is unnecessary when sticking to the rules .

@inv3rse inv3rse closed this as completed Apr 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants