[MM-51514] Implement Calls user settings #820
Conversation
streamer45
added
1: UX Review
|
@cpoile Given the above, feel free to give it a quick pass and leave thoughts, but understand that it may take a few iterations before a final review can be done. |
|
@matthewbirtch Would your team be able to help with this? It's a nice to have in my mind, but it would be great to have in v1. |
| const audioInputsRef = useRef<AudioDevicesSelectionHandle>(null); | ||
| const audioOutputsRef = useRef<AudioDevicesSelectionHandle>(null); |
There was a problem hiding this comment.
Is this necessary? I wonder if there's an easier way (like passing in a setState so that the child can save the current selection)?
There was a problem hiding this comment.
@cpoile I had to choose between those two approaches and didn't really like either. Pushing state from child to parent was equally (if not more) verbose as it would have duplicated more state or worse needed to pass getters as well. In the end I think the ref approach is okaish but if you have a substantial simplification in mind please let me know.
@abhijit-singh could you review this (if you can get it running locally)? It looks good at first glance but may need a few style tweaks on fonts and such. |
There was a problem hiding this comment.
Not able to get the plugin running locally with these and the related webapp changes but the screencap is helpful and makes it clear.
I did put together mockups for the ideal UI to match with what we currently have - Figma link. Probably makes sense to match the title and help text styling to maintain consistency.
I'll add the expanded states for other settings as well in the same file shortly.
|
@abhijit-singh Updated to match the new designs. |
|
Thanks @streamer45. Could you share a screencap or put this on a test server maybe? I still am not able to see the settings in my local env even after deploying the plugin from this PR. |
@abhijit-singh To test locally, you'll need to deploy the corresponding monorepo branch and rebuild the client. Also, the video above has been updated. |
|
@DHaussermann QA here is the same for mattermost/mattermost#27535 . Mostly want to make sure audio device selection for calls works as expected from the new Calls user settings (see video on how to access them). |
streamer45
removed
1: UX Review
There was a problem hiding this comment.
Tested and passed
- Ensured no issues with setting or saving new plugin preferences
- Ensured when setting are saved or modified these are the starting settings that are applied when a call is stared
- As a precaution, checked that changes to the settings are reflected in other webapp clients
- Tested using new settings on browser and desktop app
- Tested on mobile webview
- No applicable changes to mobile for this
LGTM!
DHaussermann
added
the
3: Reviews Complete
Summary
PR attempts to implement a dedicated Calls user setting section that can be also accessed from outside the context of a call and is part of the existing user settings modal. This will be quite useful for implementing many future features, such as custom shortcuts, experimental settings, and so on.
The existing designs (see below) were slightly out of date and incomplete, so I tried to do my best to come up with something that works and doesn't look completely inconsistent. That said, to avoid overcommitting, I focused on enabling this functionality in the first place, and now I am happy to iterate further with the help of a UX person.
This PR currently implements audio devices selection only. These are client specific so they don't use the Mattermost preference model. I am also not porting call notification settings (e.g. ringing) pending further discussion on where they should live.
Note This requires changes to Webapp to work, namely this branch.
Example Video
calls_settings.mp4
Designs (sort of)
https://www.figma.com/design/ngGHVBsmJzzeYbYkHvxVGJ/MM-51514-Calls%3A-Settings?node-id=1-7&t=VPMqAzHdUi18nMGw-0
Ticket Link
https://mattermost.atlassian.net/browse/MM-51514