VIDCS-3442: Refactor session context #140

cpettet · 2025-04-01T14:49:31Z

What is this PR doing?

Refactoring session context (eventually).

How should this be manually tested?

TBD

What are the relevant tickets?

A maintainer will add this ticket number.

Resolves VIDCS-3442

Checklist

[✅] Branch is based on develop (not main).
[ ] Resolves a Known Issue.
[ ] If yes, did you remove the item from the docs/KNOWN_ISSUES.md?
[ ] Resolves an item reported in Issues.
If yes, which issue? Issue Number?

cpettet · 2025-04-04T21:29:10Z

frontend/src/hooks/useEmoji.tsx

-  useEffect(() => {
-    session?.on('signal', emojiHandler);
-
-    return () => {
-      session?.off('signal', emojiHandler);
-    };
-  }, [emojiHandler, session]);


We handle this in the session context. Removing this silences the event listener console warning.

cpettet · 2025-04-04T21:34:26Z

frontend/src/Context/SessionProvider/session.tsx

-  changedStream: null | ChangedStreamType;
-  connections: null | Connection[];


These two weren't used anywhere.

cpettet · 2025-04-04T21:35:43Z

frontend/src/Context/SessionProvider/session.tsx

@@ -146,7 +118,8 @@ const MAX_PIN_COUNT = isMobile() ? MAX_PIN_COUNT_MOBILE : MAX_PIN_COUNT_DESKTOP;
 * @returns {SessionContextType} a context provider for a publisher preview
 */
 const SessionProvider = ({ children }: SessionProviderProps): ReactElement => {
-  const session = useRef<Session | null>(null);
+  const [, forceUpdate] = useState<boolean>(false); // NOSONAR


Takes the place of the setChangedStream state function. Without a re-render during a stream change, we don't get visual indicators for a subscriber muting themselves or the initials being displayed.

Would be good to add this context somewhere 🙏

cpettet · 2025-04-04T21:36:39Z

frontend/src/Context/SessionProvider/session.tsx

-  /**
-   * Handles the creation of a new connection. Adding it to the connection array
-   * @param {ConnectionEventType} e - The connection event object.
-   */
-  const handleConnectionCreated = (e: ConnectionEventType) => {
-    setConnections((prevConnections) => [...prevConnections, e.connection]);
-  };
-
-  /**
-   * Handles the destruction of a connection. Removing it from the connections array
-   * @param {ConnectionEventType} e - The connection event object.
   */
-  const handleConnectionDestroyed = (e: ConnectionEventType) => {
-    setConnections((prevConnections) =>
-      [...prevConnections].filter(
-        (connection) => connection.connectionId !== e.connection.connectionId
-      )
-    );


These two weren't used at all. It may have been leftovers from our call data function from the beginning.

cpettet · 2025-04-04T21:37:21Z

frontend/src/Context/SessionProvider/session.tsx

-  const subscribe = useCallback(
-    async (stream: Stream, localSession: Session, options: SubscriberProperties = {}) => {
-      const { streamId } = stream;
-      if (localSession) {
-        const finalOptions: SubscriberProperties = {
-          ...options,
-          insertMode: 'append',
-          width: '100%',
-          height: '100%',
-          preferredResolution: 'auto',
-          style: {
-            buttonDisplayMode: 'off',
-            nameDisplayMode: 'on',
-          },
-          insertDefaultUI: false,
-        };
-        const isScreenshare = stream.videoType === 'screen';
-        const subscriber = localSession.subscribe(stream, undefined, finalOptions);
-        subscriber.on('videoElementCreated', (event: VideoElementCreatedEvent) => {
-          const { element } = event;
-          const subscriberWrapper: SubscriberWrapper = {
-            element,
-            subscriber,
-            isScreenshare,
-            isPinned: false,
-            // subscriber.id is refers to the targetElement id and will be undefined when insertDefaultUI is false so we use streamId to track our subscriber
-            id: streamId,
-          };
-          // prepend new subscriber to beginning of array so that is is visible on joining
-          setSubscriberWrappers((previousSubscriberWrappers) =>
-            [subscriberWrapper, ...previousSubscriberWrappers].sort(
-              sortByDisplayPriority(activeSpeakerIdRef.current)
-            )
-          );
-        });
-
-        subscriber.on('destroyed', () => {
-          activeSpeakerTracker.current.onSubscriberDestroyed(streamId);
-          const isNotDestroyedStreamId = ({ id }: { id: string }) => streamId !== id;
-          setSubscriberWrappers((prevSubscriberWrappers) =>
-            prevSubscriberWrappers.filter(isNotDestroyedStreamId)
-          );
-        });
-
-        // Create moving average tracker and add handler for subscriber audioLevelUpdated event emitted periodically with subscriber audio volume
-        // See for reference: https://developer.vonage.com/en/video/guides/ui-customization/general-customization#adjusting-user-interface-based-on-audio-levels
-        const getMovingAverageAudioLevel = createMovingAvgAudioLevelTracker();
-        subscriber.on('audioLevelUpdated', ({ audioLevel }) => {
-          const { logMovingAvg } = getMovingAverageAudioLevel(audioLevel);
-          activeSpeakerTracker.current.onSubscriberAudioLevelUpdated({
-            subscriberId: streamId,
-            movingAvg: logMovingAvg,
-          });
-        });
-      }
-    },
-    []
-  );
-
-  // handle the subscription (Receiving media from remote parties)
-  const handleStreamCreated = (e: StreamCreatedEvent) => {
-    if (session.current) {
-      subscribe(e.stream, session.current);
-    }
-  };


Most of this was moved to the VonageVideoClient class. Some are bespoke handlers in this file, see below.

cpettet · 2025-04-04T21:38:40Z

frontend/src/Context/SessionProvider/session.tsx

-      await new Promise<string | undefined>((resolve, reject) => {
-        session.current?.connect(token, (err?: OTError) => {
-          if (err) {
-            // We ignore the following lint warning because we are rejecting with an OTError object.
-            reject(err); // NOSONAR
-          } else {
-            setConnected(true);
-            logOnConnect(apiKey, sessionId, session.current?.connection?.connectionId);
-            resolve(session.current?.sessionId);
-          }
-        });
-      });


Moved to VonageVideoClient file. We set connected state if we successfully connect, below.

can this error though?

Yep! The session.current.connect(credential); call can throw if there's an issue calling Session.connect

maikthomas

Looking good so far, good to separate all the nitty gritty logic around the SDK away from some of the react stuff by adding this more simple interface 🙌

frontend/src/utils/VonageVideoClient/vonageVideoClient.ts

maikthomas · 2025-04-23T17:42:35Z

frontend/src/Context/SessionProvider/session.tsx

-      await new Promise<string | undefined>((resolve, reject) => {
-        session.current?.connect(token, (err?: OTError) => {
-          if (err) {
-            // We ignore the following lint warning because we are rejecting with an OTError object.
-            reject(err); // NOSONAR
-          } else {
-            setConnected(true);
-            logOnConnect(apiKey, sessionId, session.current?.connection?.connectionId);
-            resolve(session.current?.sessionId);
-          }
-        });
-      });


can this error though?

maikthomas · 2025-04-23T17:43:09Z

frontend/src/Context/SessionProvider/session.tsx

@@ -146,7 +118,8 @@ const MAX_PIN_COUNT = isMobile() ? MAX_PIN_COUNT_MOBILE : MAX_PIN_COUNT_DESKTOP;
 * @returns {SessionContextType} a context provider for a publisher preview
 */
 const SessionProvider = ({ children }: SessionProviderProps): ReactElement => {
-  const session = useRef<Session | null>(null);
+  const [, forceUpdate] = useState<boolean>(false); // NOSONAR


Would be good to add this context somewhere 🙏

cpettet added skip-ci Skip CI do-not-review Do not review labels Apr 1, 2025

cpettet commented Apr 4, 2025

View reviewed changes

cpettet force-pushed the cpettet/vidcs-3442-refactor-session-context branch from 23694b0 to 89d7cd6 Compare April 15, 2025 20:59

maikthomas reviewed Apr 23, 2025

View reviewed changes

cpettet closed this Apr 30, 2025

cpettet force-pushed the cpettet/vidcs-3442-refactor-session-context branch from 177d926 to b575d9d Compare April 30, 2025 20:49

This was referenced Apr 30, 2025

VIDCS-3442: Refactor session context (old) #155

Closed

VIDCS-3442: Refactor session context #160

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

VIDCS-3442: Refactor session context #140

VIDCS-3442: Refactor session context #140

Uh oh!

cpettet commented Apr 1, 2025

Uh oh!

cpettet Apr 4, 2025

Uh oh!

cpettet Apr 4, 2025

Uh oh!

cpettet Apr 4, 2025

Uh oh!

maikthomas Apr 23, 2025

Uh oh!

cpettet Apr 4, 2025

Uh oh!

cpettet Apr 4, 2025 •

edited

Loading

Uh oh!

cpettet Apr 4, 2025

Uh oh!

maikthomas Apr 23, 2025

Uh oh!

cpettet Apr 29, 2025

Uh oh!

maikthomas left a comment

Uh oh!

Uh oh!

Uh oh!

maikthomas Apr 23, 2025

Uh oh!

maikthomas Apr 23, 2025

Uh oh!

Uh oh!

		changedStream: null \| ChangedStreamType;
		connections: null \| Connection[];

VIDCS-3442: Refactor session context #140

VIDCS-3442: Refactor session context #140

Uh oh!

Conversation

cpettet commented Apr 1, 2025

What is this PR doing?

How should this be manually tested?

What are the relevant tickets?

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpettet Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maikthomas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cpettet Apr 4, 2025 •

edited

Loading