Add a setting to allow users to set account-wide custom notification sound #10494

kenwuuu · 2023-03-31T19:04:06Z

Issues Addressed

Custom notification issues

element-hq/element-web#5891
element-hq/element-web#20286
element-hq/element-web#15023

Planned simple changes

I'll work on these separately (new PR) if someone with write access tells me that they'll get this PR through first.
element-hq/element-web#18392
element-hq/element-web#20864

Changes

extract NotificationSound.tsx component
add NotificationSound.tsx to Notifications.tsx
allow NotificationSound.tsx to set/get notification sound without roomId
rename getSoundForRoom(), generalize function usage
rename some unnecessarily short variable names

How to test

To upload

clone, yarn start in root
open localhost:8080
click avatar at top left
open Notifications
upload sexy sound (no visual indicator of success)
make a friend. wait for them to message you

Screenshot of change

Here's what your changelog entry will look like:

✨ Features

Add a setting to allow users to set account-wide custom notification sound for messages (#10494). Fixes element-hq/element-web#5891; element-hq/element-web#20286; element-hq/element-web#15023. Contributed by @kenwuuu

Checklist

Tests written for new code (and old code if feasible)
Linter and other CI checks pass
Sign-off given on the changes (see CONTRIBUTING.md)

This PR currently has none of the required changelog labels.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

- extract NotificationSound.tsx component - rename getSoundForRoom, generalize function usage - rename unnecessarily short variable name

kenwuuu · 2023-03-31T19:30:52Z

@Half-Shot @t3chguy @turt2live

Hey folks, just looking to get confirmation that this is a feature that we're willing to merge in if I finish it out. You can test it out yourself.

ShadowJonathan · 2023-04-01T16:54:49Z

@kenwuuu thank you a lot for making this PR, and taking the time to try to resolve this 💚

…ix-org#10582) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* #21451 Fix WebGl disabled error message * #21451 Fix WebGl disabled error message Signed-off-by: Rashmit Pankhania <rashmitpankhania@gmail.com> Signed-off-by: Rashmit Pankhania <raspankh@publicisgroupe.net> * Fix message Signed-off-by: Rashmit Pankhania <raspankh@publicisgroupe.net> * Fix ordering of cases in LocationShareErrors.ts Signed-off-by: Rashmit Pankhania <raspankh@publicisgroupe.net> * Fix linting LocationPicker.tsx Signed-off-by: Rashmit Pankhania <raspankh@publicisgroupe.net> * Fix eslint Signed-off-by: Rashmit Pankhania <raspankh@publicisgroupe.net> * Fix file encoding for i18n CI issue Signed-off-by: Rashmit Pankhania <raspankh@publicisgroupe.net> * Fix ts strict CI issue Signed-off-by: Rashmit Pankhania <raspankh@publicisgroupe.net> --------- Signed-off-by: Rashmit Pankhania <rashmitpankhania@gmail.com> Signed-off-by: Rashmit Pankhania <raspankh@publicisgroupe.net> Co-authored-by: Rashmit Pankhania <raspankh@publicisgroupe.net> Co-authored-by: Kerry <kerrya@element.io>

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Conform more of the codebase to `strictNullChecks` * Conform more of the codebase to `strictNullChecks` * Fix types

matrix-org#10617) 'data-test-id' is not discoverable with findByTestId() of Cypress Testing Library. Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Update room.spec.ts - use Cypress Testing Library Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com> * Empty commit Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com> --------- Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Co-authored-by: Kerry <kerrya@element.io>

…0612) * Update location.spec.ts - use Cypress Testing Library Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com> * Make the test id of location share option discoverable with findByTestId() findByTestId seeks for data-testid, instead of data-test-id. Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com> --------- Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

…10611) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Conform more of the codebase to `strictNullChecks` * Conform more of the codebase to `strictNullChecks` * Fix types * Conform more of the codebase to `strictNullChecks` * Conform more of the codebase to `strictNullChecks`

…sting Library (matrix-org#10619) * Update support files - use Cypress Testing Library Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com> * Fix openMessageComposerOptions() Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com> --------- Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

…0613) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

…10539) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* replace hardcoded strings with MsgType constants * fix import and revert comments Signed-off-by: Ken Wu kenqiwu@gmail.com * fix import Signed-off-by: Ken Wu kenqiwu@gmail.com --------- Signed-off-by: Ken Wu kenqiwu@gmail.com

…rg#10596) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

…0574) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

…#10591) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

…atrix-org#10626)

* Mark effects overlay canvas as aria hidden * Ensure date separators aren't seen as focusable aria separators * Fix * Fix font slider not having aria label * Add missing aria labels * Fix settings flags setting aria-checked={null} * Update snapshots

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

- extract NotificationSound.tsx component - rename getSoundForRoom, generalize function usage - rename unnecessarily short variable name

…on_sound' into feature/custom_global_notification_sound # Conflicts: # src/components/views/settings/NotificationSound.tsx

robintown

Hi, thanks for your contribution! The first order of business will be to get our product team's input on whether this is in fact the kind of change that we want to make to our notification settings. In particular, I think our team has a project in progress to improve the notifications settings page that could conflict with this change.

From a quick look at the code, I think something went wrong with one of your merges? The diff includes quite a few unrelated changes at the moment.

robintown · 2023-04-25T04:14:02Z

src/components/views/settings/NotificationSound.tsx

+import AccessibleButton, {ButtonEvent} from "../elements/AccessibleButton";
+import {chromeFileInputFix} from "../../../utils/BrowserWorkarounds";
+import {SettingLevel} from "../../../settings/SettingLevel";
+import {logger} from "../../../../../matrix-js-sdk/src/logger";


This isn't compiling in our CI, because it needs to be an absolute import:

Suggested change

import {logger} from "../../../../../matrix-js-sdk/src/logger";

import {logger} from "matrix-js-sdk/src/logger";

joshuacheong · 2023-10-21T05:52:23Z

@kenwuuu @robintown
It has been months, I desperately need sound customization. It gives me anxiety to listen to this unholy notification and causes unnecessary stress especially on a weekend.

claell · 2023-11-13T18:22:13Z

Product apparently gave feedback on the general feature. But not here 🥲.

See element-hq/element-web#5891 (comment)

@daniellekirkwood @kongo09

Also note, that you said:

Instead we're spending our time focussed on bugs such as stuck notifications, accessibility, timeline jumpiness, etc.

But this doesn't make sense. This one is an accessibility bug to users, apparently. Some even say "health" affecting (not sure about that). So if you work on accessibility, here you go, this is an important accessibility problem, which is additionally easy to provide with a quick fix for now, that can then perfected in later work 🙂.

dbkr · 2024-05-30T09:38:05Z

With apologies to all those who are desperately wanting a way to change the sound, I'm going to close this PR: this isn't necessarily to say that we don't want the feature, but this one isn't in a mergeable state and hasn't been updated in over a year. If someone were motivated enough, they could extract the changes and make a fresh PR.

joshuacheong · 2024-05-30T09:44:09Z

I've disabled the sound and accepted that I would never use this messaging application and protocol for emergencies. Unfortunate but a reality.

kenwuuu and others added 2 commits March 31, 2023 14:01

Add custom notification setting

c97f619

- extract NotificationSound.tsx component - rename getSoundForRoom, generalize function usage - rename unnecessarily short variable name

Merge branch 'develop' into feature/custom_global_notification_sound

164dc99

github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Mar 31, 2023

kenwuuu mentioned this pull request Mar 31, 2023

Custom sound packs? (My 🐕 hates the incoming message sound) element-hq/element-web#15023

Closed

kenwuuu changed the title ~~Add a setting to allow users to set custom notification sound~~ Add a setting to allow users to set account-wide custom notification sound Mar 31, 2023

kenwuuu added 4 commits April 8, 2023 15:45

Merge branch 'develop' into feature/custom_global_notification_sound

cef6c6d

Call NotificationSound with roomId

b2032f1

Merge branch 'develop' into feature/custom_global_notification_sound

3839113

remove comment

c6c5112

kenwuuu marked this pull request as ready for review April 16, 2023 00:20

kenwuuu requested a review from a team as a code owner April 16, 2023 00:20

kenwuuu requested review from justjanne and robintown April 16, 2023 00:20

luixxiul and others added 15 commits April 16, 2023 22:50

Update pills-click-in-app.spec.ts - use Cypress Testing Library (matr…

41c0b08

…ix-org#10582) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Update stickers.spec.ts - use Cypress Testing Library (matrix-org#10622)

9385881

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Conform more of the codebase to strictNullChecks (matrix-org#10602)

daad630

* Conform more of the codebase to `strictNullChecks` * Conform more of the codebase to `strictNullChecks` * Fix types

Make test ID compatible with findByTestId() of Cypress Testing Library (

8a4a584

matrix-org#10617) 'data-test-id' is not discoverable with findByTestId() of Cypress Testing Library. Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Apply strictNullChecks to Markdown.ts (matrix-org#10623)

1d9df7e

Co-authored-by: Kerry <kerrya@element.io>

Update pollHistory.spec.ts - use Cypress Testing Library (matrix-org#…

816a078

…10611) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Update send_event.spec.ts - use Cypress Testing Library (matrix-org#1…

8471e45

…0613) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Update right-panel.spec.ts - use Cypress Testing Library (matrix-org#…

4d859a3

…10539) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Update room-directory.spec.ts - use Cypress Testing Library (matrix-o…

013b720

…rg#10596) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

luixxiul and others added 13 commits April 17, 2023 11:07

Update file-panel.spec.ts - use Cypress Testing Library (matrix-org#1…

9c277d6

…0574) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Make SonarCloud happier about our code quality (matrix-org#10630)

568ec77

Update spotlight.spec.ts - use Cypress Testing Library (matrix-org#10621

7114f82

) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Update lazy-loading.spec.ts - use Cypress Testing Library (matrix-org…

a092a91

…#10591) Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Fix view source from edit history dialog always showing latest event (m…

270a26d

…atrix-org#10626)

Update polls.spec.ts - use Cypress Testing Library (matrix-org#10609)

d1461d3

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

Add missing rel noreferrer noopener attributes (matrix-org#10629)

6bf1815

Add custom notification setting

2cf6782

- extract NotificationSound.tsx component - rename getSoundForRoom, generalize function usage - rename unnecessarily short variable name

Call NotificationSound with roomId

aa90786

remove comment

037be66

add aria-label

1c64888

Merge remote-tracking branch 'origin/feature/custom_global_notificati…

37f35d1

…on_sound' into feature/custom_global_notification_sound # Conflicts: # src/components/views/settings/NotificationSound.tsx

robintown reviewed Apr 25, 2023

View reviewed changes

robintown requested review from a team and removed request for justjanne April 25, 2023 04:28

This was referenced May 27, 2023

Add interface for setting a global notififcation sound element-hq/element-web#20286

Open

Ringtone and Notification sounds are jarring/stressful element-hq/element-web#5031

Open

ShadowJonathan mentioned this pull request Nov 13, 2023

Support for multiple matrix accounts element-hq/element-web#2320

Open

dbkr closed this May 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a setting to allow users to set account-wide custom notification sound #10494

Add a setting to allow users to set account-wide custom notification sound #10494

kenwuuu commented Mar 31, 2023 •

edited by github-actions bot

Loading

kenwuuu commented Mar 31, 2023 •

edited

Loading

ShadowJonathan commented Apr 1, 2023

robintown left a comment •

edited

Loading

robintown Apr 25, 2023

joshuacheong commented Oct 21, 2023 •

edited

Loading

claell commented Nov 13, 2023

dbkr commented May 30, 2024

joshuacheong commented May 30, 2024 •

edited

Loading

	import {logger} from "../../../../../matrix-js-sdk/src/logger";
	import {logger} from "matrix-js-sdk/src/logger";

Add a setting to allow users to set account-wide custom notification sound #10494

Add a setting to allow users to set account-wide custom notification sound #10494

Conversation

kenwuuu commented Mar 31, 2023 • edited by github-actions bot Loading

Issues Addressed

Custom notification issues

Planned simple changes

Changes

How to test

To upload

Screenshot of change

✨ Features

Checklist

kenwuuu commented Mar 31, 2023 • edited Loading

ShadowJonathan commented Apr 1, 2023

robintown left a comment • edited Loading

Choose a reason for hiding this comment

robintown Apr 25, 2023

Choose a reason for hiding this comment

joshuacheong commented Oct 21, 2023 • edited Loading

claell commented Nov 13, 2023

dbkr commented May 30, 2024

joshuacheong commented May 30, 2024 • edited Loading

kenwuuu commented Mar 31, 2023 •

edited by github-actions bot

Loading

kenwuuu commented Mar 31, 2023 •

edited

Loading

robintown left a comment •

edited

Loading

joshuacheong commented Oct 21, 2023 •

edited

Loading

joshuacheong commented May 30, 2024 •

edited

Loading