-
-
Notifications
You must be signed in to change notification settings - Fork 674
android: Update target SDK version to 31, i.e., Android 12 #5174
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
Conversation
Please also see #5175 for some further improvements to toast texts, and more. |
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.
Thanks for taking care of this! Looks great; small comments below.
src/action-sheets/index.js
Outdated
@@ -587,3 +624,53 @@ export const showStreamActionSheet = ({ | |||
}), | |||
); | |||
}; | |||
|
|||
export const showPmActionSheet = ({ |
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.
Really more like "PM conversation action sheet", right? For an individual PM, there's the message action sheet.
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.
(Similarly for the function constructing the buttons.)
src/compose/ComposeBox.js
Outdated
this.setMessageInputValue( | ||
this.state.message.replace( | ||
placeholder, | ||
`[${_('Uploading {fileName} failed.', { fileName })}]()`, | ||
`[${_('Failed to upload file: {fileName}.', { fileName })}]()`, |
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.
Let's also drop the .
at the end. That works when the filename is in the middle of the sentence, but here it risks looking like part of the filename; and in general I think we don't put periods at the end of these short messages.
|}>; | ||
|
||
export default function GroupDetailsScreen(props: Props): Node { | ||
/** | ||
* Details about a PM conversation, identified by PM key recipients. |
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 second half of this is redundant with the type. Best to leave it out -- that way it doesn't become wrong if we change the type in the future (renaming it, or changing to something like PmNarrow
, or something else.)
c05e11e
to
4bb1a85
Compare
Thanks! Revision pushed. |
…vice Looks like we missed this in bb2000f.
Thanks to Greg for aligning the type of the `recipients` route param with this new name, in the recent commit 06a4abc. Suggested-by: Greg Price <greg@zulip.com>
Instead of a toast with the recipients' names. When we start targeting Android 12, which we hope to do soon, toasts will be limited to two lines [1]. It was never a great design to use a toast here in the first place, since in theory you can have a group PM conversation with arbitrarily many members, and there's only so big a toast can get before it starts to look weird or cut things off in any implementation. With just two lines of space, the toasts will be even less helpful. Better to give an action sheet, like we do on long-pressing topic headers. The action sheet gives a button to take you to the PM recipients screen, where you can see all the details of the recipients. [1] https://developer.android.com/about/versions/12/behavior-changes-12#toast-redesign Fixes-part-of: zulip#5171
The "failed" part is really important, and we can't have it get cut off [1] just because a file name is very long. [1] https://developer.android.com/about/versions/12/behavior-changes-12#toast-redesign Fixes: zulip#5171
…id 12 See the change described at https://developer.android.com/about/versions/12/behavior-changes-12#exported . A value of "true" means that the component accepts input from other apps. - The launcher is another app, so we need "true" there so that Zulip can be launched. - We need "true" for the share-to-Zulip activity; the content to be shared originates from other apps.
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
4bb1a85
to
8c2440c
Compare
Thanks! Looks good -- merging. |
Thanks! |
Fixes: #5101
Fixes: #5171