-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add Profile.BellSound to Settings UI #17983
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
Probably we don't need to have one browse button per line. Since the indices don't matter in the backend, there is no difference between "replace an entry inline" and "delete old entry, click add (which opens a Browse window)", except that the first is much more complicated from both a developer standpoint (tracking which index to replace) and a user one ("why are there so many buttons?") |
This comment has been minimized.
This comment has been minimized.
Added the demo above. Yeah, for designs 2 & 3, we I think we may be able to remove I'm gonna hold off on making that change for a bit though, because I want consensus on a design first. Adding "Needs-Discussion" so we touch base on the design at the upcoming sync. |
Feedback from team sync (10/7):
|
Summary of the Pull Request
Adds the Profile.BellSound setting to the Settings UI under the Profile > Advanced page.
CurrentBellSounds
keeps track of the bell sounds added and exposed via the UI.BellSoundViewModel
wraps each sound. This allows us to listen (and propagate) changes to the registered sounds.BellSoundPreview
provides a written preview of the current bell sound to display in the expander#10000