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

Target Android 12 (set targetSdkVersion to 31), by 2022-11 deadline #5101

Closed
gnprice opened this issue Nov 4, 2021 · 2 comments · Fixed by #5174
Closed

Target Android 12 (set targetSdkVersion to 31), by 2022-11 deadline #5101

gnprice opened this issue Nov 4, 2021 · 2 comments · Fixed by #5174

Comments

@gnprice
Copy link
Member

gnprice commented Nov 4, 2021

This is the successor to #4284 (and #3665 and #3563 and #3075). We should update our targetSdkVersion to 31, meaning Android 12.

Based on the last few years' experience, Google Play will probably set a deadline for this of late 2022. It's been the first business day of November in each of the last few years.

The important steps for this upgrade are:

  1. Read about the potentially-breaking changes, and identify those that might affect us.
  2. Make a WIP change to targetSdkVersion.
  3. Test the WIP change thoroughly, especially in the areas highlighted in step 1. Fix things as needed, and repeat.

We aren't aware of any blockers to this, so we can try doing it any time. It'd be nice to do it well before the deadline -- or at least to go far enough to learn about any unknown blockers.

gnprice added a commit to gnprice/zulip-mobile that referenced this issue Nov 11, 2021
This puts it in the context of the notification where it's used.

And in fact that makes it easier for the linter to see what we're
doing, too, and it starts giving a useful new warning!  Suppress it,
with an explanatory comment.  (We'll also note this on zulip#5101.)
@gnprice
Copy link
Member Author

gnprice commented Nov 11, 2021

Well, here's one blocking issue:
https://developer.android.com/about/versions/12/behavior-changes-12#notification-trampolines
We'll need to replace our NotificationIntentService, which we use when you tap a notification to open it, with directly opening an Activity. (We learned about this from a lint warning in Android Studio: #5121 (comment))

  • Ideally this would be our MainActivity. We'll need to play with things somewhat to try to get the correct behavior in all three cases of the app being in the foreground, in the background, and not running. (Currently NotificationIntentService has NotifyReact.kt do this work.)
    • The main advice from Android upstream won't work for us verbatim because it assumes the canonical Android model where basically every screen in your app is a distinct activity. That structure isn't even the unanimous advice of Android upstream these days -- some of their voices say you should have one or a few activities and use Fragments for navigation instead. But in any case it's not our structure of things using RN, and we have just one activity where the app's whole UI (apart from the notifications) lives. Hence the need to play with things to find a solution.
  • If we can't make that work, it should certainly be possible to make a trampoline activity, analogous to our ShareToZulipActivity. That sure won't be in the spirit of what they're going for here, though. It may also cause visible jank in the UI, like an extra transition.

gnprice added a commit to gnprice/zulip-mobile that referenced this issue Nov 16, 2021
This puts it in the context of the notification where it's used.

And in fact that makes it easier for the linter to see what we're
doing, too, and it starts giving a useful new warning!  Suppress it,
with an explanatory comment.  (We'll also note this on zulip#5101.)
gnprice added a commit to gnprice/zulip-mobile that referenced this issue Nov 16, 2021
This puts it in the context of the notification where it's used.

And in fact that makes it easier for the linter to see what we're
doing, too, and it starts giving a useful new warning!  Suppress it,
with an explanatory comment.  (We'll also note this on zulip#5101.)
chetas411 pushed a commit to chetas411/zulip-mobile that referenced this issue Nov 28, 2021
This puts it in the context of the notification where it's used.

And in fact that makes it easier for the linter to see what we're
doing, too, and it starts giving a useful new warning!  Suppress it,
with an explanatory comment.  (We'll also note this on zulip#5101.)
@chrisbobbe
Copy link
Contributor

chrisbobbe commented Dec 17, 2021

A few items stand out in the list of breaking changes. Blank checkboxes are things we need to fix before targeting Android 12.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 21, 2021
This change will be required in order to upload new releases to the
Play Store, effective probably 2022-11-01. That isn't very soon, but
we cut it kind of close last time, and we don't want to this time.

The change causes Android 12 and later to apply to our app a number
of behavior changes introduced in that version, documented here:
  https://developer.android.com/about/versions/12/behavior-changes-12

Most of the work for this was zulip#5145, "android notif: Replace our
`NotificationIntentService` with directly opening MainActivity".
Earlier in this series of commits, we also fixed zulip#5171 ("Handle
stricter length limit on Android toasts when targeting Android 12")
and handled the "Safer component exporting" change described at
  https://developer.android.com/about/versions/12/behavior-changes-12#exported .

There were a few other things that stood out in that "Behavior
Changes" article, but none that seem to require any specific action
from us; see
  zulip#5101 (comment) .

Fixes: zulip#5101
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Dec 21, 2021
This change will be required in order to upload new releases to the
Play Store, effective probably 2022-11-01. That isn't very soon, but
we cut it kind of close last time, and we don't want to this time.

The change causes Android 12 and later to apply to our app a number
of behavior changes introduced in that version, documented here:
  https://developer.android.com/about/versions/12/behavior-changes-12

Most of the work for this was zulip#5145, "android notif: Replace our
`NotificationIntentService` with directly opening MainActivity".
Earlier in this series of commits, we also fixed zulip#5171 ("Handle
stricter length limit on Android toasts when targeting Android 12")
and handled the "Safer component exporting" change described at
  https://developer.android.com/about/versions/12/behavior-changes-12#exported .

There were a few other things that stood out in that "Behavior
Changes" article, but none that seem to require any specific action
from us; see
  zulip#5101 (comment) .

Fixes: zulip#5101
sumj25 pushed a commit to sumj25/zulip-mobile that referenced this issue Jan 12, 2022
This change will be required in order to upload new releases to the
Play Store, effective probably 2022-11-01. That isn't very soon, but
we cut it kind of close last time, and we don't want to this time.

The change causes Android 12 and later to apply to our app a number
of behavior changes introduced in that version, documented here:
  https://developer.android.com/about/versions/12/behavior-changes-12

Most of the work for this was zulip#5145, "android notif: Replace our
`NotificationIntentService` with directly opening MainActivity".
Earlier in this series of commits, we also fixed zulip#5171 ("Handle
stricter length limit on Android toasts when targeting Android 12")
and handled the "Safer component exporting" change described at
  https://developer.android.com/about/versions/12/behavior-changes-12#exported .

There were a few other things that stood out in that "Behavior
Changes" article, but none that seem to require any specific action
from us; see
  zulip#5101 (comment) .

Fixes: zulip#5101
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants