Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Allow to reuse sounds for custom notifications #6002

Closed
wants to merge 2 commits into from

Conversation

rummatee
Copy link

@rummatee rummatee commented May 9, 2021

This addresses one point in element-hq/element-web#9687

image

I added a List of already uploaded Sounds, including an option for the default and an option to upload a new Sound.
I changed the behavior of the reset Button to reset the selection to the previously saved option, instead of setting the sound to the default (This is now an option in the list instead)


Here's what your changelog entry will look like:

✨ Features

  • Allow to reuse sounds for custom notifications (#6002). Contributed by @rummatee.

@t3chguy
Copy link
Member

t3chguy commented May 10, 2021

This UX has already been redesigned and awaiting re-implementation as part of the FTUE Notifications project so this might not get the review in needs based on that

@t3chguy t3chguy requested a review from a team May 10, 2021 10:35
Signed-off-by: Florian Schunk <[email protected]>
@MadLittleMods MadLittleMods added Z-Community-PR Issue is solved by a community member's PR T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements labels Jun 2, 2022
@MadLittleMods
Copy link
Contributor

MadLittleMods commented Jun 2, 2022

For reference the FTUE Notifications project never made it.

The concept seems decent to ship if this PR was revived. Haven't looked at the code.

@daniellekirkwood
Copy link

From my perspective this looks like an ok add for now.

I would like for Design to review (already pending a design review, have pinged the Design-Internal room for this)

@amshakal
Copy link

Hello, thanks for contributing! This seems like an acceptable change from my perspective. A few changes I'd request for are:

  1. Can we make sure that the title 'Select a custom sound' follows the same typographic style as the 'Sounds' heading above it? (Colour, text style, spacing etc)?
  2. Can we change the 'Reset' button to 'Use default sound' just so that it's crystal clear what that button does?
  3. Can we use sentence case for the hint/title in the drop down? i.e. change 'custom sound' to 'Custom sound'?

Thank you!

@germain-gg
Copy link
Contributor

Converting this to a draft until we have made the updates mentioned above

@germain-gg germain-gg marked this pull request as draft January 12, 2023 12:44
Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know once the changes have been made 👍 and we'll give this another review

@langleyd langleyd closed this Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants