Mute room feature #5731

keianhzo · 2022-09-26T00:37:56Z

This features add support for room owners to disable/enable voice and chat for the rest of the users in a room from the room settings panel. In case voice/chat is disable this will only affect to users that are not room owners.
¡

Related Hubs-Foundation/reticulum#629

netpro2k

Mostly lgtm. I focused mostly on just auditing hooks/effects and didn't dig very deep on the actual components or css. Made a few notes, biggest one I would change before shipping is moving the toggling of the actual mic state out of an effect since its pretty weird to have the UI dictating that.

netpro2k · 2022-10-19T09:11:26Z

src/react-components/room/useMicrophone.js

@@ -69,8 +71,16 @@ export function useMicrophone(scene) {
    [mediaDevicesManager]
  );

+  useEffect(() => {
+    if (canVoiceChat) {
+      mediaDevicesManager.startMicShare({});


Doesn't seem right to tie the actual behavior of mic toggling to a react hook. That would mean if no UI happens to be showing the uses useMicrophone this would not be toggled. Should just be handled outside of react land. Probably in MediaDevicesManager in the same spot we are toggling it off on permission changes?

This is mostly because the useMicrophone hook is used in MicSetupModal and the audio popover as that's where we want to conditionally start the mic based on if the voice permission is enabled. I don't have a strong opinion about this but I've moved it back to ui-root as a hook so we can still update the UI without relaying on the useMicrophone hook.

Having MicSetupModal have an effect that initially enables the mic makes sense, since that's part of what that screen does. But having it here means this ends up being what is re-enabling the mic in response to permission changes which seems wrong. MediaDeviceManager is what handles turning disabling it when permissions change, so I would think in the same spot it should also handle re-enabling it.

There are two different things. One is mic sharing and another one is mic enabling. Mic sharing just triggers the sharing process and ask for permission if needed. Enabling pauses/resumes the producer so we are effectively sending voice data.

In ui-root we are listening to voice permission updates to start sharing the mic during the different entry steps.
In the MediaDevicesManager we are listening to voice permission updates to pause the mic producer and we rely on the user to manually resume it as it might be tricky if the voice permission is re-enabled by a moderator and that forces the user's mic to open.

Ah right, that makes sense. We should still probably move this effect out of useMicrophone though, since if anything happens to be using it when a permission change happens its going to trigger, since it has canVoiceChat as a dependency, and useCan listens for permission change events.

netpro2k · 2022-10-19T09:15:06Z

src/react-components/room/RoomSettingsSidebarContainer.js

+
+      NotifiablePermissions.forEach(perm => {
+        if (APP.hub.member_permissions[perm] !== settings.member_permissions[perm]) {
+          hubChannel.sendMessage({


Is there a reason this isn't just part of hubChannel.updateHub(settings) like the other settings?

We are not sending messages for room permissions updates so we need to manually send these permissions as messages so we also have all the added message info that ret adds to correctly visualize them in the chat. Let me know if I'm missing something.

We send messages for other permission updates like "allow object spawning" and "allow drawing" no? Shouldn't this be the same?

Right now we don't send messages for room permissions although if we are doing it for chat/voice probably we should also do for the rest? Maybe that could be part of a follow-up PR as this is how it's in production right now.

Yeah seems worth changing on the server side so this can be removed as a special case.

src/components/avatar-audio-source.js

src/message-dispatch.js

netpro2k · 2022-10-19T09:35:09Z

src/react-components/presence-log.js

-    return <div className={classNames(presenceClasses)}>{this.props.entries.map(this.domForEntry)}</div>;
-  }
+  useEffect(() => {
+    setLogEntries(entries.map(domForEntry));


whats the goal with this effect and logEntries state vs just using entries directly?

Not sure if I get this one. Entries is the raw message data, logEntries are the DOM elements processed via map.

Right. But why have an effect and state instead of mapping in render like we were doing before. In either case we still have to render whenever a new entry comes in, so this just seems like an added layer of indirection.

Oh yep, right, yes this is redundant. Removing.

netpro2k · 2022-10-19T09:36:26Z

src/react-components/presence-log.js

-  }
+export function PresenceLog({ entries, preset, hubId, history, presences, onViewProfile, include, exclude, ...rest }) {
+  const intl = useIntl();
+  const isMod = useCan("kick_users");


We seem to be mixing moderator and owner in a few places. Here we are checking of kick_user permission as "mod" but in other spots related to this we are checking if they have the owner role. Is that intended?

I think in our case we want the room owners to be the ones with voice/chat permissions so I've introduced a useRole hook to check for owner permissions and add some consistency.

Should this be using useRole then?

I did that in 9602625 maybe you are referring to some other one? I think I've moved all of them.

ah must have been looking at old code.

src/react-components/room/ChatSidebarContainer.js

…sion

keianhzo requested a review from johnshaughnessy September 26, 2022 00:48

keianhzo had a problem deploying to hc-bio September 26, 2022 00:52 Failure

keianhzo mentioned this pull request Sep 26, 2022

Mute room Hubs-Foundation/reticulum#629

Merged

keianhzo had a problem deploying to hc-bio September 26, 2022 01:01 Failure

keianhzo had a problem deploying to hc-bio September 29, 2022 15:50 Failure

keianhzo had a problem deploying to hc-bio September 29, 2022 23:33 Failure

keianhzo had a problem deploying to hc-bio October 17, 2022 11:34 Failure

keianhzo marked this pull request as ready for review October 17, 2022 13:01

keianhzo had a problem deploying to hc-bio October 17, 2022 14:22 Failure

keianhzo had a problem deploying to hc-bio October 17, 2022 15:39 Failure

keianhzo had a problem deploying to hc-bio October 18, 2022 13:54 Failure

netpro2k reviewed Oct 19, 2022

View reviewed changes

keianhzo had a problem deploying to hc-bio October 19, 2022 10:41 Failure

keianhzo had a problem deploying to hc-bio October 19, 2022 15:37 Failure

keianhzo had a problem deploying to hc-bio October 20, 2022 07:51 Failure

keianhzo had a problem deploying to hc-bio October 20, 2022 08:45 Failure

keianhzo requested a review from netpro2k October 20, 2022 08:56

keianhzo had a problem deploying to hc-bio October 21, 2022 07:46 Failure

keianhzo added 12 commits October 25, 2022 15:36

Add room mute state

2599193

Mic setup modal voice chat status

db224cf

Room mute state updates

4146acf

Room mute UI updates

66a39c2

Linter updates

8b78f10

Chat off icon added

0b6107e

Remove unused code

8ac7f5b

Center icons

22431ba

Update space term to room

6d2d596

Update Mic setup modal title to account for voice disabled

f1766ae

Ask for mic permissions in useMicEnabled and account for voice permis…

3917b97

…sion

Add title to mic toolbar button when voice chat is disabled

81b02ae

keianhzo added 23 commits October 25, 2022 15:38

Update toolbar button styles

c5baee7

Notification container and style updates

3b4fdf4

check permission type

8ba20a6

Fix message param

9a849f2

Enforce voice chat permission on the user end

559fcc8

Update disabled icons color and cursor

5117551

Add title to disabled chat input buttons

d0b8153

Fix mobile notifications container width

54f2019

Fix React warnings

ffdd20b

Fix wrongly disabled Chat input

d66d2bf

Fix virtual gamepad z-index to be above in-room notifications

89a736c

Remove moderator for set when permission is revoked

f938006

Unify owner check by checking user presence roles

d7010fb

Fix useRole initialization

1d6b783

Show permission notifications to other moderators

85fbd0c

Allow commands when chat is disabled

da1a17f

Move mic start outside the useMicrophone hook

405cd38

Convert timestamp to int for comparison

907d496

Remove unused

bc9789b

Use hooks to correctly update the voice permission state in UIRoot

080fcd6

Replace kick_user for useRole for consistency

6dd9ff3

Remove entries update indirection layer

e4cbcab

Rebase updates

c59050b

keianhzo force-pushed the features/mute-room branch from e29fb94 to c59050b Compare October 25, 2022 13:56

keianhzo had a problem deploying to hc-bio October 25, 2022 14:04 Failure

keianhzo merged commit 1af56c4 into master Oct 25, 2022

keianhzo deleted the features/mute-room branch October 25, 2022 14:05

keianhzo added the whats new Include this PR on the "What's New" page label Oct 25, 2022

netpro2k mentioned this pull request Nov 1, 2022

Adding feature to block access and/or chat functionality (mozfest features) #5529

Closed

takahirox mentioned this pull request Dec 19, 2022

Camera control not functioning on mobile with virtual joysticks enabled #5859

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mute room feature #5731

Mute room feature #5731

keianhzo commented Sep 26, 2022 •

edited

Loading

netpro2k left a comment

netpro2k Oct 19, 2022

keianhzo Oct 19, 2022 •

edited

Loading

netpro2k Oct 20, 2022

keianhzo Oct 21, 2022

netpro2k Oct 21, 2022

netpro2k Oct 19, 2022

keianhzo Oct 19, 2022 •

edited

Loading

netpro2k Oct 20, 2022

keianhzo Oct 21, 2022 •

edited

Loading

netpro2k Oct 21, 2022

netpro2k Oct 19, 2022

keianhzo Oct 19, 2022

netpro2k Oct 20, 2022

keianhzo Oct 21, 2022

netpro2k Oct 19, 2022

keianhzo Oct 19, 2022

netpro2k Oct 20, 2022

keianhzo Oct 21, 2022 •

edited

Loading

netpro2k Oct 21, 2022

Mute room feature #5731

Mute room feature #5731

Conversation

keianhzo commented Sep 26, 2022 • edited Loading

netpro2k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keianhzo Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keianhzo Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keianhzo Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keianhzo Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keianhzo commented Sep 26, 2022 •

edited

Loading

keianhzo Oct 19, 2022 •

edited

Loading

keianhzo Oct 19, 2022 •

edited

Loading

keianhzo Oct 21, 2022 •

edited

Loading

keianhzo Oct 21, 2022 •

edited

Loading