fix(filmstrip): Move thumbnails reordering behind a config.js flag.

enableThumbnailReordering flag (enabled by default) will be used to check if the thumbnails needs to be reodred in the UI.
This commit is contained in:
Jaya Allamsetty 2021-08-19 17:56:45 -04:00 committed by Jaya Allamsetty
parent 751d9a9b8e
commit 7827c3d1ad
8 changed files with 147 additions and 77 deletions

View File

@ -61,7 +61,7 @@ const DEFAULT_STATE = {
pinnedParticipant: undefined, pinnedParticipant: undefined,
remote: new Map(), remote: new Map(),
sortedRemoteParticipants: new Map(), sortedRemoteParticipants: new Map(),
speakersList: [] speakersList: new Map()
}; };
/** /**
@ -96,12 +96,28 @@ ReducerRegistry.register('features/base/participants', (state = DEFAULT_STATE, a
case DOMINANT_SPEAKER_CHANGED: { case DOMINANT_SPEAKER_CHANGED: {
const { participant } = action; const { participant } = action;
const { id, previousSpeakers = [] } = participant; const { id, previousSpeakers = [] } = participant;
const { dominantSpeaker, local } = state; const { dominantSpeaker, local, speakersList } = state;
const speakersList = []; const newSpeakers = [ id, ...previousSpeakers ];
const sortedSpeakersList = Array.from(speakersList);
// Update the speakers list. // Update the speakers list.
id !== local?.id && speakersList.push(id); for (const speaker of newSpeakers) {
speakersList.push(...previousSpeakers.filter(p => p !== local?.id)); if (!state.speakersList.has(speaker) && speaker !== local?.id) {
const remoteParticipant = state.remote.get(speaker);
remoteParticipant && sortedSpeakersList.push([ speaker, _getDisplayName(remoteParticipant.name) ]);
}
}
// Also check if any of the existing speakers have been kicked off the list.
for (const existingSpeaker of sortedSpeakersList.keys()) {
if (!newSpeakers.find(s => s === existingSpeaker)) {
sortedSpeakersList.filter(sortedSpeaker => sortedSpeaker[0] !== existingSpeaker);
}
}
// Keep the remote speaker list sorted alphabetically.
sortedSpeakersList.sort((a, b) => a[1].localeCompare(b[1]));
// Only one dominant speaker is allowed. // Only one dominant speaker is allowed.
if (dominantSpeaker) { if (dominantSpeaker) {
@ -112,7 +128,7 @@ ReducerRegistry.register('features/base/participants', (state = DEFAULT_STATE, a
return { return {
...state, ...state,
dominantSpeaker: id, dominantSpeaker: id,
speakersList speakersList: new Map(sortedSpeakersList)
}; };
} }
@ -227,8 +243,7 @@ ReducerRegistry.register('features/base/participants', (state = DEFAULT_STATE, a
state.remote.set(id, participant); state.remote.set(id, participant);
// Insert the new participant. // Insert the new participant.
const displayName = name const displayName = _getDisplayName(name);
?? (typeof interfaceConfig === 'object' ? interfaceConfig.DEFAULT_REMOTE_DISPLAY_NAME : 'Fellow Jitser');
const sortedRemoteParticipants = Array.from(state.sortedRemoteParticipants); const sortedRemoteParticipants = Array.from(state.sortedRemoteParticipants);
sortedRemoteParticipants.push([ id, displayName ]); sortedRemoteParticipants.push([ id, displayName ]);
@ -297,7 +312,7 @@ ReducerRegistry.register('features/base/participants', (state = DEFAULT_STATE, a
} }
// Remove the participant from the list of speakers. // Remove the participant from the list of speakers.
state.speakersList = state.speakersList.filter(speaker => speaker !== id); state.speakersList.has(id) && state.speakersList.delete(id);
if (pinnedParticipant === id) { if (pinnedParticipant === id) {
state.pinnedParticipant = undefined; state.pinnedParticipant = undefined;
@ -314,6 +329,17 @@ ReducerRegistry.register('features/base/participants', (state = DEFAULT_STATE, a
return state; return state;
}); });
/**
* Returns the participant's display name, default string if display name is not set on the participant.
*
* @param {string} name - The display name of the participant.
* @returns {string}
*/
function _getDisplayName(name) {
return name
?? (typeof interfaceConfig === 'object' ? interfaceConfig.DEFAULT_REMOTE_DISPLAY_NAME : 'Fellow Jitser');
}
/** /**
* Loops trough the participants in the state in order to check if all participants are moderators. * Loops trough the participants in the state in order to check if all participants are moderators.
* *
@ -335,32 +361,6 @@ function _isEveryoneModerator(state) {
return false; return false;
} }
/**
* Updates a specific property for a participant.
*
* @param {State} state - The redux state.
* @param {string} id - The ID of the participant.
* @param {string} property - The property to update.
* @param {*} value - The new value.
* @returns {boolean} - True if a participant was updated and false otherwise.
*/
function _updateParticipantProperty(state, id, property, value) {
const { remote, local } = state;
if (remote.has(id)) {
remote.set(id, set(remote.get(id), property, value));
return true;
} else if (local?.id === id) {
state.local = set(local, property, value);
return true;
}
return false;
}
/** /**
* Reducer function for a single participant. * Reducer function for a single participant.
* *
@ -458,3 +458,28 @@ function _participantJoined({ participant }) {
role: role || PARTICIPANT_ROLE.NONE role: role || PARTICIPANT_ROLE.NONE
}; };
} }
/**
* Updates a specific property for a participant.
*
* @param {State} state - The redux state.
* @param {string} id - The ID of the participant.
* @param {string} property - The property to update.
* @param {*} value - The new value.
* @returns {boolean} - True if a participant was updated and false otherwise.
*/
function _updateParticipantProperty(state, id, property, value) {
const { remote, local } = state;
if (remote.has(id)) {
remote.set(id, set(remote.get(id), property, value));
return true;
} else if (local?.id === id) {
state.local = set(local, property, value);
return true;
}
return false;
}

View File

@ -132,7 +132,7 @@ export function clickOnVideo(n: number) {
const state = getState(); const state = getState();
const { id: localId } = getLocalParticipant(state); const { id: localId } = getLocalParticipant(state);
// Use the reordered list of participants. // Use the list that correctly represents the current order of the participants as visible in the UI.
const { remoteParticipants } = state['features/filmstrip']; const { remoteParticipants } = state['features/filmstrip'];
const participants = [ localId, ...remoteParticipants ]; const participants = [ localId, ...remoteParticipants ];
const { id, pinned } = getParticipantById(state, participants[n]); const { id, pinned } = getParticipantById(state, participants[n]);

View File

@ -94,6 +94,11 @@ type Props = {
*/ */
_thumbnailWidth: number, _thumbnailWidth: number,
/**
* Flag that indicates whether the thumbnails will be reordered.
*/
_thumbnailsReordered: Boolean,
/** /**
* Additional CSS class names to add to the container of all the thumbnails. * Additional CSS class names to add to the container of all the thumbnails.
*/ */
@ -222,6 +227,33 @@ class Filmstrip extends PureComponent <Props> {
); );
} }
/**
* Calculates the start and stop indices based on whether the thumbnails need to be reordered in the filmstrip.
*
* @param {number} startIndex - The start index.
* @param {number} stopIndex - The stop index.
* @returns {Object}
*/
_calculateIndices(startIndex, stopIndex) {
const { _currentLayout, _thumbnailsReordered } = this.props;
let start = startIndex;
let stop = stopIndex;
if (_thumbnailsReordered) {
// In tile view, the start index needs to be offset by 1 because the first thumbnail is that of the local
// endpoint. The remote participants start from index 1.
if (_currentLayout === LAYOUTS.TILE_VIEW) {
start = startIndex > 0 ? startIndex - 1 : 0;
stop = stopIndex - 1;
}
}
return {
startIndex: start,
stopIndex: stop
};
}
_onTabIn: () => void; _onTabIn: () => void;
/** /**
@ -262,18 +294,22 @@ class Filmstrip extends PureComponent <Props> {
* @returns {string} - The key. * @returns {string} - The key.
*/ */
_gridItemKey({ columnIndex, rowIndex }) { _gridItemKey({ columnIndex, rowIndex }) {
const { _columns, _remoteParticipants, _remoteParticipantsLength } = this.props; const { _columns, _remoteParticipants, _remoteParticipantsLength, _thumbnailsReordered } = this.props;
const index = (rowIndex * _columns) + columnIndex; const index = (rowIndex * _columns) + columnIndex;
// When the thumbnails are reordered, local participant is inserted at index 0.
const localIndex = _thumbnailsReordered ? 0 : _remoteParticipantsLength;
const remoteIndex = _thumbnailsReordered ? index - 1 : index;
if (index > _remoteParticipantsLength) { if (index > _remoteParticipantsLength) {
return `empty-${index}`; return `empty-${index}`;
} }
if (index === 0) { if (index === localIndex) {
return 'local'; return 'local';
} }
return _remoteParticipants[index - 1]; return _remoteParticipants[remoteIndex];
} }
_onListItemsRendered: Object => void; _onListItemsRendered: Object => void;
@ -286,8 +322,9 @@ class Filmstrip extends PureComponent <Props> {
*/ */
_onListItemsRendered({ visibleStartIndex, visibleStopIndex }) { _onListItemsRendered({ visibleStartIndex, visibleStopIndex }) {
const { dispatch } = this.props; const { dispatch } = this.props;
const { startIndex, stopIndex } = this._calculateIndices(visibleStartIndex, visibleStopIndex);
dispatch(setVisibleRemoteParticipants(visibleStartIndex, visibleStopIndex + 1)); dispatch(setVisibleRemoteParticipants(startIndex, stopIndex));
} }
_onGridItemsRendered: Object => void; _onGridItemsRendered: Object => void;
@ -305,13 +342,11 @@ class Filmstrip extends PureComponent <Props> {
visibleRowStopIndex visibleRowStopIndex
}) { }) {
const { _columns, dispatch } = this.props; const { _columns, dispatch } = this.props;
let startIndex = (visibleRowStartIndex * _columns) + visibleColumnStartIndex; const start = (visibleRowStartIndex * _columns) + visibleColumnStartIndex;
const endIndex = (visibleRowStopIndex * _columns) + visibleColumnStopIndex; const stop = (visibleRowStopIndex * _columns) + visibleColumnStopIndex;
const { startIndex, stopIndex } = this._calculateIndices(start, stop);
// In tile view, the start index needs to be offset by 1 because the first participant is the local dispatch(setVisibleRemoteParticipants(startIndex, stopIndex));
// participant.
startIndex = startIndex > 0 ? startIndex - 1 : 0;
dispatch(setVisibleRemoteParticipants(startIndex, endIndex));
} }
/** /**
@ -493,6 +528,7 @@ class Filmstrip extends PureComponent <Props> {
*/ */
function _mapStateToProps(state) { function _mapStateToProps(state) {
const toolbarButtons = getToolbarButtons(state); const toolbarButtons = getToolbarButtons(state);
const { enableThumbnailReordering = true } = state['features/base/config'];
const { visible, remoteParticipants } = state['features/filmstrip']; const { visible, remoteParticipants } = state['features/filmstrip'];
const reduceHeight = state['features/toolbox'].visible && toolbarButtons.length; const reduceHeight = state['features/toolbox'].visible && toolbarButtons.length;
const remoteVideosVisible = shouldRemoteVideosBeVisible(state); const remoteVideosVisible = shouldRemoteVideosBeVisible(state);
@ -565,6 +601,7 @@ function _mapStateToProps(state) {
_rows: gridDimensions.rows, _rows: gridDimensions.rows,
_thumbnailWidth: _thumbnailSize?.width, _thumbnailWidth: _thumbnailSize?.width,
_thumbnailHeight: _thumbnailSize?.height, _thumbnailHeight: _thumbnailSize?.height,
_thumbnailsReordered: enableThumbnailReordering,
_videosClassName: videosClassName, _videosClassName: videosClassName,
_visible: visible, _visible: visible,
_isToolboxVisible: isToolboxVisible(state) _isToolboxVisible: isToolboxVisible(state)

View File

@ -104,6 +104,7 @@ function _mapStateToProps(state, ownProps) {
const _currentLayout = getCurrentLayout(state); const _currentLayout = getCurrentLayout(state);
const { remoteParticipants } = state['features/filmstrip']; const { remoteParticipants } = state['features/filmstrip'];
const remoteParticipantsLength = remoteParticipants.length; const remoteParticipantsLength = remoteParticipants.length;
const { enableThumbnailReordering = true } = state['features/base/config'];
if (_currentLayout === LAYOUTS.TILE_VIEW) { if (_currentLayout === LAYOUTS.TILE_VIEW) {
const { columnIndex, rowIndex } = ownProps; const { columnIndex, rowIndex } = ownProps;
@ -126,8 +127,11 @@ function _mapStateToProps(state, ownProps) {
return {}; return {};
} }
// Make the local participant as the first thumbnail (top left corner) in tile view. // When the thumbnails are reordered, local participant is inserted at index 0.
if (index === 0) { const localIndex = enableThumbnailReordering ? 0 : remoteParticipantsLength;
const remoteIndex = enableThumbnailReordering ? index - 1 : index;
if (index === localIndex) {
return { return {
_participantID: 'local', _participantID: 'local',
_horizontalOffset: horizontalOffset _horizontalOffset: horizontalOffset
@ -135,7 +139,7 @@ function _mapStateToProps(state, ownProps) {
} }
return { return {
_participantID: remoteParticipants[index - 1], _participantID: remoteParticipants[remoteIndex],
_horizontalOffset: horizontalOffset _horizontalOffset: horizontalOffset
}; };
} }

View File

@ -6,31 +6,52 @@ import { setRemoteParticipants } from './actions';
* Computes the reorderd list of the remote participants. * Computes the reorderd list of the remote participants.
* *
* @param {*} store - The redux store. * @param {*} store - The redux store.
* @param {string} participantId - The endpoint id of the participant that joined the call.
* @returns {void} * @returns {void}
* @private * @private
*/ */
export function updateRemoteParticipants(store: Object) { export function updateRemoteParticipants(store: Object, participantId: ?number) {
const state = store.getState(); const state = store.getState();
const { enableThumbnailReordering = true } = state['features/base/config'];
let reorderedParticipants = [];
if (!enableThumbnailReordering) {
if (participantId) {
const { remoteParticipants } = state['features/filmstrip'];
reorderedParticipants = [ ...remoteParticipants, participantId ];
store.dispatch(setRemoteParticipants(reorderedParticipants));
}
return;
}
const { fakeParticipants, sortedRemoteParticipants, speakersList } = state['features/base/participants']; const { fakeParticipants, sortedRemoteParticipants, speakersList } = state['features/base/participants'];
const { remoteScreenShares } = state['features/video-layout']; const { remoteScreenShares } = state['features/video-layout'];
const screenShares = (remoteScreenShares || []).slice(); const screenShares = (remoteScreenShares || []).slice();
let speakers = (speakersList || []).slice(); const speakers = new Map(speakersList);
const remoteParticipants = new Map(sortedRemoteParticipants); const remoteParticipants = new Map(sortedRemoteParticipants);
const sharedVideos = fakeParticipants ? Array.from(fakeParticipants.keys()) : []; const sharedVideos = fakeParticipants ? Array.from(fakeParticipants.keys()) : [];
for (const screenshare of screenShares) { for (const screenshare of screenShares) {
remoteParticipants.delete(screenshare); remoteParticipants.delete(screenshare);
speakers = speakers.filter(speaker => speaker !== screenshare); speakers.delete(screenshare);
} }
for (const sharedVideo of sharedVideos) { for (const sharedVideo of sharedVideos) {
remoteParticipants.delete(sharedVideo); remoteParticipants.delete(sharedVideo);
speakers = speakers.filter(speaker => speaker !== sharedVideo); speakers.delete(sharedVideo);
} }
for (const speaker of speakers) { for (const speaker of speakers.keys()) {
remoteParticipants.delete(speaker); remoteParticipants.delete(speaker);
} }
const reorderedParticipants
= [ ...screenShares.reverse(), ...sharedVideos, ...speakers, ...Array.from(remoteParticipants.keys()) ]; // Always update the order of the thumnails.
reorderedParticipants = [
...screenShares.reverse(),
...sharedVideos,
...Array.from(speakers.keys()),
...Array.from(remoteParticipants.keys())
];
store.dispatch(setRemoteParticipants(reorderedParticipants)); store.dispatch(setRemoteParticipants(reorderedParticipants));
} }

View File

@ -47,7 +47,7 @@ MiddlewareRegistry.register(store => next => action => {
break; break;
} }
case PARTICIPANT_JOINED: { case PARTICIPANT_JOINED: {
updateRemoteParticipants(store); updateRemoteParticipants(store, action.participant?.id);
break; break;
} }
case PARTICIPANT_LEFT: { case PARTICIPANT_LEFT: {

View File

@ -155,7 +155,8 @@ ReducerRegistry.register(
...state, ...state,
visibleParticipantsStartIndex: action.startIndex, visibleParticipantsStartIndex: action.startIndex,
visibleParticipantsEndIndex: action.endIndex, visibleParticipantsEndIndex: action.endIndex,
visibleRemoteParticipants: new Set(state.remoteParticipants.slice(action.startIndex, action.endIndex)) visibleRemoteParticipants:
new Set(state.remoteParticipants.slice(action.startIndex, action.endIndex + 1))
}; };
} }
case PARTICIPANT_LEFT: { case PARTICIPANT_LEFT: {

View File

@ -17,22 +17,4 @@ StateListenerRegistry.register(
*/ */
StateListenerRegistry.register( StateListenerRegistry.register(
/* selector */ state => state['features/base/participants'].dominantSpeaker, /* selector */ state => state['features/base/participants'].dominantSpeaker,
/* listener */ (dominantSpeaker, store) => _reorderDominantSpeakers(store)); /* listener */ (dominantSpeaker, store) => updateRemoteParticipants(store));
/**
* Private helper function that reorders the remote participants based on dominant speaker changes.
*
* @param {*} store - The redux store.
* @returns {void}
* @private
*/
function _reorderDominantSpeakers(store) {
const state = store.getState();
const { dominantSpeaker, local } = state['features/base/participants'];
const { visibleRemoteParticipants } = state['features/filmstrip'];
// Reorder the participants if the new dominant speaker is currently not visible.
if (dominantSpeaker !== local?.id && !visibleRemoteParticipants.has(dominantSpeaker)) {
updateRemoteParticipants(store);
}
}