Skip to content

notif: Enable autoCancel flag for summary notification #755

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

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

rajveermalviya
Copy link
Member

While trying out the release build from main on a relatively older (Android 11) device, I found that it results in lingering summary notif:

Android 14 Android 11
android-14.mp4
android-11.mp4

Specifying autoCancel: true fixes this behavior on the older device.

@rajveermalviya rajveermalviya force-pushed the summary-notif-autocancel branch from 6d37e7b to 6139855 Compare June 19, 2024 09:16
@rajveermalviya rajveermalviya changed the title notif: Specify autoCancel for summary notification notif: Enable autoCancel flag for summary notification Jun 19, 2024
On Android 11 and lower, if autoCancel is not specified,
the summary notification may linger even after all child
notifications have been opened and cleared.
@rajveermalviya rajveermalviya force-pushed the summary-notif-autocancel branch from 6139855 to ea0144e Compare June 19, 2024 13:16
@rajveermalviya
Copy link
Member Author

So, I tried the emulators (from Android 12 - Android 9) and turns out it's specifically the behavior on Android 11 (API 30) and lower:

Android 11 Android 10 Android 9
api-30 api-29 api-28

It would have been great if this was documented somewhere ^^. Also the need for an integration test that would automate this and would have caught it — if it was set-up to run on multiple emulators with different Android versions. But definitely not great for each CI run, but could be good to run before each release in CI or locally.

@gnprice
Copy link
Member

gnprice commented Jun 20, 2024

Excellent, thanks for following up and for the thorough investigation!

(Previous thread for cross-reference: #703 (comment) .)

Merging this. I'll also add a line in the comment, to use our pattern for marking things we can simplify away when we drop support for an old platform. (We most commonly use this for versions of the Zulip server, the TODO(server-N) comments mentioned in the README, but it applies equally well for versions of Android or iOS.)

It would have been great if this was documented somewhere ^^.

Yep. Android unfortunately isn't real consistent about documenting this sort of thing.

Also the need for an integration test that would automate this and would have caught it

Yeah, agreed. I'll go file an issue or two about doing that. We're not going to invest in it soon; but I want to record this experience, because it's a great motivating example to have in mind when the launch is complete and we're thinking about what to prioritize next.

@gnprice gnprice merged commit 8bf700c into zulip:main Jun 20, 2024
1 check passed
@rajveermalviya rajveermalviya deleted the summary-notif-autocancel branch June 20, 2024 19:49
@gnprice
Copy link
Member

gnprice commented Jun 20, 2024

I'll go file an issue or two about doing that.

Done:

@rajveermalviya
Copy link
Member Author

Done

Great, Thanks @gnprice!

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

Successfully merging this pull request may close these issues.

2 participants