Add Voice and Video call in room header #11444
Conversation
germain-gg
added
the
T-Task
src/hooks/room/useRoomCallStatus.ts
Outdated
|
|
||
| const hasGroupCall = useCall(room.roomId) !== null; | ||
|
|
||
| const functionalMembers = useRoomMembers(room); |
There was a problem hiding this comment.
What does functional mean here? Aren't his just all joined room members?
There was a problem hiding this comment.
The logic is ported over from
It seems that a functional member is a member that is not a bot according to this JSDoc comment,
all room members that are non-functional (bots etc.).
There was a problem hiding this comment.
useRoomMembers does something different here. The legacy code is also annoying, because it assigned the result of getJoinedNonFunctionalMembers to functionalMembers.
→ It should be nonFunctionalMembers (in the old code, out of scope here but also here in the new code)
| * @param room the room to track | ||
| * @returns the type of notification for this room | ||
| */ | ||
| export const useRoomThreadNotifications = (room: Room): NotificationColor => { |
There was a problem hiding this comment.
I wonder if logic like this (and the call status…) should not better go to a store or something else, that emits events. A hook could then just couple it to the component. If we start using these hooks in many places we will add quite a number of the same event listeners and code execution. This can become heavy, esp. in loops.
Probably not for this PR, but rather for general discussion.
There was a problem hiding this comment.
Definitely agree. Maybe worth you adding a discussion point in the web weekly.
There was a problem hiding this comment.
Tested and works now. 👍 Still the non-functional room member logic is different than before: #11444 (comment)
Not covered in this pull request
Checklist
This change is marked as an internal change (Task), so will not be included in the changelog.