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

Notifications centralization #4944

Open
mikehardy opened this issue Sep 11, 2018 · 23 comments
Open

Notifications centralization #4944

mikehardy opened this issue Sep 11, 2018 · 23 comments
Labels
Accepted Maintainers welcome a PR implementing this feature Advanced Reminders Enhancement Keep Open avoids the stale bot Priority-Medium UI

Comments

@mikehardy
Copy link
Member

Per a comment on #4928 - we need notifications to be more coherent.

On the implementation side the receivers and services can be streamlined, and the UI in particular needs to be implemented in a single place.

First job is to define what that means:

I had some visions on "what should notifications look like" - similar to your thinking on a unified control panel for auto-syncing, I think notifications should be in a single spot (though I'm not 100% against an entry in Deck Options, I'd have it connect to a main notifications control panel)

On that sub-preference, I'd see a top section that indicates can configure global vibration and LED setting, that you can turn on or off a global notification for total work due, and you can set a threshold for that work. While the current global notification is slaved to "start of day" time right now I think it would be nice to set the time.

Then under a divider, I could see a recycler view of decks, with checkboxes of reminders per deck, then the deck name, then the notification time. If that's two cramped, I'd show checkboxes and deck name and pop up the time dialog for edit. Bonus points for making the checkboxes and deck title sortable.

I want the deck notifications to actually be per-deck, not per-deck-options (as they are now) but I'm not sure if that's possible. If it is not possible then I'd have the scroll be for deck options and when you click in to it, you see which decks are using it. I really don't like that but it's only if I can't do per-deck at all - I'm simply unsure on that point.

@timrae
Copy link
Member

timrae commented Sep 11, 2018

I had some visions

Could you make a rough sketch?

I want the deck notifications to actually be per-deck, not per-deck-options (as they are now) but I'm not sure if that's possible.

It is definitely possible, and not terribly difficult, you just need to store the values in an sqlite db rather than into the collection (see for example the way TTS state is saved)

@mikehardy
Copy link
Member Author

  1. Added possible entries in the main screen (does it ever make sense to alphabetize these menus? even with localization in play? Or do we just hard-code order forever? Just curious)

anki_main_preferences_mock_20180911

  1. Mock of combined Notifications control panel. The little things on the right side are dialog fragments that popup to set the combined notification, or to set or edit the time if you turn on a deck notification. Should be a RecyclerView and that's the little up/down arrow on the right side of main screen

anki_notifications_preferences_mock_20180911

Alright, barf all over this nauseatingly amateur UI sketch ;-)

@timrae timrae added Enhancement Priority-High UI Accepted Maintainers welcome a PR implementing this feature labels Sep 12, 2018
@timrae
Copy link
Member

timrae commented Sep 12, 2018

I'm not sure it makes sense to concurrently have both a global reminder and a deck specific reminder (i.e. should probably be either/or)...? Also, I think it might be useful to have a threshold for minimum cards for individual decks as well...? How would we handle subdecks in the RecyclerView?

@mikehardy
Copy link
Member Author

I still like the combination reminder idea, but your subdeck point is right on. I don't use them so forgot. Is it just two levels? I'd do quotes and colon separation as a first cut. But I don't feel strongly. Could do indentation

@timrae
Copy link
Member

timrae commented Sep 12, 2018

I still like the combination reminder idea

Perhaps you misunderstood; I'm saying that I don't think it makes sense to concurrently have two separate reminders set for both "all decks" and "deck x". Rather the UI should exclusively force the user to choose between "all decks" or "deck x". And also that a minimum number of cards to trigger the reminder for "deck x" should be available too, rather than the current design where the minimum cards for a specific deck is always 1.

Is it just two levels?

Nope, arbitrary nesting is possible. I think notifications would need to apply recursively from parent to child, so it might make sense to present them in a way that conveys that to the user.

@mikehardy
Copy link
Member Author

mikehardy commented Sep 12, 2018

Okay - so perhaps there is a divider separating global reminder and deck reminders, and if global is selected then per-deck is greyed out, and vice versa. I could see it working that way, but I have to say I'm not generally in favor of disabling something that's there otherwise, just because I think I know how people want to use things. As mentioned elsewhere though, I'm aware I have questionable taste vs the majority so I could go either way but it's simpler to implement and my preference to just allow any given reminder to be turned on any time (though they'd all default off of course, or maybe a global one one at 25 or something).

For the sub-decks, seems if I follow the DeckPicker presentation or even encapsulate it, with an added checkbox decoration on a left column and an "edit pencil" icon on the right column, I could allow a notification for any particular deck.

If recursive behavior was desired, it could get complicated quickly. I could have a tri-state checkbox, empty, filled, dashed where filled means a notification for sum of all work due for deck + children, and dashed means notification for sum of deck + all selected children, and you could unselect various children. I've seen that idiom before anyway. I could display the edit pencil only for the top of any recursive hierarchy, and in there set the time and work due threshold. That idea sketch would impose a constraint that you could not have both a notification for a parent of a sub-deck and an independent notification for a sub-deck. If that constraint wasn't okay, in the deck-specific notification editor we could have a toggle for "include child deck(s) in work due count?" and display the tree of child decks to toggle on or off to include. Then the main notification panel would be a copy of Deck Picker with checkbox and edit pencil, with all selectable as simple toggles and, recursive inclusion specified there for children if they exist and the user wants? I kind of like that. Yes, possible duplication but also just whatever people want, separate, multiple, single-but-recursive, etc.

@mikehardy
Copy link
Member Author

anki_notifications_preferences_mock_20180912

@mikehardy mikehardy changed the title Enhancement: Notifications centralization Notifications centralization Sep 12, 2018
@timrae
Copy link
Member

timrae commented Sep 14, 2018

Well this escalated quickly lol... I'm starting to think the current UI with the options groups is much more realistic.

@mikehardy
Copy link
Member Author

I'm nearly done with the back-end janitorial duties but yeah, the front-end is pretty heavy if you want to handle recursion etc. If it were in stages - first centralize to single screen but with existing deck options, then go to per-deck (with indents, but no recursion), then maybe recursion I think each step on it's own is achievable and especially the first which starts things rolling. Is half a loaf worth (centralize the UI, but leave features as is) worth anything?

@timrae
Copy link
Member

timrae commented Sep 14, 2018

Well honestly speaking I've been investing far more time into the project than I'm comfortable with recently, so I think a major feature redesign like this might need to wait until after 2.9 is released. For now I feel it's probably better to just do the backend unification changes, and potentially adding feature parity to the UI for the global vs. deck-specific reminders to reflect that backend unification (i.e. expose the minimum cards option in the deck-specific reminders UI, and give an option to configure the time in the global reminders).

@mikehardy
Copy link
Member Author

roger that - and yes, I have been asking a lot, I know. Okay, time in global, count in deck options reminder. If you want to time-box the commitment for 2.9, you want to set a milestone for what's required and tag things with it then we can punch it down and pause for a bit after?

@timrae timrae added this to the v2.9 release milestone Sep 14, 2018
@timrae
Copy link
Member

timrae commented Sep 14, 2018

Yeah I think we need to start focusing on getting a 2.9 beta out, at which point we'll make a beta branch with a freeze on new features, and we cherry-pick each commit from master into the beta branch. I've added a milestone now, and tagged a few things, am I missing anything? It might be good to make a new issue to discuss the release in a central place and maintain a checklist of issues at the top. Would you mind creating one?

@timrae
Copy link
Member

timrae commented Sep 14, 2018

Actually I guess a checklist is unnecessary with the creation of the milestone, but an issue would be useful I think.

@mikehardy
Copy link
Member Author

2.9 coordination: #4961

@github-actions
Copy link
Contributor

Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

@mikehardy
Copy link
Member Author

mikehardy commented Mar 18, 2021

From #8264 - related to how notifications and sync interact, since sync is an external way for data to change completely, which can/should impact notification state


Unrelated: for a real "make the feature work" fix, which I would define as "notification service does not crash and if sync starts before or during notification activity, they interact correctly", it would look something like:

  • there is the ability (via an API like SomeSyncObject.isSyncInProgress()) to know if a sync is in progress, as that can change data completely and thus make any notifications based on old data obsolete
  • notification service while working can periodically check that status and stop working, additionally if it hits any exception cases related to database being closed it can check that and log a message while stopping work gracefully, maybe saving a pref indicating that it was interrupted
  • sync when it is done, can send an event to the notification service that says essentially "a sync was completed, notifications based on old data should be rebuilt"
  • notification service, if was interrupted can generate new notifications, or if not interrupted but there are existing notifications it may update them.

@lionslancer
Copy link

lionslancer commented Sep 5, 2021

Considerations for notifications rework

I've been thinking a lot about this as a user recently, and one thing I've realized is that before one even begins to think about the implementation, there are a lot of things that have to be considered beyond what is immediately obvious. I apologize if any of what I describe below is overly verbose, unclear, unhelpful, or considered common knowledge. I hope it can be of some use.

The main question is the level of detail to which notifications should be configurable. On one hand, giving users more flexibility is never a bad thing, but spending a lot of effort to support features that won't actually be used is a waste of time and resources. Most importantly, it would be very unfortunate to design a system that is incompatible with features that may be desirable to add at a later time.

Configuration options

The following are a couple of examples of questions that (in my opinion) need to be considered before a system can be outlined:

  • Should users be able to configure to configure notifications on a per-(sub)deck basis?

    • Should the time at which notifications are delivered be configurable per-deck?
  • Which notifications should be persistent and which should be possible to swipe away?

    • Should this property be configurable by the user?
      • Should it then be possible to create per-deck persistent notifications?
  • How should notifications group together (summary notifications)?

    • Android 11 changes to notifications may have an impact on this.

Some thoughts about implementation

The first thing to be aware of is that notifications can be configured in two places: in-app, and in Android settings. In order for notification management to be intuitive to the user, both should ideally be used, with more general settings done via the operating system, and more detailed settings inside the app. The OS notification settings can actually link directly to the in-app settings via an option called "Additional settings in the app". Similarly, the in-app notification settings can link back to the OS settings.

Here is a rough outline of the two areas and things that need to be considered for each:

  • Android notification settings
    • Notification channels
      • Silent vs. Audible notifications (Android 11)
      • Notification groups (bundles)
      • Notification channel groups
  • In-app notification settings
    • Toggling individual notifications on and off
    • Customizing per-deck notifications?

Android settings

Within the OS-level settings, the main thing to take note of is proper usage of notification channels. As users have ultimate control of notification channel priority-level, assigning notifications to appropriate channels is important in order to ensure that important notifications are delivered even when the user turns another channel off.

Since Android 11, silent and audible notifications are visibly separated in the notification shade, which is also going to impact how notifications are displayed to the user. One approach I've seen that accounts for this is to only have 2 channels: one for audible notications and one for silent ones, and then configure all other details in-app. The problem with this approach is that if users ever tamper with the default notification settings, the whole system falls apart.

Notifications groups affect the creation of summary notifications when an app posts many notifications (such as an expandable notification containing all unread emails). Notification channel groups, allow for the grouping together of multiple related channels. This seems to most often be used as a way of handling multiple accounts, by allowing the user to independently configure notification priority for each email account (as an example).

In-app settings

Unless the 2-channel (silent+audible) approach is used, I imagine that in-app configuration will mainly relate to which decks should have deck-specific notifications enabled, and when these should be posted.

Further reading

These articles describe considerations to avoid overwhelming users with notifications. This may or may not be relevant for older versions of Android depending on how the new system ends up looking.
https://blog.danlew.net/2017/02/07/correctly-handling-bundled-android-notifications/
https://blog.danlew.net/2017/09/06/working-with-android-notification-channels/
https://blog.danlew.net/2018/01/22/notifications-in-android-8-1/

tl;dr

I think the notifications rework should try to integrate well with Android settings, which requires deciding what should go where, which in turn requires a clear idea of what should and should not be possible.

prateek-singh-3212 added a commit to prateek-singh-3212/Anki-Android that referenced this issue Oct 13, 2022
New Notification Preference will show the list of decks as per my GSoC Proposal and dissucced on issue (ankidroid#4944)
prateek-singh-3212 added a commit to prateek-singh-3212/Anki-Android that referenced this issue Oct 17, 2022
New Notification Preference will show the list of decks as per my GSoC Proposal and dissucced on issue (ankidroid#4944)
prateek-singh-3212 added a commit to prateek-singh-3212/Anki-Android that referenced this issue Oct 20, 2022
New Notification Preference will show the list of decks as per my GSoC Proposal and dissucced on issue (ankidroid#4944)
prateek-singh-3212 added a commit to prateek-singh-3212/Anki-Android that referenced this issue Oct 21, 2022
New Notification Preference will show the list of decks as per my GSoC Proposal and dissucced on issue (ankidroid#4944)
@BrayanDSO BrayanDSO removed this from the 2.16 release milestone Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Maintainers welcome a PR implementing this feature Advanced Reminders Enhancement Keep Open avoids the stale bot Priority-Medium UI
Projects
None yet
Development

No branches or pull requests

6 participants