fix: role change not happening from broadcaster to hls #3291
Conversation
amar-1995
requested review from
raviteja83,
eswarclynn and
KaustubhKumar05
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -115,7 +114,6 @@ export function Notifications() { | |||
| ToastManager.addToast({ | |||
| title: `You are now a ${notification.data.roleName}`, | |||
| }); | |||
| updateRoomLayoutForRole?.(notification.data.roleName); | |||
There was a problem hiding this comment.
why is this not working? is the role not correct here?
There was a problem hiding this comment.
In some scenarios, we are not getting useIsNotification or the notification is itself is empty. So this will never reach.
|
|
||
| export const RoleChangeNotification = () => { | ||
| const notification = useHMSNotifications(HMSNotificationTypes.ROLE_UPDATED); | ||
| const isNotificationDisabled = useIsNotificationDisabled(); |
There was a problem hiding this comment.
this is probably not required as it is handled in parent component
There was a problem hiding this comment.
I thought to completely remove it from the parent and handle everything in this file as it will be single point if anything happens when the roleChange notification arrives.
There was a problem hiding this comment.
no it should be at the parent as single point, not in each individual component
| const updateRoomLayoutForRole = useUpdateRoomLayout(); | ||
|
|
||
| useEffect(() => { | ||
| if (notification && !isNotificationDisabled) { |
There was a problem hiding this comment.
this condition is not required. we anyways get this only for local peer.
There was a problem hiding this comment.
do we get the notification for all peers here?
| title: `You are now a ${notification.data.roleName}`, | ||
| }); | ||
| } | ||
| if (notification && notification.data?.isLocal && notification.data?.roleName) { |
There was a problem hiding this comment.
isLocal check also should not be needed.
| const updateRoomLayoutForRole = useUpdateRoomLayout(); | ||
|
|
||
| useEffect(() => { | ||
| if (notification && !isNotificationDisabled) { |
There was a problem hiding this comment.
why will this work and not the earlier implementation? i don't see a clear difference.
No description provided.