Skip to content

Commit

Permalink
MM-14196 Add virtualised list for posts (mattermost#2447)
Browse files Browse the repository at this point in the history
* MM-14196 Add virtualised list for posts

 * Use custom fork of react-window for creating virtualised lists
 * Remove all of the scroll corrections and rely on resize observer
   in virtualised list to figure out the scroll corrections
 * Use 100px as the trigger for loading more posts

* Fix channel sync issue for first team switch

* Add tests
  * Fix review comments
  * Remove global event emitter
  * remove references to postlist scroll change

* Revert css change for post_hover menu

* Remove unused import

* Add inital value for floatingTimestamp

* Update react window has to fix mattermost#2447 (comment)
  • Loading branch information
sudheerDev committed Mar 25, 2019
1 parent 9681600 commit ec5c279
Show file tree
Hide file tree
Showing 37 changed files with 799 additions and 619 deletions.
16 changes: 0 additions & 16 deletions actions/global_actions.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import debounce from 'lodash/debounce';
import {batchActions} from 'redux-batched-actions';

import {
Expand Down Expand Up @@ -37,7 +36,6 @@ import LocalStorageStore from 'stores/local_storage_store';
import WebSocketClient from 'client/web_websocket_client.jsx';

import {ActionTypes, Constants, PostTypes, RHSStates} from 'utils/constants.jsx';
import EventTypes from 'utils/event_types.jsx';
import {filterAndSortTeamsByDisplayName} from 'utils/team_utils.jsx';
import * as Utils from 'utils/utils.jsx';
import {equalServerVersions} from 'utils/server_version';
Expand Down Expand Up @@ -321,20 +319,6 @@ export async function redirectUserToDefaultTeam() {
}
}

export const postListScrollChange = debounce(() => {
AppDispatcher.handleViewAction({
type: EventTypes.POST_LIST_SCROLL_CHANGE,
value: false,
});
});

export function postListScrollChangeToBottom() {
AppDispatcher.handleViewAction({
type: EventTypes.POST_LIST_SCROLL_CHANGE,
value: true,
});
}

let serverVersion = '';

export function reloadIfServerVersionChanged() {
Expand Down
1 change: 0 additions & 1 deletion components/create_post/create_post.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ jest.mock('actions/global_actions.jsx', () => ({
emitUserPostedEvent: jest.fn(),
showChannelNameUpdateModal: jest.fn(),
toggleShortcutsModal: jest.fn(),
postListScrollChange: jest.fn(),
}));

jest.mock('react-dom', () => ({
Expand Down
2 changes: 0 additions & 2 deletions components/create_post/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {Posts, Preferences as PreferencesRedux} from 'mattermost-redux/constants

import {connectionErrorCount} from 'selectors/views/system';

import {postListScrollChangeToBottom} from 'actions/global_actions.jsx';
import {addReaction, createPost, setEditingPost} from 'actions/post_actions.jsx';
import {selectPostFromRightHandSideSearchByPostId} from 'actions/views/rhs';
import {executeCommand} from 'actions/command';
Expand Down Expand Up @@ -99,7 +98,6 @@ function makeMapStateToProps() {
function onSubmitPost(post, fileInfos) {
return (dispatch) => {
dispatch(createPost(post, fileInfos));
postListScrollChangeToBottom();
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ exports[`components/PermalinkView should match snapshot 1`] = `
<withRouter(Connect(ChannelHeader))
channelId="channel_id"
/>
<Connect(PostList)
<withRouter(Connect(PostList))
channelId="channel_id"
focusedPostId="post_id"
/>
Expand Down Expand Up @@ -41,7 +41,7 @@ exports[`components/PermalinkView should match snapshot with archived channel 1`
<withRouter(Connect(ChannelHeader))
channelId="channel_id"
/>
<Connect(PostList)
<withRouter(Connect(PostList))
channelId="channel_id"
focusedPostId="post_id"
/>
Expand Down
5 changes: 4 additions & 1 deletion components/post_view/floating_timestamp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ export default class FloatingTimestamp extends React.PureComponent {
static propTypes = {
isScrolling: PropTypes.bool.isRequired,
isMobile: PropTypes.bool,
createAt: PropTypes.number,
createAt: PropTypes.oneOfType([
PropTypes.instanceOf(Date),
PropTypes.number,
]).isRequired,
isRhsPost: PropTypes.bool,
}

Expand Down
49 changes: 43 additions & 6 deletions components/post_view/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,49 @@

import {connect} from 'react-redux';
import {bindActionCreators} from 'redux';
import {withRouter} from 'react-router-dom';

import {getPosts, getPostsAfter, getPostsBefore, getPostThread} from 'mattermost-redux/actions/posts';
import {getChannel} from 'mattermost-redux/selectors/entities/channels';
import {makeGetPostsAroundPost, makeGetPostsInChannel} from 'mattermost-redux/selectors/entities/posts';
import {get} from 'mattermost-redux/selectors/entities/preferences';
import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users';
import {getTeamByName} from 'mattermost-redux/selectors/entities/teams';

import {increasePostVisibility} from 'actions/post_actions.jsx';
import {checkAndSetMobileView} from 'actions/views/channel';
import {Preferences} from 'utils/constants.jsx';
import {makePreparePostIdsForPostList} from 'selectors/posts';
import {Constants} from 'utils/constants.jsx';

import PostList from './post_list.jsx';

// This function is added as a fail safe for the channel sync issue we have.
// When the user switches to a team for the first time we show the channel of previous team and then settle for the right channel after that
// This causes the scroll correction etc an issue because post_list is not mounted for new channel instead it is updated
const isChannelLoading = (params, channel, team) => {
if (params.postid) {
return false;
}

if (channel && team) {
if (channel.type !== Constants.DM_CHANNEL && channel.type !== Constants.GM_CHANNEL) {
if (channel.name !== params.identifier) {
return true;
}
}
if (channel.team_id && channel.team_id !== team.id) {
return true;
}

return false;
}

return true;
};

function makeMapStateToProps() {
const getPostsInChannel = makeGetPostsInChannel();
const getPostsAroundPost = makeGetPostsAroundPost();
const preparePostIdsForPostList = makePreparePostIdsForPostList();

return function mapStateToProps(state, ownProps) {
const postVisibility = state.views.channel.postVisibility[ownProps.channelId];
Expand All @@ -29,15 +57,24 @@ function makeMapStateToProps() {
posts = getPostsInChannel(state, ownProps.channelId, postVisibility);
}

const channel = getChannel(state, ownProps.channelId);
const team = getTeamByName(state, ownProps.match.params.team);

const channelLoading = isChannelLoading(ownProps.match.params, channel, team);
const lastViewedAt = state.views.channel.lastChannelViewTime[ownProps.channelId];
const {postIds, postsObjById} = preparePostIdsForPostList(state, {posts, lastViewedAt, indicateNewMessages: true});

return {
channel: getChannel(state, ownProps.channelId) || {},
lastViewedAt: state.views.channel.lastChannelViewTime[ownProps.channelId],
channel,
lastViewedAt,
posts,
postsObjById,
postVisibility,
postListIds: postIds,
loadingPosts: state.views.channel.loadingPosts[ownProps.channelId],
focusedPostId: ownProps.focusedPostId,
currentUserId: getCurrentUserId(state),
fullWidth: get(state, Preferences.CATEGORY_DISPLAY_SETTINGS, Preferences.CHANNEL_DISPLAY_MODE, Preferences.CHANNEL_DISPLAY_MODE_DEFAULT) === Preferences.CHANNEL_DISPLAY_MODE_FULL_SCREEN,
channelLoading,
};
};
}
Expand All @@ -55,4 +92,4 @@ function mapDispatchToProps(dispatch) {
};
}

export default connect(makeMapStateToProps, mapDispatchToProps)(PostList);
export default withRouter(connect(makeMapStateToProps, mapDispatchToProps)(PostList));
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
import PropTypes from 'prop-types';
import React from 'react';

import {postListScrollChange} from 'actions/global_actions';

import {getImageSrc} from 'utils/post_utils';
import {isUrlSafe} from 'utils/url';
import {handleFormattedTextClick} from 'utils/utils';
Expand Down Expand Up @@ -99,8 +97,6 @@ export default class MessageAttachment extends React.PureComponent {
this.setState((prevState) => {
return {checkOverflow: prevState.checkOverflow + 1};
});

postListScrollChange();
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@ import {mount, shallow} from 'enzyme';

import SizeAwareImage from 'components/size_aware_image';
import MessageAttachment from 'components/post_view/message_attachments/message_attachment/message_attachment.jsx';
import {postListScrollChange} from 'actions/global_actions';

jest.mock('actions/global_actions.jsx', () => ({
postListScrollChange: jest.fn(),
}));

describe('components/post_view/MessageAttachment', () => {
const attachment = {
Expand Down Expand Up @@ -48,18 +43,16 @@ describe('components/post_view/MessageAttachment', () => {
expect(wrapper).toMatchSnapshot();
});

test('should match state and have called postListScrollChange on handleImageHeightReceived', () => {
test('should change checkOverflow state on handleHeightReceived change', () => {
const wrapper = shallow(<MessageAttachment {...baseProps}/>);
const instance = wrapper.instance();
instance.checkAttachmentTextOverflow = jest.fn();

wrapper.setState({checkOverflow: 0});
instance.handleHeightReceived(1);
expect(postListScrollChange).toHaveBeenCalledTimes(1);
expect(wrapper.state('checkOverflow')).toEqual(1);

instance.handleHeightReceived(0);
expect(postListScrollChange).toHaveBeenCalledTimes(1);
expect(wrapper.state('checkOverflow')).toEqual(1);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`components/post_view/new_message_separator should render new_message_separator 1`] = `
<div
className="new-separator"
id="1234"
>
<hr
className="separator__hr"
/>
<div
className="separator__text"
>
<FormattedMessage
defaultMessage="New Messages"
id="posts_view.newMsg"
values={Object {}}
/>
</div>
</div>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import PropTypes from 'prop-types';
import React from 'react';
import {FormattedMessage} from 'react-intl';

export default class NewMessageSeparator extends React.PureComponent {
static propTypes = {
separatorId: PropTypes.string.isRequired,
}

render() {
return (
<div
id={this.props.separatorId}
className='new-separator'
>
<hr className='separator__hr'/>
<div className='separator__text'>
<FormattedMessage
id='posts_view.newMsg'
defaultMessage='New Messages'
/>
</div>
</div>
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import React from 'react';

import {shallowWithIntl} from 'tests/helpers/intl-test-helper.jsx';

import NewMessageSeparator from './new_message_separator.jsx';

describe('components/post_view/new_message_separator', () => {
test('should render new_message_separator', () => {
const wrapper = shallowWithIntl(
<NewMessageSeparator separatorId='1234'/>
);
expect(wrapper).toMatchSnapshot();
});
});
1 change: 0 additions & 1 deletion components/post_view/post/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ function mapStateToProps(state, ownProps) {
post: getPost(state, detailedPost.id),
currentUserId: getCurrentUserId(state),
isFirstReply: Boolean(detailedPost.isFirstReply && detailedPost.commentedOnPost),
highlight: detailedPost.highlight,
consecutivePostByUser: detailedPost.consecutivePostByUser,
previousPostIsComment: detailedPost.previousPostIsComment,
replyCount: detailedPost.replyCount,
Expand Down
10 changes: 2 additions & 8 deletions components/post_view/post/post.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default class Post extends React.PureComponent {
/**
* Set to highlight the background of the post
*/
highlight: PropTypes.bool,
shouldHighlight: PropTypes.bool,

/**
* Set to render this post as if it was attached to the previous post
Expand All @@ -63,11 +63,6 @@ export default class Post extends React.PureComponent {
*/
replyCount: PropTypes.number,

/**
* Function to get the post list HTML element
*/
getPostList: PropTypes.func.isRequired,

actions: PropTypes.shape({
selectPost: PropTypes.func.isRequired,
}).isRequired,
Expand Down Expand Up @@ -129,7 +124,7 @@ export default class Post extends React.PureComponent {
className += ' post--hide-controls';
}

if (this.props.highlight) {
if (this.props.shouldHighlight) {
className += ' post--highlight';
}

Expand Down Expand Up @@ -255,7 +250,6 @@ export default class Post extends React.PureComponent {
isFirstReply={this.props.isFirstReply}
replyCount={this.props.replyCount}
showTimeWithoutHover={!hideProfilePicture}
getPostList={this.props.getPostList}
hover={this.state.hover}
/>
<PostBody
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import PropTypes from 'prop-types';
import React from 'react';

import SizeAwareImage from 'components/size_aware_image';
import {postListScrollChange} from 'actions/global_actions.jsx';
import * as CommonUtils from 'utils/commons.jsx';
import {PostTypes} from 'utils/constants.jsx';
import {useSafeUrl} from 'utils/url';
Expand Down Expand Up @@ -139,7 +138,6 @@ export default class PostAttachmentOpenGraph extends React.PureComponent {
this.setState({
hasLargeImage,
});
postListScrollChange();
}

imageToggleAnchorTag(imageUrl) {
Expand Down
6 changes: 0 additions & 6 deletions components/post_view/post_header/post_header.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ export default class PostHeader extends React.PureComponent {
*/
isFirstReply: PropTypes.bool,

/**
* Function to get the post list HTML element
*/
getPostList: PropTypes.func.isRequired,

/**
* Set to mark post as being hovered over
*/
Expand Down Expand Up @@ -154,7 +149,6 @@ export default class PostHeader extends React.PureComponent {
replyCount={this.props.replyCount}
isFirstReply={this.props.isFirstReply}
showTimeWithoutHover={this.props.showTimeWithoutHover}
getPostList={this.props.getPostList}
hover={this.props.hover}
/>
</div>
Expand Down
4 changes: 0 additions & 4 deletions components/post_view/post_image/post_image.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import PropTypes from 'prop-types';
import React from 'react';

import {postListScrollChange} from 'actions/global_actions';
import SizeAwareImage from 'components/size_aware_image';
import * as PostUtils from 'utils/post_utils.jsx';

Expand Down Expand Up @@ -50,9 +49,6 @@ export default class PostImage extends React.PureComponent {
return;
}

if (!this.props.dimensions) {
postListScrollChange();
}
if (this.props.onLinkLoaded) {
this.props.onLinkLoaded();
}
Expand Down
Loading

0 comments on commit ec5c279

Please sign in to comment.