From fa9009e4a59a76ab7560b6f533ef94501ad3f333 Mon Sep 17 00:00:00 2001 From: Kimberlee Johnson Date: Mon, 10 Jan 2022 11:54:46 -0800 Subject: [PATCH 1/3] Temporarily removing use of useActiveSpeaker hook --- .../ParticipantBar/ParticipantBar.js | 59 ++++++++++--------- .../shared/contexts/ParticipantsProvider.js | 13 ++-- 2 files changed, 37 insertions(+), 35 deletions(-) diff --git a/custom/shared/components/ParticipantBar/ParticipantBar.js b/custom/shared/components/ParticipantBar/ParticipantBar.js index 6b2e27e..dbf59d1 100644 --- a/custom/shared/components/ParticipantBar/ParticipantBar.js +++ b/custom/shared/components/ParticipantBar/ParticipantBar.js @@ -12,7 +12,6 @@ import { useParticipants } from '@custom/shared/contexts/ParticipantsProvider'; import { useTracks } from '@custom/shared/contexts/TracksProvider'; import { useUIState } from '@custom/shared/contexts/UIStateProvider'; import { isLocalId } from '@custom/shared/contexts/participantsState'; -import { useActiveSpeaker } from '@custom/shared/hooks/useActiveSpeaker'; import { useCamSubscriptions } from '@custom/shared/hooks/useCamSubscriptions'; import { useResize } from '@custom/shared/hooks/useResize'; import { useScrollbarWidth } from '@custom/shared/hooks/useScrollbarWidth'; @@ -40,18 +39,21 @@ export const ParticipantBar = ({ width, }) => { const { networkState } = useCallState(); - const { currentSpeaker, screens, swapParticipantPosition } = - useParticipants(); + const { + currentSpeaker, + screens, + swapParticipantPosition, + } = useParticipants(); const { maxCamSubscriptions } = useTracks(); const { pinnedId, showParticipantsBar } = useUIState(); - const itemHeight = useMemo( - () => width / aspectRatio + GAP, - [aspectRatio, width] - ); - const paddingTop = useMemo( - () => itemHeight * fixed.length, - [fixed, itemHeight] - ); + const itemHeight = useMemo(() => width / aspectRatio + GAP, [ + aspectRatio, + width, + ]); + const paddingTop = useMemo(() => itemHeight * fixed.length, [ + fixed, + itemHeight, + ]); const scrollTop = useRef(0); const spaceBefore = useRef(null); const spaceAfter = useRef(null); @@ -61,14 +63,13 @@ export const ParticipantBar = ({ const [isSidebarScrollable, setIsSidebarScrollable] = useState(false); const blockScrolling = useBlockScrolling(scrollRef); const scrollbarWidth = useScrollbarWidth(); - const activeSpeakerId = useActiveSpeaker(); - const hasScreenshares = useMemo(() => screens.length > 0, [screens]); const othersCount = useMemo(() => others.length, [others]); - const visibleOthers = useMemo( - () => others.slice(range[0], range[1]), - [others, range] - ); + const visibleOthers = useMemo(() => others.slice(range[0], range[1]), [ + others, + range, + ]); + const currentSpeakerId = useMemo(() => currentSpeaker?.id, [currentSpeaker]); /** * Store other ids as string to reduce amount of running useEffects below. @@ -96,7 +97,7 @@ export const ParticipantBar = ({ if (!showParticipantsBar) { setCamSubscriptions({ subscribedIds: [ - currentSpeaker?.id, + currentSpeakerId, pinnedId, ...fixedRemote.map((p) => p.id), ], @@ -117,8 +118,8 @@ export const ParticipantBar = ({ const min = Math.max(0, r[0] - buffer); const max = Math.min(otherIds.length, r[1] + buffer); const ids = otherIds.slice(min, max); - if (!ids.includes(currentSpeaker?.id) && !isLocalId(currentSpeaker?.id)) { - ids.push(currentSpeaker?.id); + if (!ids.includes(currentSpeakerId) && !isLocalId(currentSpeakerId)) { + ids.push(currentSpeakerId); } // Calculate paused participant ids by determining their tile position const subscribedIds = [...fixedRemote.map((p) => p.id), ...ids]; @@ -126,7 +127,7 @@ export const ParticipantBar = ({ // ignore unrendered ids, they'll be unsubscribed instead if (!ids.includes(id)) return false; // ignore current speaker, it should never be paused - if (id === currentSpeaker?.id) return false; + if (id === currentSpeakerId) return false; const top = i * itemHeight; const fixedHeight = fixed.length * itemHeight; const visibleScrollHeight = scrollEl.clientHeight - fixedHeight; @@ -143,7 +144,7 @@ export const ParticipantBar = ({ }); }, [ - currentSpeaker?.id, + currentSpeakerId, fixed, itemHeight, maxCamSubscriptions, @@ -244,21 +245,21 @@ export const ParticipantBar = ({ // Active speaker not rendered at all, promote immediately if ( - visibleOthers.every((p) => p.id !== activeSpeakerId) && - !isLocalId(activeSpeakerId) + visibleOthers.every((p) => p.id !== currentSpeakerId) && + !isLocalId(currentSpeakerId) ) { - swapParticipantPosition(fixedOther.id, activeSpeakerId); + swapParticipantPosition(fixedOther.id, currentSpeakerId); return false; } const activeTile = othersRef.current?.querySelector( - `[id="${activeSpeakerId}"]` + `[id="${currentSpeakerId}"]` ); // Ignore when active speaker is not within "others" if (!activeTile) return false; // Ignore when active speaker is already pinned - if (activeSpeakerId === pinnedId) return false; + if (currentSpeakerId === pinnedId) return false; const { height: tileHeight } = activeTile.getBoundingClientRect(); const othersVisibleHeight = @@ -273,7 +274,7 @@ export const ParticipantBar = ({ ) return false; - return swapParticipantPosition(fixedOther.id, activeSpeakerId); + return swapParticipantPosition(fixedOther.id, currentSpeakerId); }; maybePromoteActiveSpeaker(); const throttledHandler = debounce(maybePromoteActiveSpeaker, 100); @@ -283,7 +284,7 @@ export const ParticipantBar = ({ scrollEl?.removeEventListener('scroll', throttledHandler); }; }, [ - activeSpeakerId, + currentSpeakerId, fixed, hasScreenshares, pinnedId, diff --git a/custom/shared/contexts/ParticipantsProvider.js b/custom/shared/contexts/ParticipantsProvider.js index f6c9428..b214f84 100644 --- a/custom/shared/contexts/ParticipantsProvider.js +++ b/custom/shared/contexts/ParticipantsProvider.js @@ -43,8 +43,10 @@ export const ParticipantsProvider = ({ children }) => { initialParticipantsState ); const { viewMode } = useUIState(); - const [participantMarkedForRemoval, setParticipantMarkedForRemoval] = - useState(null); + const [ + participantMarkedForRemoval, + setParticipantMarkedForRemoval, + ] = useState(null); /** * ALL participants (incl. shared screens) in a convenient array @@ -95,10 +97,9 @@ export const ParticipantsProvider = ({ children }) => { [allParticipants] ); - const isOwner = useMemo( - () => !!localParticipant?.isOwner, - [localParticipant] - ); + const isOwner = useMemo(() => !!localParticipant?.isOwner, [ + localParticipant, + ]); /** * The participant who should be rendered prominently right now From 63c4d253a29bffb44e6e20d9bc19108cd88f1d5e Mon Sep 17 00:00:00 2001 From: Kimberlee Johnson Date: Tue, 11 Jan 2022 11:22:27 -0800 Subject: [PATCH 2/3] Simplified currentSpeaker calculation, added curly braces, storeed event id value once --- .../ParticipantBar/ParticipantBar.js | 6 ++-- .../shared/contexts/ParticipantsProvider.js | 28 ++++++++++++++----- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/custom/shared/components/ParticipantBar/ParticipantBar.js b/custom/shared/components/ParticipantBar/ParticipantBar.js index dbf59d1..7552fdb 100644 --- a/custom/shared/components/ParticipantBar/ParticipantBar.js +++ b/custom/shared/components/ParticipantBar/ParticipantBar.js @@ -240,8 +240,9 @@ export const ParticipantBar = ({ const maybePromoteActiveSpeaker = () => { const fixedOther = fixed.find((f) => !f.isLocal); // Ignore when speaker is already at first position or component unmounted - if (!fixedOther || fixedOther?.id === activeSpeakerId || !scrollEl) + if (!fixedOther || fixedOther?.id === activeSpeakerId || !scrollEl) { return false; + } // Active speaker not rendered at all, promote immediately if ( @@ -271,8 +272,9 @@ export const ParticipantBar = ({ if ( scrolledOffsetTop + tileHeight / 2 < othersVisibleHeight && scrolledOffsetTop > -tileHeight / 2 - ) + ) { return false; + } return swapParticipantPosition(fixedOther.id, currentSpeakerId); }; diff --git a/custom/shared/contexts/ParticipantsProvider.js b/custom/shared/contexts/ParticipantsProvider.js index b214f84..64843f6 100644 --- a/custom/shared/contexts/ParticipantsProvider.js +++ b/custom/shared/contexts/ParticipantsProvider.js @@ -106,21 +106,28 @@ export const ParticipantsProvider = ({ children }) => { */ const currentSpeaker = useMemo(() => { /** - * Ensure activeParticipant is still present in the call. + * If the activeParticipant is still in the call, return the activeParticipant. * The activeParticipant only updates to a new active participant so * if everyone else is muted when AP leaves, the value will be stale. */ const isPresent = participants.some((p) => p?.id === activeParticipant?.id); + if (isPresent) { + return activeParticipant; + } + /** + * If the activeParticipant has left, calculate the remaining displayable participants + */ const displayableParticipants = participants.filter((p) => !p?.isLocal); + /** + * If nobody ever unmuted, return the first participant with a camera on + * Or, if all cams are off, return the first remote participant + */ if ( - !isPresent && displayableParticipants.length > 0 && displayableParticipants.every((p) => p.isMicMuted && !p.lastActiveDate) ) { - // Return first cam on participant in case everybody is muted and nobody ever talked - // or first remote participant, in case everybody's cam is muted, too. return ( displayableParticipants.find((p) => !p.isCamMuted) ?? displayableParticipants?.[0] @@ -131,7 +138,13 @@ export const ParticipantsProvider = ({ children }) => { .sort((a, b) => sortByKey(a, b, 'lastActiveDate')) .reverse(); - return isPresent ? activeParticipant : sorted?.[0] ?? localParticipant; + const lastActiveSpeaker = sorted?.[0]; + + if (lastActiveSpeaker) { + return lastActiveSpeaker; + } + + return localParticipant; }, [activeParticipant, localParticipant, participants]); /** @@ -297,11 +310,12 @@ export const ParticipantsProvider = ({ children }) => { * Our UX doesn't ever highlight the local user as the active speaker. */ const localId = callObject.participants().local.session_id; - if (localId === activeSpeaker?.peerId) return; + const activeSpeakerId = activeSpeaker?.peerId; + if (localId === activeSpeakerId) return; dispatch({ type: ACTIVE_SPEAKER, - id: activeSpeaker?.peerId, + id: activeSpeakerId, }); }; callObject.on('active-speaker-change', handleActiveSpeakerChange); From a07e17a31f80048616bf8f32e3edc41d12ae03c4 Mon Sep 17 00:00:00 2001 From: Kimberlee Johnson Date: Tue, 11 Jan 2022 14:25:30 -0800 Subject: [PATCH 3/3] Shortened return statement for currentSpeaker memo --- custom/shared/contexts/ParticipantsProvider.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/custom/shared/contexts/ParticipantsProvider.js b/custom/shared/contexts/ParticipantsProvider.js index 64843f6..50db416 100644 --- a/custom/shared/contexts/ParticipantsProvider.js +++ b/custom/shared/contexts/ParticipantsProvider.js @@ -140,11 +140,7 @@ export const ParticipantsProvider = ({ children }) => { const lastActiveSpeaker = sorted?.[0]; - if (lastActiveSpeaker) { - return lastActiveSpeaker; - } - - return localParticipant; + return lastActiveSpeaker || localParticipant; }, [activeParticipant, localParticipant, participants]); /**