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

Ongoing notification #1

Closed
wants to merge 1 commit into from
Closed

Ongoing notification #1

wants to merge 1 commit into from

Conversation

simonesestito
Copy link

Avoid dismiss the notification because an error. To close it, you can use the appropriate button.

Avoid dismiss the notification because an error. To close it, you can use the appropriate button.
@tananaev
Copy link
Owner

I'm not convinced that it's a good idea. Some people might find it annoying that you can remove it with a swipe.

@simonesestito
Copy link
Author

I found the swipe annoying, so I forked the repo and "fixed" it myself. I don't know what other people want

@tananaev
Copy link
Owner

I'll watch for feedback from people and maybe merge it in future. Thanks for pull request.

@hmueller01
Copy link

I find it annoying that calculator notification goes away with "clean all". So I forked the repo and added .setOngoing(true) to the notification builder. I havn't seen this pull before and was about to write a pull by myself. But @simonesestito 's solution is much more sophisticated and I vote for that pull!

@tananaev
Copy link
Owner

tananaev commented Aug 2, 2018

If we can make it configurable, I'm happy to accept this change.

hmueller01 pushed a commit to hmueller01/calculator-notification that referenced this pull request Aug 18, 2018
hmueller01 pushed a commit to hmueller01/calculator-notification that referenced this pull request Aug 18, 2018
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

Successfully merging this pull request may close these issues.

None yet

3 participants