Always call #onNewIntent(), even without @AfterExtras method(s)

If there is any method with @AfterExtras passing new Intent to activity causes calling onNewIntent(), from there setIntent() and finally injectExtras_() where all new extras are set. And it's great.

On the other hand if there is no any @AfterExtras method generated subclass has no onNewIntent() method, and in result extra is not updated.

This is unfortunate if extra does not cause any instant action, but is used later - for example in my case there is only one extra that is passed by to next activity, without any operations on it. So currently I had to add blank method with @AfterExtras annotation just to make sure that onNewIntent() is generated.

@dodgex

The @Extras are injected every time when setIntent() is called. I am not sure we should change that to onNewIntent(). @dodgex @yDelouis ?

But setIntent() is not called if there is no @AfterExtras method. On the other hand, if there is any, it is called. So depending on having any @AfterExtras methods extras are or are not updated - it's inconsistency for me.

The extras are injected in two places In setIntent() and in onCreate(). So they will be injected even if there is no @AfterExtras method, in onCreate(). If the Activity is not created, just a new intent reaches the Activity, then we only inject the extras if the setIntent() method is called. Which seem to be correct to me, because if that is not called, the new Intent does not really belong to the Activity. The @AfterExtras annotation clearly states the the user wants to work with the injected extras, so we call setIntent() there.

You are saying that you are just using @AfterExtras as dummy now. My question is: which lifecycle method you use to actually work with the newly injected extras?

You are saying that you are just using @AfterExtras as dummy now. My question is: which lifecycle method you use to actually work with the newly injected extras?

In my (very uncommon I think ;-) ) case I'm only passing this particular extra (if exists) to the next Activity, without interacting with it. The case is this Intent comes from notification click and next Activity must be informed, what was in this notification.

Please consider following two example Activities:

@EActivity
public class AbcActivity extends Activity {

@Extra
String firstExtra;

@Extra
String secondExtra;

@AfterExtras
void doSthWithSecondExtra() {
    if (secondExtra != null) {
        Log.i("Second extra is: " + secondExtra)
    }
}

@Click(R.id.my_button)
void onMyButtonClick() {
    Log.i("First extra is: " + firstExtra)
}
}

@EActivity
public class XyzActivity extends Activity {

@Extra
String firstExtra;

@Click(R.id.my_button)
void onMyButtonClick() {
    Log.i("First extra is: " + firstExtra)
}
}

And now passing new Intent with single top flag etc. (so without re-creating the Activity) and then clicking the button will result in different values logged.

I agree this is an uncommon case. In AA, we can always support the most common case, because we have to decide things in compile-time, which makes adding options really hard.
Can you just use the workaround? Or something wrong with that?

Yea, it works with workaround (empty @AfterExtras method). But maybe at least add some info in wiki? I can do this if you want.

And about making decisions at compile-time - from my point of view onNewIntent() should be generated always if there is any @AfterExtras method (as it is currently) OR @Extra field. Looks like adding holder.getOnNewIntent(); into ExtraHandler should do the thing and I can try to do this as well, if you would like me to. But I understand it may be not so easy as it looks like at first glance.

@WonderCsabo

@WonderCsabo i think this makes sense and i see no reason not to support this. i even have a similar case where i use only one of my four extras inside @AfterExtras and the others in either click or EventBus events. here i have the @AfterExtras but that method could become deprecated in near future.

OK, let's override onNewIntent() and call setIntent() always. This is maybe a breaking change, because some people may expect the original Intent with getIntent() if only uses @Extra.

@dodgex

@dodgex should we move extra injecton to onNewIntent, or always call setIntent?

@WonderCsabo

@WonderCsabo that is a difficult question. is onNewIntent called when the activity is created?

i think i would keep it in setIntent.

but the best option would be somehow to allow the developer to decide where it should be injected and if setIntent should be called from onNewIntent

this might be an interesting read

What about setting the extras in both onNewIntent() and onCreate(). And only call setIntent() with @AfterExtras (or not call that at all)?

i think we should do it that way.

inject in onCreate() and onNewIntent() but not call setIntent(). the user has to override onNewIntent() if he needs the new Intent in getIntent() instead of the original one.

but that might mean a silently breaking change. as one may rely on the setIntent() call when using getIntent() somewhere.

Yeah, that would be a breaking change, we will mark it in the release notes. I agree, this is a best solution, and gives the user maximum flexibility.

But please remember that by default AA calls setIntent() in onNewIntent() right now in most of the cases - if there is any @Extra field.

That is not true, only if there is a method annotated with @AfterExtras.

Sorry, you are right. But still - sometimes AA calls setIntent(), sometimes it does not.

After thinking this trough i came to this conclusion:

We should inject extras as we do currently, meaning in onCreate() and in setIntent(). The injected extras are becoming the state of the Activity. If we would inject extras in onNewIntent(), the state would contain any last Intent extras which may be not interesting to the object at all, and also makes the real last interesting extras inaccessible in the injected fields. Also, the current way stores the same values in the fields as getIntent().getExtras() do, which is consistent.

I think we should not override onNewIntent(), ever, even with @AfterExtras. We could add a flag to @AfterExtras but what happens when there are multiple methods annotated with that, or after extras methods are coming from the super classes? So the user should have the liberty to decide whether an Intent should be set to the Activity or not.

@dodgex

@dodgex @yDelouis what do you think about my last proposal?

to clarify how i understand your proposal:

by default never override onNewIntent() to call setIntent()
inject in onCreate() and setIntent()
(maybe) add an option to @AfterExtras to yet again generate a onNewIntent() that calls setIntent()

this sounds ok, but i would not add the option to @AfterExtras but to @EActivity that bypasses the issue of having more than one @AfterExtras and it is imo a better place for an activity wide option. also this allows to have the overriden onNewIntent() even when not using @AfterExtras.

also i think if the user has a case where the parent and the child activity require diffrent behaviour related to setIntent() the user has a bigger problem than we can take care of.

I do not think @EActivity is a good place for this, the purpose of it is different.

If have a different idea: add processing option which decides whether do override onNewIntent() or not. This could be true by default, which is okay for most of the projects.

However this is also not really cool. This is a tough problem, and it seems there is no best-case scenario. :(

I'd prefer the @EActivity over a processing option as this allows to have a per activity setting.

i belive having the option on @EActivity is the best solution. one can configure the behaviour on a per activity base and can have onNewIntent() independently of @AfterExtras and @Extra - this way he could even benefit from it when not using any of the extras related annotations.

I think we should not add more properties to @EActivity, but we may could introduce a new annotation for this.

@OverrideOnNewIntent? :D

a new annotation is ok too. at least it should be possible to decide that on a per activity base.

I would have injected extras only onCreate and setIntent. If the developer wants, he still can override onNewIntent to call setIntent.
To help him doing so, we could add a new annotation on the Activity class (@SetIntentOnNewIntent) but I don't think it helps a lot.

@dodgex

OK, a decision has been made. So the only action for use is remove the onNewIntent override. @dodgex can you do this as the original author of that code?

I just broke my own app/code with this PR :/ :D

@m-radzikowski

Implemented, thanks for the cooperation everybody.

@m-radzikowski please override onNewIntent() manually when you need it.

Navigation Menu

Repository navigation

Always call #onNewIntent(), even without @AfterExtras method(s) #1578

4 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Always call #onNewIntent(), even without @AfterExtras method(s) #1578

Description

Activity

WonderCsabo commented on Oct 7, 2015

m-radzikowski commented on Oct 7, 2015

WonderCsabo commented on Oct 7, 2015

m-radzikowski commented on Oct 8, 2015

WonderCsabo commented on Oct 8, 2015

m-radzikowski commented on Oct 8, 2015

dodgex commented on Oct 8, 2015

WonderCsabo commented on Oct 8, 2015

WonderCsabo commented on Oct 11, 2015

dodgex commented on Oct 12, 2015

dodgex commented on Oct 12, 2015

WonderCsabo commented on Oct 12, 2015

dodgex commented on Oct 12, 2015

WonderCsabo commented on Oct 12, 2015

4 remaining items

m-radzikowski commented on Oct 20, 2015

WonderCsabo commented on Oct 20, 2015

m-radzikowski commented on Oct 20, 2015

WonderCsabo commented on Oct 21, 2015

WonderCsabo commented on Oct 28, 2015

dodgex commented on Oct 28, 2015

WonderCsabo commented on Oct 28, 2015

dodgex commented on Oct 28, 2015

WonderCsabo commented on Oct 28, 2015

dodgex commented on Oct 28, 2015

yDelouis commented on Oct 29, 2015

WonderCsabo commented on Oct 29, 2015

dodgex commented on Oct 29, 2015

WonderCsabo commented on Oct 29, 2015

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions