Browse Source

Thumbnail reordering and participant pane enhancements.

* fix(participant-pane) Use the sorted participant list from redux instead of sorting it on every render making it better performant. Match the participant order with that of the order in the filmstrip. Also move the participants with raised hand to the top of the list.

* ref(filmstrip) Move enableThumbnailReordering flag to testing section.

* fix(participants) Add new selectors for getting sorted participants.
master
Jaya Allamsetty 4 years ago
parent
commit
40a485ec6c
No account linked to committer's email address

+ 4
- 0
config.js View File

41
         // issues related to insertable streams.
41
         // issues related to insertable streams.
42
         // disableE2EE: false,
42
         // disableE2EE: false,
43
 
43
 
44
+        // Enables/disables thumbnail reordering in the filmstrip. It is enabled by default unless explicitly
45
+        // disabled by the below option.
46
+        // enableThumbnailReordering: true,
47
+
44
         // P2P test mode disables automatic switching to P2P when there are 2
48
         // P2P test mode disables automatic switching to P2P when there are 2
45
         // participants in the conference.
49
         // participants in the conference.
46
         p2pTestMode: false
50
         p2pTestMode: false

+ 42
- 53
react/features/base/participants/functions.js View File

273
     return toState(stateful)['features/base/participants'].remote;
273
     return toState(stateful)['features/base/participants'].remote;
274
 }
274
 }
275
 
275
 
276
+/**
277
+ * Selectors for the getting the remote participants in the order that they are displayed in the filmstrip.
278
+ *
279
+@param {(Function|Object)} stateful - The (whole) redux state, or redux's {@code getState} function to be used to
280
+ * retrieve the state features/filmstrip.
281
+ * @returns {Array<string>}
282
+ */
283
+export function getRemoteParticipantsSorted(stateful: Object | Function) {
284
+    return toState(stateful)['features/filmstrip'].remoteParticipants;
285
+}
286
+
276
 /**
287
 /**
277
  * Returns the participant which has its pinned state set to truthy.
288
  * Returns the participant which has its pinned state set to truthy.
278
  *
289
  *
444
     return undefined;
455
     return undefined;
445
 }
456
 }
446
 
457
 
447
-
448
 /**
458
 /**
449
- * Selector for retrieving sorted participants by display name.
459
+ * Selector for retrieving ids of participants in the order that they are displayed in the filmstrip (with the
460
+ * exception of participants with raised hand). The participants are reordered as follows.
461
+ * 1. Local participant.
462
+ * 2. Participants with raised hand.
463
+ * 3. Participants with screenshare sorted alphabetically by their display name.
464
+ * 4. Shared video participants.
465
+ * 5. Recent speakers sorted alphabetically by their display name.
466
+ * 6. Rest of the participants sorted alphabetically by their display name.
450
  *
467
  *
451
  * @param {(Function|Object)} stateful - The (whole) redux state, or redux's
468
  * @param {(Function|Object)} stateful - The (whole) redux state, or redux's
452
- * {@code getState} function to be used to retrieve the state
453
- * features/base/participants.
454
- * @returns {Array<Object>}
469
+ * {@code getState} function to be used to retrieve the state features/base/participants.
470
+ * @returns {Array<string>}
455
  */
471
  */
456
-export function getSortedParticipants(stateful: Object | Function) {
457
-    const localParticipant = getLocalParticipant(stateful);
458
-    const remoteParticipants = getRemoteParticipants(stateful);
459
-    const raisedHandParticipantIds = getRaiseHandsQueue(stateful);
460
-
461
-    const items = [];
462
-    const dominantSpeaker = getDominantSpeakerParticipant(stateful);
463
-    const raisedHandParticipants = [];
464
-
465
-    raisedHandParticipantIds
466
-        .map(id => remoteParticipants.get(id) || localParticipant)
467
-        .forEach(p => {
468
-            if (p !== dominantSpeaker) {
469
-                raisedHandParticipants.push(p);
470
-            }
471
-        });
472
-
473
-    remoteParticipants.forEach(p => {
474
-        if (p !== dominantSpeaker && !raisedHandParticipantIds.find(id => p.id === id)) {
475
-            items.push(p);
472
+export function getSortedParticipantIds(stateful: Object | Function): Array<string> {
473
+    const { id } = getLocalParticipant(stateful);
474
+    const remoteParticipants = getRemoteParticipantsSorted(stateful);
475
+    const reorderedParticipants = new Set(remoteParticipants);
476
+    const raisedHandParticipants = getRaiseHandsQueue(stateful);
477
+    const remoteRaisedHandParticipants = new Set(raisedHandParticipants || []);
478
+
479
+    for (const participant of remoteRaisedHandParticipants.keys()) {
480
+        // Avoid duplicates.
481
+        if (reorderedParticipants.has(participant)) {
482
+            reorderedParticipants.delete(participant);
483
+        } else {
484
+            remoteRaisedHandParticipants.delete(participant);
476
         }
485
         }
477
-    });
478
-
479
-    if (!raisedHandParticipantIds.find(id => localParticipant.id === id)) {
480
-        items.push(localParticipant);
481
-    }
482
-
483
-    items.sort((a, b) =>
484
-        getParticipantDisplayName(stateful, a.id).localeCompare(getParticipantDisplayName(stateful, b.id))
485
-    );
486
-
487
-    items.unshift(...raisedHandParticipants);
488
-
489
-    if (dominantSpeaker && dominantSpeaker !== localParticipant) {
490
-        items.unshift(dominantSpeaker);
491
     }
486
     }
492
 
487
 
493
-    return items;
494
-}
495
-
496
-/**
497
- * Selector for retrieving ids of alphabetically sorted participants by name.
498
- *
499
- * @param {(Function|Object)} stateful - The (whole) redux state, or redux's
500
- * {@code getState} function to be used to retrieve the state
501
- * features/base/participants.
502
- * @returns {Array<string>}
503
- */
504
-export function getSortedParticipantIds(stateful: Object | Function): Array<string> {
505
-    const participantIds = getSortedParticipants(stateful).map((p): Object => p.id);
488
+    // Remove self.
489
+    remoteRaisedHandParticipants.has(id) && remoteRaisedHandParticipants.delete(id);
506
 
490
 
507
-    return participantIds;
491
+    // Move self and participants with raised hand to the top of the list.
492
+    return [
493
+        id,
494
+        ...Array.from(remoteRaisedHandParticipants.keys()),
495
+        ...Array.from(reorderedParticipants.keys())
496
+    ];
508
 }
497
 }
509
 
498
 
510
 /**
499
 /**

+ 2
- 2
react/features/base/participants/reducer.js View File

61
     haveParticipantWithScreenSharingFeature: false,
61
     haveParticipantWithScreenSharingFeature: false,
62
     local: undefined,
62
     local: undefined,
63
     pinnedParticipant: undefined,
63
     pinnedParticipant: undefined,
64
+    raisedHandsQueue: [],
64
     remote: new Map(),
65
     remote: new Map(),
65
     sortedRemoteParticipants: new Map(),
66
     sortedRemoteParticipants: new Map(),
66
     sortedRemoteScreenshares: new Map(),
67
     sortedRemoteScreenshares: new Map(),
67
-    speakersList: new Map(),
68
-    raisedHandsQueue: []
68
+    speakersList: new Map()
69
 };
69
 };
70
 
70
 
71
 /**
71
 /**

+ 4
- 3
react/features/filmstrip/components/web/Filmstrip.js View File

240
         let stop = stopIndex;
240
         let stop = stopIndex;
241
 
241
 
242
         if (_thumbnailsReordered) {
242
         if (_thumbnailsReordered) {
243
-            // In tile view, the start index needs to be offset by 1 because the first thumbnail is that of the local
243
+            // In tile view, the indices needs to be offset by 1 because the first thumbnail is that of the local
244
             // endpoint. The remote participants start from index 1.
244
             // endpoint. The remote participants start from index 1.
245
             if (_currentLayout === LAYOUTS.TILE_VIEW) {
245
             if (_currentLayout === LAYOUTS.TILE_VIEW) {
246
-                start = startIndex > 0 ? startIndex - 1 : 0;
246
+                start = Math.max(startIndex - 1, 0);
247
                 stop = stopIndex - 1;
247
                 stop = stopIndex - 1;
248
             }
248
             }
249
         }
249
         }
528
  */
528
  */
529
 function _mapStateToProps(state) {
529
 function _mapStateToProps(state) {
530
     const toolbarButtons = getToolbarButtons(state);
530
     const toolbarButtons = getToolbarButtons(state);
531
-    const { enableThumbnailReordering = true } = state['features/base/config'];
531
+    const { testing = {} } = state['features/base/config'];
532
+    const enableThumbnailReordering = testing.enableThumbnailReordering ?? true;
532
     const { visible, remoteParticipants } = state['features/filmstrip'];
533
     const { visible, remoteParticipants } = state['features/filmstrip'];
533
     const reduceHeight = state['features/toolbox'].visible && toolbarButtons.length;
534
     const reduceHeight = state['features/toolbox'].visible && toolbarButtons.length;
534
     const remoteVideosVisible = shouldRemoteVideosBeVisible(state);
535
     const remoteVideosVisible = shouldRemoteVideosBeVisible(state);

+ 2
- 1
react/features/filmstrip/components/web/ThumbnailWrapper.js View File

104
     const _currentLayout = getCurrentLayout(state);
104
     const _currentLayout = getCurrentLayout(state);
105
     const { remoteParticipants } = state['features/filmstrip'];
105
     const { remoteParticipants } = state['features/filmstrip'];
106
     const remoteParticipantsLength = remoteParticipants.length;
106
     const remoteParticipantsLength = remoteParticipants.length;
107
-    const { enableThumbnailReordering = true } = state['features/base/config'];
107
+    const { testing = {} } = state['features/base/config'];
108
+    const enableThumbnailReordering = testing.enableThumbnailReordering ?? true;
108
 
109
 
109
     if (_currentLayout === LAYOUTS.TILE_VIEW) {
110
     if (_currentLayout === LAYOUTS.TILE_VIEW) {
110
         const { columnIndex, rowIndex } = ownProps;
111
         const { columnIndex, rowIndex } = ownProps;

+ 2
- 1
react/features/filmstrip/functions.any.js View File

12
  */
12
  */
13
 export function updateRemoteParticipants(store: Object, participantId: ?number) {
13
 export function updateRemoteParticipants(store: Object, participantId: ?number) {
14
     const state = store.getState();
14
     const state = store.getState();
15
-    const { enableThumbnailReordering = true } = state['features/base/config'];
15
+    const { testing = {} } = state['features/base/config'];
16
+    const enableThumbnailReordering = testing.enableThumbnailReordering ?? true;
16
     let reorderedParticipants = [];
17
     let reorderedParticipants = [];
17
 
18
 
18
     if (!enableThumbnailReordering) {
19
     if (!enableThumbnailReordering) {

+ 9
- 4
react/features/filmstrip/middleware.web.js View File

22
  * The middleware of the feature Filmstrip.
22
  * The middleware of the feature Filmstrip.
23
  */
23
  */
24
 MiddlewareRegistry.register(store => next => action => {
24
 MiddlewareRegistry.register(store => next => action => {
25
+    if (action.type === PARTICIPANT_LEFT) {
26
+        // This has to be executed before we remove the participant from features/base/participants state in order to
27
+        // remove the related thumbnail component before we need to re-render it. If we do this after next()
28
+        // we will be in sitation where the participant exists in the remoteParticipants array in features/filmstrip
29
+        // but doesn't exist in features/base/participants state which will lead to rendering a thumbnail for
30
+        // non-existing participant.
31
+        updateRemoteParticipantsOnLeave(store, action.participant?.id);
32
+    }
33
+
25
     const result = next(action);
34
     const result = next(action);
26
 
35
 
27
     switch (action.type) {
36
     switch (action.type) {
50
         updateRemoteParticipants(store, action.participant?.id);
59
         updateRemoteParticipants(store, action.participant?.id);
51
         break;
60
         break;
52
     }
61
     }
53
-    case PARTICIPANT_LEFT: {
54
-        updateRemoteParticipantsOnLeave(store, action.participant?.id);
55
-        break;
56
-    }
57
     case SETTINGS_UPDATED: {
62
     case SETTINGS_UPDATED: {
58
         if (typeof action.settings?.localFlipX === 'boolean') {
63
         if (typeof action.settings?.localFlipX === 'boolean') {
59
             // TODO: This needs to be removed once the large video is Reactified.
64
             // TODO: This needs to be removed once the large video is Reactified.

+ 1
- 1
react/features/participants-pane/components/web/MeetingParticipantContextMenu.js View File

396
         }
396
         }
397
 
397
 
398
         const actions
398
         const actions
399
-            = _participant.isFakeParticipant ? (
399
+            = _participant?.isFakeParticipant ? (
400
                 <>
400
                 <>
401
                     {_localVideoOwner && (
401
                     {_localVideoOwner && (
402
                         <ContextMenuItem onClick = { this._onStopSharedVideo }>
402
                         <ContextMenuItem onClick = { this._onStopSharedVideo }>

+ 2
- 2
react/features/participants-pane/components/web/MeetingParticipantItem.js View File

170
             videoMediaState = { _videoMediaState }
170
             videoMediaState = { _videoMediaState }
171
             youText = { youText }>
171
             youText = { youText }>
172
 
172
 
173
-            {!overflowDrawer && !_participant.isFakeParticipant
173
+            {!overflowDrawer && !_participant?.isFakeParticipant
174
                 && <>
174
                 && <>
175
                     <ParticipantQuickAction
175
                     <ParticipantQuickAction
176
                         askUnmuteText = { askUnmuteText }
176
                         askUnmuteText = { askUnmuteText }
184
                  </>
184
                  </>
185
             }
185
             }
186
 
186
 
187
-            {!overflowDrawer && _localVideoOwner && _participant.isFakeParticipant && (
187
+            {!overflowDrawer && _localVideoOwner && _participant?.isFakeParticipant && (
188
                 <ParticipantActionEllipsis
188
                 <ParticipantActionEllipsis
189
                     aria-label = { participantActionEllipsisLabel }
189
                     aria-label = { participantActionEllipsisLabel }
190
                     onClick = { onContextMenu } />
190
                     onClick = { onContextMenu } />

Loading…
Cancel
Save