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.

Sign up for GitHub

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

Jump to bottom

Change default notif volume for non-1:1s #1359

Closed
ara4n opened this issue Feb 19, 2017 · 9 comments
Closed

Change default notif volume for non-1:1s #1359

ara4n opened this issue Feb 19, 2017 · 9 comments
Assignees
@t3chguy
Labels
A-Notifications P1 S-Minor Impairs non-critical functionality or suitable workarounds exist T-Enhancement Z-FTUE Z-FTUE-Notifications Z-GetYourUpdates Z-Synapse

Comments

@ara4n
Copy link
Member

ara4n commented Feb 19, 2017

All my notifs are turned up way too high. I suspect we should not notify for any message in group rooms by default. To fix up existing rooms we need a way of bulk editing notif levels really. This overlaps a bit with 'night mode' style presets for setting alternative notif levels
@ara4n ara4n added S-Critical Prevents work, causes data loss and/or has no workaround P1 A-Notifications S-Major Severely degrades major functionality or product features, with no satisfactory workaround and removed S-Critical Prevents work, causes data loss and/or has no workaround labels Feb 19, 2017
@ara4n
Copy link
Member Author

ara4n commented Mar 2, 2017 •
edited
Loading

Notification volume control is basically a mess full stop right now:

  • The master overrides in UserSettings (e.g. "turn group chat notifs to 'off'") are not reflected whatsoever in the volume context menu for that room. We can't even tell if they're overrides or not!
  • People can't find that menu anyway (plus it should be merged with the general avatar menu rather than hiding two of them there: consider combining the two RoomTile context menus element-web#3013)
  • We don't show the current volume state of the room in RoomTiles in RoomList (#3346)
  • The volume control refers to the 'noisy' setting as 'loud' rather than 'noisy'.
  • ...and there's no way to bulk-edit rooms anyway, short of the confusing radio-button overrides in UserSettings.

So, what shall we do? After brainstorming with @lukebarnard1:

  • Merge 'tag' & volume control context menus together (consider combining the two RoomTile context menus element-web#3013). Let's put the volume on top and 'tag' below with a horizontal separator in between like the tag menu has already
  • Fix the label to be 'noisy' rather than 'loud'
  • If the room volume has been overridden by the radio buttons in UserSettings, grey out the selector or replace it with a "set to noisy in Settings" label (with a link through to User Settings"? Assuming it overrides at all?
  • Create a UnkDevDialog style Dialog which you get at from advanced settings for notifications in UserSettings, which:
    • Table, listing all the rooms
    • Dropdown (<select/>) per room to select between mute / mentions-only / normal / noisy
    • Checkbox on LHS to allow bulk-selection for bulk-operations, using a bulk assign dropdown (similar to the GH Issues one)
    • Allow shift-clicks to bulk-select subsets of the checkboxes.

Stuff we're descoping from this:

@ara4n ara4n mentioned this issue Mar 8, 2017
Open
@lampholder lampholder mentioned this issue Mar 9, 2017
Open
@lukebarnard1 lukebarnard1 mentioned this issue Mar 14, 2017
Merged
@lampholder lampholder mentioned this issue Apr 24, 2017
Open
@ara4n
Copy link
Member Author

ara4n commented Sep 18, 2017 •
edited
Loading

Further thought was that bulk editing is probably not needed that often, as it's fairly rare to override per-room, and as long as we can easily see these overrides from RoomList we're probably okay. The reason I was worrying about it is that i wanted to turn off all of the notifs of my group chats. However, i'd forgotten we have a setting for this already:

screen shot 2017-09-18 at 01 15 57

So, I think all this bug really needs is:

  • Change default notifs to have group chats not push by default.
  • s/Loud/Noisy/ in contextual menu
  • We show the current default volume for a room in the contextual menu somehow - possibly by defaulting the control to a "Default (Loud)" setting or something, and then have the other controls be overrides?
  • Show per-room volume override icons in the RoomList (#3346)

@lukebarnard1 lukebarnard1 removed their assignment Nov 6, 2017
@ara4n ara4n mentioned this issue Dec 31, 2017
Open
@ara4n
Copy link
Member Author

ara4n commented Feb 20, 2018 •
edited
Loading

Just had another iteration on this with @lampholder . Conclusions were:

  1. Changing default for group chat to not notify... for new users:
  • Add background task to synapse on upgrading that adds an explicit push rule for .m.rule.message to the old behaviour.
  • Change the base rule (which will then apply to new users)
  1. Riot/Web: make badges for missed notifs red rather than green if the room is tagged as noisy.
  • We can't just do this by changing the default highlight tweak because that really does mean "make this event red".
  • Option 1: We could add a new tweak for: { "priority": "high" } - and change the clients to interpret that tweak to mean "don't make the message red, but do make the badge red"
    • This means we can apply custom UI nicely for high priority messages.
    • ...and provide a proper push rule for toggling it on & off
    • Change the 'noisy' UI option in push settings to also set { "priority": "high" }?
  • Option 2: Or we fix highlight rules in general to specify the things that they highlight, and make them work with E2E too.
  • Going with Option 1 for now. - actually, ditch this entirely; see below.
  1. Put little mute icons (or 'noisy' icons) on RoomTiles.
  • If an entire sublist is noisified (e.g. 1:1s) then we could put it on the section header?

@ara4n ara4n changed the title Change default notif volume and bulk edit existing rooms Change default notif volume for non-1:1s Feb 20, 2018
@ara4n
Copy link
Member Author

ara4n commented Feb 21, 2018

@AmandineLP correctly pointed out that our current default rules have 1:1s as 'noisy' by default - and so shoving red badges all over the 1:1s by default risks overloading the user with big red lights for no particularly good reason.

Perhaps a better solution for item 2 above would be to finally implement element-hq/riot-meta#96 and turn it on by default; making sure that rooms with missed notifs always rise to the top of their section. This might also solve @lampholder's problems of missing mentions.

@ara4n ara4n added S-Critical Prevents work, causes data loss and/or has no workaround and removed S-Major Severely degrades major functionality or product features, with no satisfactory workaround labels Sep 12, 2018
@ara4n
Copy link
Member Author

ara4n commented Jan 19, 2019

@turt2live why is this one in can't fix?

@turt2live
Copy link
Member

It's flagged as a synapse problem due to the conversation history in the comments.

@ara4n ara4n added the Z-FTUE label Sep 12, 2019
ara4n referenced this issue in matrix-org/synapse Sep 13, 2019
@ara4n
don't push by default for group chats.
b18f54c 
see https://github.com/vector-im/riot-web/issues/3268

This only works for new servers (e.g. mozilla-test.modular.im)
@ara4n ara4n mentioned this issue Sep 13, 2019
Merged
ara4n referenced this issue in matrix-org/synapse Sep 19, 2019
@ara4n
Add experimental "dont_push" push action to suppress push for notific…
2292dc3 
…ations

This is a potential solution to https://github.com/vector-im/riot-web/issues/3374
and element-hq/element-web#5953
as raised by Mozilla at element-hq/element-web#10868.

This lets you define a push rule action which increases the badge count (unread notification)
count on a given room, but doesn't actually send a push for that notification via email or HTTP.
We might want to define this as the default behaviour for group chats in future
to solve https://github.com/vector-im/riot-web/issues/3268 at last.

This is implemented as a string action rather than a tweak because:
 * Other pushers don't care about the tweak, given they won't ever get pushed
 * The DB can store the tweak more efficiently using the existing `notify` table.
 * It avoids breaking the default_notif/highlight_action optimisations.

Clients which generate their own notifs (e.g. desktop notifs from Riot/Web
would need to be aware of the new push action) to uphold it.

An alternative way to do this would be to maintain a `msg_count` alongside
`highlight_count` and `notification_count` in `unread_notifications` in sync responses.
However, doing this by counting the rows in `events` since the `stream_position`
of the user's last read receipt turns out to be painfully slow (~200ms), perhaps
due to the size of the events table.  So instead, we use the highly optimised
existing event_push_actions (and event_push_actions_staging) table to maintain
the counts - using the code paths which already exist for tracking unread
notification counts efficiently.  These queries are typically ~3ms or so.

The biggest issues I see here are:
 * We're slightly repurposing the `notif` field on `event_push_actions` to
   track whether a given action actually sent a `push` or not.  This doesn't
   seem unreasonable, but it's slightly naughty given that previously the
   field explicitly tracked whether `notify` was true for the action (and
   as a result, it was uselessly always set to 1 in the DB).
 * We're going to put more load on the `event_push_actions` table for all the
   random group chats which people had previously muted. In practice i don't
   think there are many of these though.
 * There isn't an MSC for this yet (although this comment could become one).
@ara4n ara4n mentioned this issue Sep 19, 2019
Closed
@ara4n
Copy link
Member Author

ara4n commented Sep 19, 2019

One thought: we probably only want to turn off notifs on publicly joinable rooms by default. WhatsApp etc show that notifs in private group rooms are probably desirable.

@neilisfragile neilisfragile mentioned this issue Jun 4, 2020
Closed
@t3chguy t3chguy self-assigned this Sep 9, 2020
@novocaine novocaine added S-Minor Impairs non-critical functionality or suitable workarounds exist and removed S-Critical Prevents work, causes data loss and/or has no workaround labels Aug 4, 2021
@novocaine
Copy link

@ara4n this work has shifted over time - what are the acceptance criteria that would allow us to close this ticket at this point?

@t3chguy t3chguy mentioned this issue Apr 28, 2022
Closed
@t3chguy t3chguy transferred this issue from element-hq/element-web Apr 4, 2023
@daniellekirkwood
Copy link
Contributor

I'm going to close this issue but also link it to the related discussion here: #1595

We're in the process of simplifying notifications across all of our platforms and the current decision is to not make any changes to the defaults for new users based on room type. Just because a room has several people in it, we do not think that we're currently intelligent enough to make this decision on behalf of users. Having said that, there is likely a future where this is a great path for us to take (hence keeping the discussion, comments and feedback in Discussions).

We are taking other steps to make notifications easier to manage (these are all being tracked elsewhere) and include, but not limited to:

  • simplifying the settings page: we believe if settings are easier to change not defaulting different settings for different room types is easier to manage for new users
  • making the quick settings for a room easier to access, change, and grok
  • making the notifications settings of a room clear when you join it with 'point of need information' (see draft design)
  • adding smart banners so if we believe someone has the 'wrong' setting, we can suggest that they change it (see draft design)
  • others...

Smart banners:
Screenshot 2023-05-12 at 11 52 02

Screenshot 2023-05-12 at 11 52 15

Point of need:
Screenshot 2023-05-12 at 11 58 03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@t3chguy t3chguy

Labels
A-Notifications P1 S-Minor Impairs non-critical functionality or suitable workarounds exist T-Enhancement Z-FTUE Z-FTUE-Notifications Z-GetYourUpdates Z-Synapse
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@novocaine @jryans @turt2live @ara4n @t3chguy @lukebarnard1 @daniellekirkwood