feat: Add muted indicator to people list #6152
Conversation
src/utils/avatar-volume-utils.js
Outdated
| @@ -43,6 +43,6 @@ export function updateAvatarVolumesPref(displayName, gainMultiplier, muted) { | |||
| }); | |||
| } | |||
|
|
|||
| export function getAvatarVolumePref(displayName) { | |||
| return APP.store.state.preferences.avatarVoiceLevels?.[displayName]; | |||
| export function getAvatarVolumePref(playerSessionId) { | |||
There was a problem hiding this comment.
I believe the reason to use the display name instead of the session id is that the session id is ephemeral and different for each session while the display name is stored locally and only changes if the user updates it which tends to be not that often. This way we can restore user volumes across different session as long as the user doesn't update the display name.
It seems that at some point we stopped storing the displayName here:
https://github.com/mozilla/hubs/blob/9b2c6f5ba06431c2b503567335d32f710e90348a/src/components/player-info.js#L136-L143
We have two options, either store the displayName there so we can keep on accessing it through this.playerInfo.displayName or subscribe to the presence notifications in avatar-volume-controls like we do here and store the display name locally:
https://github.com/mozilla/hubs/blob/9b2c6f5ba06431c2b503567335d32f710e90348a/src/components/player-info.js#L99
Let me know if there is any other reason why we should switch to using the sessionId other than that.
There was a problem hiding this comment.
Thank you for the context! I'll have a look into adding displayName back in.
| @@ -80,6 +84,10 @@ AFRAME.registerComponent("avatar-volume-controls", { | |||
| const step = -calcGainStepDown(gainMultiplier); | |||
| gainMultiplier = THREE.MathUtils.clamp(gainMultiplier + step, 0, MAX_GAIN_MULTIPLIER); | |||
| this.updateGainMultiplier(gainMultiplier, true); | |||
| // If the gainMultiplier is lowered to 0, updated muted status in local storage | |||
There was a problem hiding this comment.
This is already calling updateGainMultiplier which already calls updateLocalMuted so I believe we can remove the updateLocalMuted call from here.
|
|
|
We talked about this PR at the Friday meetup, so here are some additional thoughts to what Hrithik posted (although I do like Hrithik's idea).
Anyway, having an indicator for personal muting is great! |
|
I really like the idea of having a private chat embedded in the people menu. |
I love this! I'll bring this to design to get the ok on the change. Thanks for the suggestion! :) |
Seconded 🚀 |
…om/mozilla/hubs into add-muted-indicator-to-people-list
displayNameundefinedinstore._preferences. Opted to store this under the users' session ids as these are accessible from the volume entities.themeUPDATED 7/4:
