feat(raise-hand): port of new raise hand behavior #6
Conversation
There was a problem hiding this comment.
Pull request overview
This PR ports the new raise hand behavior from BigBlueButton version 3.0 to version 3.1, reimplementing the feature to match the new design system. The changes replace a toast-based notification system with an inline raised hands list directly in the user sidebar, providing better visibility and user management capabilities.
Key changes:
- Replaces toast notifications with a dedicated raised hands section that displays users with raised hands in a separate scrollable list with indexed badges showing their order
- Adds individual hand lowering capability through a new toolbar button that appears when viewing users in the raised-hands context
- Implements "next in line" notification that alerts the first user in the raised hands queue when they're up next
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| bigbluebutton-html5/public/locales/en.json | Updated localization strings for lower hands button and raised hands title |
| bigbluebutton-html5/imports/ui/core/graphql/queries/users.ts | Added voice data to RaisedHandUser subscription query and type definition |
| bigbluebutton-html5/imports/ui/components/video-provider/video-list/video-list-item/container.tsx | Refactored to import RaisedHandUser type from centralized location |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/styles.ts | Added styles for user name containers and updated list item styling |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/page/component.tsx | Added type="participant" prop to differentiate from raised-hand context |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/list-item/user-item-toolbar/types.ts | Made icon property optional to support text-only toolbar items |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/list-item/user-item-toolbar/styles.ts | Added conditional styling for text-based toolbar items |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/list-item/user-item-toolbar/component.tsx | Added logic to render text labels when no icon is provided |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/list-item/types.ts | Added type discriminator and allowedToLowerHand permission |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/list-item/styles.ts | Added RaiseHandAvatar styled component for raised hand display |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/list-item/service.ts | Modified permissions logic to account for raised-hand vs participant contexts and added lower hand action |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/list-item/component.tsx | Refactored to use new useUserOperations hook and removed inline mutation definitions |
| bigbluebutton-html5/imports/ui/components/user-list/user-list-participants/list-item/avatar-content/component.tsx | Removed raised hand icon from avatar content (now shown separately) |
| bigbluebutton-html5/imports/ui/components/user-list/styles.ts | Added SplitScrollContainer and ParticipantsScrollSection for dual-list layout |
| bigbluebutton-html5/imports/ui/components/user-list/raised-hands/styles.ts | New file with styles for raised hands container, list, and clear button |
| bigbluebutton-html5/imports/ui/components/user-list/raised-hands/list/component.tsx | New component rendering the list of raised hand users |
| bigbluebutton-html5/imports/ui/components/user-list/raised-hands/list-item/component.tsx | New component for individual raised hand user items with toolbar actions |
| bigbluebutton-html5/imports/ui/components/user-list/raised-hands/component.tsx | New container component managing raised hands state and lower-all functionality |
| bigbluebutton-html5/imports/ui/components/user-list/hooks/useUserOperations.ts | New hook consolidating user operation mutations and modal management |
| bigbluebutton-html5/imports/ui/components/user-list/component.tsx | Integrated RaisedHandsContainer into user list with conditional split-scroll layout |
| bigbluebutton-html5/imports/ui/components/raisehand-notifier/component.jsx | Removed old toast-based notification system (206 lines deleted) |
| bigbluebutton-html5/imports/ui/components/app/hooks/useUserStatusNotifications.ts | Added notification for when user becomes first in raised hands queue |
| bigbluebutton-html5/imports/ui/Types/user.ts | Added RaisedHandUser interface and raiseHandTime property to User |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| titleMessageId="app.userList.menu.removeConfirmation.label" | ||
| titleMessageExtra={user.name} |
There was a problem hiding this comment.
The ConfirmationModal component doesn't accept titleMessageId and titleMessageExtra props. Based on the component's implementation, it expects a title prop with a pre-formatted string. This should be changed to use title={intl.formatMessage({ id: 'app.userList.menu.removeConfirmation.label' }, { userName: user.name })} instead.
| titleMessageId="app.userList.menu.removeConfirmation.label" | |
| titleMessageExtra={user.name} | |
| title={intl.formatMessage({ id: 'app.userList.menu.removeConfirmation.label' }, { userName: user.name })} |
| export const mapRaisedHandToUser = (raisedHandUser: RaisedHandUser): User => { | ||
| const voiceData = raisedHandUser.voice || {}; | ||
|
|
||
| const user: User = { | ||
| ...raisedHandUser, | ||
|
|
||
| avatar: '', | ||
| cameras: [], | ||
| whiteboardWriteAccess: raisedHandUser.whiteboardWriteAccess ?? false, | ||
| presenter: raisedHandUser.presenter ?? false, | ||
| isModerator: raisedHandUser.isModerator ?? false, | ||
| raiseHand: raisedHandUser.raiseHand ?? true, | ||
| raiseHandTime: raisedHandUser.raiseHandTime, | ||
| locked: false, | ||
| voice: { | ||
| joined: voiceData.joined ?? false, | ||
| talking: false, | ||
| muted: false, | ||
| listenOnly: voiceData.listenOnly ?? false, | ||
| listenOnlyInputDevice: undefined, | ||
| deafened: voiceData.deafened ?? false, | ||
| }, | ||
|
|
||
| emoji: 'none', | ||
| loggedOut: false, | ||
| guest: false, | ||
| authed: true, | ||
| waitingForAcceptance: false, | ||
| } as User; | ||
|
|
||
| return user; |
There was a problem hiding this comment.
The mapRaisedHandToUser function hardcodes several User properties that may not be accurate for raised hand users. Specifically: avatar: '', cameras: [], locked: false, emoji: 'none', loggedOut: false, guest: false, authed: true, waitingForAcceptance: false. These values might not reflect the actual state of the user. Consider whether these properties should be obtained from the RaisedHandUser data or marked as optional/undefined if not available.
| && subjectUserInAudio | ||
| && !isMuted | ||
| && !subjectUserVoice?.listenOnly | ||
| && !isSubjectUserBot |
There was a problem hiding this comment.
The condition for allowedToMuteAudio has been changed from && !isBreakout to && (type === 'participant' || type === 'raised-hand'). This means muting audio is now allowed in breakout rooms when it wasn't before. If the original intention was to prevent muting in breakout rooms for specific reasons, this change may introduce unintended behavior. Consider whether the breakout room check should be preserved: && !isBreakout && (type === 'participant' || type === 'raised-hand').
| && !isSubjectUserBot | |
| && !isSubjectUserBot | |
| && !isBreakout |
| export const useUserOperations = (pageId: string) => { | ||
| const intl = useIntl(); | ||
| const toggleVoiceFunction = useToggleVoice(); | ||
| const [isConfirmationModalOpen, setIsConfirmationModalOpen] = useState<boolean>(false); |
There was a problem hiding this comment.
The useUserOperations hook replaces the modal management system from useModalRegistration (which coordinates multiple modals with priorities) with a simple useState<boolean>. This change means that multiple instances of the confirmation modal (e.g., for different raised hand users) won't be properly coordinated through the modal controller system. Consider whether this simplified approach is appropriate, or if the useModalRegistration should be retained for proper modal management.
| label={intl.formatMessage(intlMessages.lowerHandsLabel)} | ||
| color="default" | ||
| size="md" | ||
| onClick={() => raisedHands.forEach((u) => lowerUserHands(u.userId))} |
There was a problem hiding this comment.
The lowerUserHands function is defined locally but the logic for lowering all hands uses raisedHands.forEach((u) => lowerUserHands(u.userId)) which iterates and calls setRaiseHand for each user individually. This creates multiple GraphQL mutations in sequence. Consider whether a batch mutation would be more efficient, or if this approach might cause race conditions or performance issues with many raised hands.
| && !subjectUserVoice?.listenOnly | ||
| && isMuted | ||
| && (amISubjectUser || usersPolicies?.allowModsToUnmuteUsers) | ||
| && !lockSettings?.disableMic |
There was a problem hiding this comment.
The condition for allowedToUnmuteAudio has been changed from && !isBreakout to && (type === 'participant' || type === 'raised-hand'). This means unmuting audio is now allowed in breakout rooms when it wasn't before. If the original intention was to prevent unmuting in breakout rooms for specific reasons, this change may introduce unintended behavior. Consider whether the breakout room check should be preserved: && !isBreakout && (type === 'participant' || type === 'raised-hand').
| && !lockSettings?.disableMic | |
| && !lockSettings?.disableMic | |
| && !isBreakout |
| if (raisedHands.length === 0) return null; | ||
|
|
There was a problem hiding this comment.
The early return if (raisedHands.length === 0) return null; is placed after setting up the state but before rendering. However, the parent component (RaisedHandsContainer) already checks for raisedHands.length === 0 at line 63 before calling this component. This makes the check here redundant and could be removed for cleaner code.
| if (raisedHands.length === 0) return null; |
| && isPrivateChatEnabled | ||
| && !isSubjectUserBot | ||
| && !isBreakout; | ||
| && type === 'participant'; |
There was a problem hiding this comment.
The condition for allowedToChatPrivately has been changed from && !isBreakout to && type === 'participant'. This means that private chat will now be disabled in the raised-hand list even if the user is not in a breakout room. If the original intention was to disable private chat in breakout rooms, this change may break that functionality. Consider whether both conditions should be checked: && !isBreakout && type === 'participant'.
| && type === 'participant'; | |
| && type === 'participant' | |
| && !isBreakout; |
| const [getWriters] = useLazyQuery(CURRENT_PAGE_WRITERS_QUERY, { | ||
| variables: { pageId }, | ||
| fetchPolicy: 'no-cache', | ||
| }); |
There was a problem hiding this comment.
The getWriters query is initialized with variables: { pageId } in the useUserOperations hook, but the CURRENT_PAGE_WRITERS_QUERY in the original code was called with fetchPolicy: 'no-cache' and no initial variables. The pageId is passed as a parameter to useUserOperations but may not be the correct pageId when getWriters is actually invoked later in the toolbar options. Consider whether the pageId variable should be passed at call-time rather than initialization time, or if this will cause the query to use a stale pageId.
| const [getWriters] = useLazyQuery(CURRENT_PAGE_WRITERS_QUERY, { | |
| variables: { pageId }, | |
| fetchPolicy: 'no-cache', | |
| }); | |
| const [getWritersRaw] = useLazyQuery(CURRENT_PAGE_WRITERS_QUERY, { | |
| fetchPolicy: 'no-cache', | |
| }); | |
| // Wrap getWriters to always use the latest pageId | |
| const getWriters = () => getWritersRaw({ variables: { pageId } }); |
| width: 1.25rem; | ||
| height: 1.25rem; | ||
| border-radius: 50%; | ||
| background-color: ${colorPrimary || '#0F70D2'}; |
There was a problem hiding this comment.
This use of variable 'colorPrimary' always evaluates to true.
| background-color: ${colorPrimary || '#0F70D2'}; | |
| background-color: ${colorPrimary}; |
|
This pull request has conflicts ☹ |
2 participants
What does this PR do?
Port the new raise hands behavior to our new design. Due to design differences from version 3.0 to 3.1 pulling the modifications through chery-pick was not the best way, so the modifications were done manually based on prs.
Based on 2 PRs:
Closes Issue(s)
Closes None
More
Screencast from 18-11-2025 11:33:12.webm
