Fix listeners for chat events of AzureCommunicationCallWithChatAdapter when the threadId changes#5764
Conversation
…hen the thread id is resolved late or is changed when moving to and from breakout rooms
This reverts commit 5e1c043.
mgamis-msft
requested review from
JamesBurnside,
JoshuaLai,
Leah-Xia-Microsoft,
alkwa-msft,
carocao-msft,
dmceachernmsft,
edwardlee-msft,
emlynmac,
jimchou-dev,
jpeng-ms,
palatter,
prabhjot-msft,
vhuseinova-msft and
waynemo
|
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-bpklywqfle.chromatic.com/ |
@azure/communication-react jest test coverage for stable.
|
@azure/communication-react jest test coverage for beta.
|
|
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-mguzcdmxpf.chromatic.com/ |
| private originCallChatAdapter: ChatAdapter | undefined; | ||
| private breakoutRoomChatAdapter: ChatAdapter | undefined; | ||
|
|
||
| private chatEventListeners: Map<string, Listener[]> = new Map(); |
There was a problem hiding this comment.
Add a detailed comment here why the need to track and store all chat event listeners manually
| private originCallChatAdapter: ChatAdapter | undefined; | ||
| private breakoutRoomChatAdapter: ChatAdapter | undefined; | ||
|
|
||
| private chatEventListeners: Map<string, Listener[]> = new Map(); |
There was a problem hiding this comment.
You can use a Set instead of array to ensure no duplicates and syntactically inform the reader that it is designed for uniqueness
There was a problem hiding this comment.
Your add/remove functions won't then need to recreate the array, but can just use set.add and set.remove
| } | ||
|
|
||
| private updateChatEventListeners(chatAdapter: ChatAdapter): void { | ||
| const messageReceivedListeners = this.getListeners('messageReceived'); |
There was a problem hiding this comment.
if someone adds a new event it will be easy to miss adding it here, can you instead more blindly iterate over the listeners?
e.g. (untested)
for (const [eventName, listeners] of this.chatEventListeners) {
for (const listener of listeners) {
this.chatAdapter?.off('eventName', listener);
chatAdapter.on('eventName', listener);
}
}
(might have to use as any to satisfy typescript
| private originCallChatAdapter: ChatAdapter | undefined; | ||
| private breakoutRoomChatAdapter: ChatAdapter | undefined; | ||
|
|
||
| private chatEventListeners: Map<string, Listener[]> = new Map(); |
There was a problem hiding this comment.
Cleanup (empty) this variable in the dispose function
|
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-lfekxquate.chromatic.com/ |
|
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-jiksvluaqw.chromatic.com/ |
CallWithChat bundle size is not changed.
|
Calling bundle size is not changed.
|
Chat bundle size is not changed.
|
|
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-xwdzuahnct.chromatic.com/ |
mgamis-msft
deleted the
mgamis/fix-chat-event-listening-in-call-with-chat-adapter
branch
4 participants
What
Add map of listeners for chat events so that old chat adapter listeners can be propagated to newly created chat adapters. This will fix listeners for chat events when:
Why
Resolves one of the issue from #5755 regarding not being able chat event listeners not working when joining a breakout room
How Tested
Tested with code from reverted commit on this branch
Process & policy checklist
Is this a breaking change?