Page MenuHomePhabricator

Add some basic sound preferences
ClosedPublic

Authored by chad on Apr 19 2017, 12:32 AM.

Details

Summary

Ref T7567. This adds some constants (for adding new sounds), global setting for turning on and off sound (setting) and per thread preference for sound choice. Also specc'd out Mentions, if added.

Test Plan

I tested all the preference wiring, but need to set up notifications locally to verify if this works. Feel free to test.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

  • fairly complete, but no JS
epriestley added a subscriber: amckinley.

I'd like to do more infrastructure work before pursuing this -- if we land this as-is, I suspect we'll break all the settings later (for custom sounds, modular sounds, etc).

If the sounds are annoying and you'd like to use this preference to turn them off, I'm also fine with just disabling them until this preference is ready. I had planned to disable the "receive your own message" sound per @amckinley but leave the "receive other message" sound enabled, but not do preferences until after mention/settings/T12562 stuff.

More modular than this? It seemed pretty easy to add additional sounds to and implement. I guess I'm not sure what more features outside of the sound task you were considering.

In this change, all the sounds are hard-coded in the upstream. They're easy for us to extend, but third-party code can't add new sounds.

That's not a feature I'd be interested in seeing us build out. Is that something you planned to hit this week? My feeling is 99% of sound changes would easily be covered by use including 10-20 free ones.

I don't expect to implement sound preferences this week unless T12574 turns out to be much easier than I expect.

I agree that third-party modularity is not a very important feature, but I think it's a feature we should have eventually. I'd rather disable sound for now than implement a limited version of preferences which will make upgrading them later require migrations, make a future full-power implementation more involved, make T12562 more involved, make T12591 more involved, etc.

For me, playing the pop on a received message is already 99% of what I want, but I can understand that this might be annoying. This feature isn't important to me to have right away, I primarily just wanted to make sure it works properly and I'm satisfied that it's close enough that we could ship it later after doing more actual product work.

My concern here is we're trading hypothetical engineering resources in 3-5 years over shipping something thats 99% of what users want today. I can't see any set of outcomes where this is a net negative to Phacility. Either we'll _never_ have the resources to pursue the 10 year implementation, or we're so flush with engineers is a moot point.

Alright.

I can wire the "send" sound up if you aren't sure where to stick it. Ideally, I think have ConpherenceThreadManager._updateDOM play it (instead of the "received" sound) if the message comes from the viewer. I'm not sure if the viewer or message author are currently readily available. This will make the sound actually a "receive a message you authored"/"server confirms that it got a message you sent" sound, not exactly a "send" sound, but I think that's probably fine/good, and that the label "Send" is probably still reasonable for users.

Alternatively, you can silence the "receive" sound there and play the sound in the conpherence-pontificate behavior instead.

Or not silence the sound, so that sending a message plays "send, receive" (and perhaps add a separate setting for "receive a message I sent" I suppose, per @amckinley's request). If you do this, long "send" sounds may overlap with "receive" sounds. I believe we can adjust JX.Sound to serialize sounds in this case so that they play sequentially without overlap. Sending many messages quickly may have a similar issue. In that case, it would probably be better to stop the current sound and restart the play.

This revision is now accepted and ready to land.Apr 19 2017, 3:00 PM

I'm also fine building this in a more abstract way, I just don't know what that is. Do we have such examples or is ConpherenceSound the most flexible path?

I don't have a technical design for sound in mind and don't want to generate that design as part of this iteration (T12572) since I think it will take a significant amount of work.

Some examples of things I'd like to think more about are:

  • Can notifications play sounds? ("Play a sound when I receive a notification.")
  • Can different types of notifications play different sounds? ("Play a sound when I'm mentioned on a task.")
  • How do different sound sources synchronize when sounds try to play at the same time? ("Mention Notification" and "Chat Received" try to play at the same time.)
  • Can users ever upload custom sounds in the future? (Probably: yes?)
  • Can third-party code ever add sounds in the future? (Probably: yes?)
  • Can third-party code ever add new things which play sounds in the future? (Probably: yes?)
  • If notifications and Conpherence can play sounds, can other applications? (Broadcast alerts? Are we likely to build those as something separate?)
  • Can applications override default notification sounds? ("Calendar event reminders default to an alarm clock sound.")
    • If yes, how does that interact with other customizations?
  • Can applications play sounds without interacting with the realtime system? ("Drop Card" on a workboard, "Job Complete" on Bulk Task editor, "Build Succeeded/Failed" in Harbormaster)
  • How do users learn what sounds mean and discover how to customize them? What does the settings UI look like?
  • When choosing sounds, how do users preview what they sound like?
  • What global options exist to affect sound ("Temporarily mute all sound in a window")?
  • If these options exist, are these options sound-specific, or are they driven by use cases? (Disable sounds and notifications in fullscreen mode / "presentation mode").
  • What affordances should we make, if any, for users with accessibility issues related to sound?

Do we want a send sound at all? I'm not seeing similar features of modern chat applications. cc @amckinley

Slack is All, Mentions, None.

chad planned changes to this revision.Apr 19 2017, 5:33 PM
  • remove the ability to set a "send" sound
This revision is now accepted and ready to land.Apr 19 2017, 6:18 PM

I think this is generally more correct. Global preference is All, Mentions, None. Per room you set All or Mention sound. Gets rid of "send" as a configuable sound (which I think we can just drop). This should simply the JS as well which then just has to choose which sound to play (all or mention) and nothing on send.

This revision was automatically updated to reflect the committed changes.