fix(mobile-pagination): address PR review comments

This commit is contained in:
Hristo Terezov 2021-08-26 18:23:38 -05:00
parent 7dd43d93b6
commit b31ed40309
5 changed files with 78 additions and 52 deletions

View File

@ -237,7 +237,7 @@ class Filmstrip extends PureComponent<Props> {
renderItem = { this._renderThumbnail }
showsHorizontalScrollIndicator = { false }
showsVerticalScrollIndicator = { false }
style = { styles.scrollView }
style = { styles.flatListStageView }
viewabilityConfig = { this._viewabilityConfig }
windowSize = { 2 } />
{

View File

@ -52,7 +52,7 @@ type Props = {
_isFakeParticipant: boolean,
/**
* Indicates whether the participant is fake.
* Indicates whether the participant is screen sharing.
*/
_isScreenShare: boolean,
@ -200,12 +200,11 @@ class Thumbnail extends PureComponent<Props> {
}
/**
* Implements React's {@link Component#render()}.
* Renders the indicators for the thumbnail.
*
* @inheritdoc
* @returns {ReactElement}
*/
render() {
_renderIndicators() {
const {
_audioMuted: audioMuted,
_isScreenShare: isScreenShare,
@ -213,10 +212,56 @@ class Thumbnail extends PureComponent<Props> {
_renderDominantSpeakerIndicator: renderDominantSpeakerIndicator,
_renderModeratorIndicator: renderModeratorIndicator,
_participantId: participantId,
_videoMuted: videoMuted
} = this.props;
const indicators = [];
if (renderModeratorIndicator) {
indicators.push(<View style = { styles.moderatorIndicatorContainer }>
<ModeratorIndicator />
</View>);
}
if (!_isFakeParticipant) {
indicators.push(<View
style = { [
styles.thumbnailTopIndicatorContainer,
styles.thumbnailTopLeftIndicatorContainer
] }>
<RaisedHandIndicator participantId = { participantId } />
{ renderDominantSpeakerIndicator && <DominantSpeakerIndicator /> }
</View>);
indicators.push(<View
style = { [
styles.thumbnailTopIndicatorContainer,
styles.thumbnailTopRightIndicatorContainer
] }>
<ConnectionIndicator participantId = { participantId } />
</View>);
indicators.push(<Container style = { styles.thumbnailIndicatorContainer }>
{ audioMuted && <AudioMutedIndicator /> }
{ videoMuted && <VideoMutedIndicator /> }
{ isScreenShare && <ScreenShareIndicator /> }
</Container>);
}
return indicators;
}
/**
* Implements React's {@link Component#render()}.
*
* @inheritdoc
* @returns {ReactElement}
*/
render() {
const {
_isScreenShare: isScreenShare,
_isFakeParticipant,
_participantId: participantId,
_participantInLargeVideo: participantInLargeVideo,
_pinned,
_styles,
_videoMuted: videoMuted,
disableTint,
height,
renderDisplayName,
@ -255,39 +300,8 @@ class Thumbnail extends PureComponent<Props> {
<DisplayNameLabel participantId = { participantId } />
</Container>
}
{ renderModeratorIndicator
&& <View style = { styles.moderatorIndicatorContainer }>
<ModeratorIndicator />
</View>
}
{
!_isFakeParticipant
&& <View
style = { [
styles.thumbnailTopIndicatorContainer,
styles.thumbnailTopLeftIndicatorContainer
] }>
<RaisedHandIndicator participantId = { participantId } />
{ renderDominantSpeakerIndicator && <DominantSpeakerIndicator /> }
</View>
}
{
!_isFakeParticipant
&& <View
style = { [
styles.thumbnailTopIndicatorContainer,
styles.thumbnailTopRightIndicatorContainer
] }>
<ConnectionIndicator participantId = { participantId } />
</View>
}
{
!_isFakeParticipant
&& <Container style = { styles.thumbnailIndicatorContainer }>
{ audioMuted && <AudioMutedIndicator /> }
{ videoMuted && <VideoMutedIndicator /> }
{ isScreenShare && <ScreenShareIndicator /> }
</Container>
this._renderIndicators()
}
</Container>
);

View File

@ -71,6 +71,12 @@ type Props = {
onClick: Function
};
/**
* An empty array. The purpose of the constant is to use the same reference every time we need an empty array.
* This will prevent unnecessary re-renders.
*/
const EMPTY_ARRAY = [];
/**
* Implements a React {@link PureComponent} which displays thumbnails in a two
* dimensional grid.
@ -109,7 +115,7 @@ class TileView extends PureComponent<Props> {
itemVisiblePercentThreshold: 30
};
this._flatListStyles = {
...styles.flatList
...styles.flatListTileView
};
this._contentContainerStyles = {
...styles.contentContainer
@ -162,7 +168,7 @@ class TileView extends PureComponent<Props> {
if (this._flatListStyles.minHeight !== _height || this._flatListStyles.minWidth !== _width) {
this._flatListStyles = {
...styles.flatList,
...styles.flatListTileView,
minHeight: _height,
minWidth: _width
};
@ -207,11 +213,12 @@ class TileView extends PureComponent<Props> {
*/
_getSortedParticipants() {
const { _localParticipant, _remoteParticipants } = this.props;
const participants = [];
_localParticipant && participants.push(_localParticipant.id);
if (!_localParticipant) {
return EMPTY_ARRAY;
}
return [ ...participants, ..._remoteParticipants ];
return [ _localParticipant?.id, ..._remoteParticipants ];
}
_renderThumbnail: Object => Object;

View File

@ -71,9 +71,16 @@ export default {
},
/**
* The styles for the FlatList.
* The styles for the FlatList component in stage view.
*/
flatList: {
flatListStageView: {
flexGrow: 0
},
/**
* The styles for the FlatList component in tile view.
*/
flatListTileView: {
flex: 0
},
@ -94,13 +101,6 @@ export default {
right: 4
},
/**
* The style of the scrollview containing the remote thumbnails.
*/
scrollView: {
flexGrow: 0
},
/**
* The style of a participant's Thumbnail which renders either the video or
* the avatar of the associated participant.

View File

@ -13,6 +13,11 @@ import './subscriber';
*/
MiddlewareRegistry.register(store => next => action => {
if (action.type === PARTICIPANT_LEFT) {
// This have to be executed before we remove the participant from features/base/participants state in order to
// remove the related thumbnail component before we need to re-render it. If we do this after next()
// we will be in sitation where the participant exists in the remoteParticipants array in features/filmstrip
// but doesn't exist in features/base/participants state which will lead to rendering a thumbnail for
// non-existing participant.
updateRemoteParticipantsOnLeave(store, action.participant?.id);
}