-
Notifications
You must be signed in to change notification settings - Fork 42
OPENTOK-44484: replaced 'mute on entry' and 'disable mute on entry' buttons with a single toggle button #147
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small change in the error handling.
Also @v-kpheng isn't the wording on this kinda misleading?
Doesn't toggling 'mute on entry' mute everyone else already in the call here?
@maikthomas, MuteOnEntry comes from the spec. The side effect of toggling depends on its state. |
@v-kpheng Yea spec wise I see where this comes from, it's just UX wise we have a button that says 'Mute on Entry' that suddenly mutes everyone in the call 🤷 |
Please feel free to suggest a different approach :-) |
In terms of UI, given the way this works my personal preference would be
Another unrelated question: |
Hmmm...looks like we need a holistic approach. @cpettet, @austinwu13, can you guys sync up w/ @maikthomas. Let's come up with a more complete DoD please. The one I suggested isn't sufficient. Sorry. Once you guys have consensus on changes needed, then let's implement that please. Thanks! |
Sounds good to me, I think my previous answer is incomplete, better to meet and discuss 👍 |
What is this PR doing?
This PR is replacing the 'Mute on Entry' and 'Disable Mute on Entry' buttons with a single 'Toggle Mute on Entry' button that combines their functionality.
How should this be manually tested?
What are the relevant tickets?
OPENTOK-44484