Improving Performance Part 1 (#1009)

* Dispatch retry failed action | fix refreshing state | change postVisibility to 15

* Improve channel switching

* Improve handling of visible tooltip

* Improve watching for status bar height changes

* Remove unnecessary mapStateToProps from code screen

* Use drawer native functions to open/close instead of state

* Feedback review
This commit is contained in:
enahum 2017-10-11 14:33:26 -03:00 committed by GitHub
parent 5bfe97b1f2
commit dabad3eca6
18 changed files with 132 additions and 153 deletions

View file

@ -8,7 +8,6 @@ import {ViewTypes} from 'app/constants';
import {UserTypes} from 'mattermost-redux/action_types';
import {
fetchMyChannelsAndMembers,
getChannelStats,
selectChannel,
leaveChannel as serviceLeaveChannel,
unfavoriteChannel
@ -185,10 +184,12 @@ async function retryGetPostsAction(action, dispatch, getState, maxTries = MAX_PO
const posts = await action(dispatch, getState);
if (posts) {
dispatch(setChannelRetryFailed(false));
return posts;
}
}
dispatch(setChannelRetryFailed(true));
return null;
}
@ -255,15 +256,20 @@ export function handleSelectChannel(channelId) {
return async (dispatch, getState) => {
const {currentTeamId} = getState().entities.teams;
loadPostsIfNecessaryWithRetry(channelId)(dispatch, getState);
selectChannel(channelId)(dispatch, getState);
dispatch(setChannelLoading(false));
dispatch({
type: ViewTypes.SET_LAST_CHANNEL_FOR_TEAM,
teamId: currentTeamId,
channelId
});
getChannelStats(channelId)(dispatch, getState);
dispatch(batchActions([
{
type: ViewTypes.SET_INITIAL_POST_VISIBILITY,
data: channelId
},
setChannelLoading(false),
{
type: ViewTypes.SET_LAST_CHANNEL_FOR_TEAM,
teamId: currentTeamId,
channelId
}
]), 'BATCH_CHANNEL_LOADED');
};
}
@ -340,8 +346,11 @@ export function closeGMChannel(channel) {
}
export function refreshChannelWithRetry(channelId) {
return (dispatch, getState) => {
return retryGetPostsAction(getPosts(channelId), dispatch, getState);
return async (dispatch, getState) => {
dispatch(setChannelRefreshing(true));
const posts = await retryGetPostsAction(getPosts(channelId), dispatch, getState);
dispatch(setChannelRefreshing(false));
return posts;
};
}
@ -369,10 +378,10 @@ export function setChannelRefreshing(loading = true) {
};
}
export function setPostTooltipVisible(visible = true) {
export function setChannelRetryFailed(failed = true) {
return {
type: ViewTypes.POST_TOOLTIP_VISIBLE,
visible
type: ViewTypes.SET_CHANNEL_RETRY_FAILED,
failed
};
}

View file

@ -60,7 +60,6 @@ export default class ChannelDrawer extends PureComponent {
openDrawerOffset = DRAWER_LANDSCAPE_OFFSET;
}
this.state = {
openDrawer: false,
openDrawerOffset
};
}
@ -74,7 +73,6 @@ export default class ChannelDrawer extends PureComponent {
EventEmitter.on('close_channel_drawer', this.closeChannelDrawer);
EventEmitter.on(WebsocketEvents.CHANNEL_UPDATED, this.handleUpdateTitle);
BackHandler.addEventListener('hardwareBackPress', this.handleAndroidBack);
this.mounted = true;
}
componentWillReceiveProps(nextProps) {
@ -95,12 +93,11 @@ export default class ChannelDrawer extends PureComponent {
EventEmitter.off('close_channel_drawer', this.closeChannelDrawer);
EventEmitter.off(WebsocketEvents.CHANNEL_UPDATED, this.handleUpdateTitle);
BackHandler.removeEventListener('hardwareBackPress', this.handleAndroidBack);
this.mounted = false;
}
handleAndroidBack = () => {
if (this.state.openDrawer) {
this.setState({openDrawer: false});
if (this.refs.drawer && this.refs.drawer.isOpened()) {
this.refs.drawer.close();
return true;
}
@ -108,8 +105,8 @@ export default class ChannelDrawer extends PureComponent {
};
closeChannelDrawer = () => {
if (this.mounted) {
this.setState({openDrawer: false});
if (this.refs.drawer && this.refs.drawer.isOpened()) {
this.refs.drawer.close();
}
};
@ -124,13 +121,6 @@ export default class ChannelDrawer extends PureComponent {
InteractionManager.clearInteractionHandle(this.closeLeftHandle);
this.closeLeftHandle = null;
}
if (this.state.openDrawer && this.mounted) {
// The state doesn't get updated if you swipe to close
this.setState({
openDrawer: false
});
}
};
handleDrawerCloseStart = () => {
@ -154,13 +144,6 @@ export default class ChannelDrawer extends PureComponent {
if (!this.openLeftHandle) {
this.openLeftHandle = InteractionManager.createInteractionHandle();
}
if (!this.state.openDrawer && this.mounted) {
// The state doesn't get updated if you swipe to open
this.setState({
openDrawer: true
});
}
};
handleDrawerTween = (ratio) => {
@ -192,10 +175,8 @@ export default class ChannelDrawer extends PureComponent {
openChannelDrawer = () => {
this.props.blurPostTextBox();
if (this.mounted) {
this.setState({
openDrawer: true
});
if (this.refs.drawer && !this.refs.drawer.isOpened()) {
this.refs.drawer.open();
}
};
@ -220,10 +201,13 @@ export default class ChannelDrawer extends PureComponent {
InteractionManager.runAfterInteractions(() => {
handleSelectChannel(channel.id);
markChannelAsRead(channel.id, currentChannelId);
if (channel.id !== currentChannelId) {
viewChannel(currentChannelId);
}
requestAnimationFrame(() => {
// mark the channel as viewed after all the frame has flushed
markChannelAsRead(channel.id, currentChannelId);
if (channel.id !== currentChannelId) {
viewChannel(currentChannelId);
}
});
});
};
@ -279,7 +263,11 @@ export default class ChannelDrawer extends PureComponent {
onSearchEnds = () => {
//hack to update the drawer when the offset changes
const {isLandscape, isTablet} = this.props;
this.refs.drawer._syncAfterUpdate = true; //eslint-disable-line no-underscore-dangle
if (this.refs.drawer) {
this.refs.drawer._syncAfterUpdate = true; //eslint-disable-line no-underscore-dangle
}
let openDrawerOffset = DRAWER_INITIAL_OFFSET;
if (isLandscape || isTablet) {
openDrawerOffset = DRAWER_LANDSCAPE_OFFSET;
@ -288,7 +276,10 @@ export default class ChannelDrawer extends PureComponent {
};
onSearchStart = () => {
this.refs.drawer._syncAfterUpdate = true; //eslint-disable-line no-underscore-dangle
if (this.refs.drawer) {
this.refs.drawer._syncAfterUpdate = true; //eslint-disable-line no-underscore-dangle
}
this.setState({openDrawerOffset: 0});
};
@ -372,12 +363,11 @@ export default class ChannelDrawer extends PureComponent {
render() {
const {children} = this.props;
const {openDrawer, openDrawerOffset} = this.state;
const {openDrawerOffset} = this.state;
return (
<Drawer
ref='drawer'
open={openDrawer}
onOpenStart={this.handleDrawerOpenStart}
onOpen={this.handleDrawerOpen}
onClose={this.handleDrawerClose}

View file

@ -16,11 +16,10 @@ import {getTheme} from 'app/selectors/preferences';
import ChannelDrawer from './channel_drawer.js';
function mapStateToProps(state, ownProps) {
function mapStateToProps(state) {
const {currentUserId} = state.entities.users;
return {
...ownProps,
currentTeamId: getCurrentTeamId(state),
currentChannelId: getCurrentChannelId(state),
currentUserId,

View file

@ -29,4 +29,8 @@ export default class Drawer extends BaseDrawer {
return false;
};
isOpened = () => {
return this._open; // eslint-disable-line no-underscore-dangle
}
}

View file

@ -0,0 +1,16 @@
// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
import {connect} from 'react-redux';
import {getStatusBarHeight} from 'app/selectors/device';
import KeyboardLayout from './keyboard_layout';
function mapStateToProps(state) {
return {
statusBarHeight: getStatusBarHeight(state)
};
}
export default connect(mapStateToProps)(KeyboardLayout);

View file

@ -1,19 +1,24 @@
// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
import React from 'react';
import React, {PureComponent} from 'react';
import PropTypes from 'prop-types';
import {KeyboardAvoidingView, Platform, View} from 'react-native';
export default class KeyboardLayout extends React.PureComponent {
export default class KeyboardLayout extends PureComponent {
static propTypes = {
behaviour: PropTypes.string,
children: PropTypes.node,
keyboardVerticalOffset: PropTypes.number
keyboardVerticalOffset: PropTypes.number,
statusBarHeight: PropTypes.number
};
static defaultProps = {
keyboardVerticalOffset: 0
};
render() {
const {behaviour, children, keyboardVerticalOffset, ...otherProps} = this.props;
const {behaviour, children, keyboardVerticalOffset, statusBarHeight, ...otherProps} = this.props;
if (Platform.OS === 'android') {
return (
@ -23,10 +28,17 @@ export default class KeyboardLayout extends React.PureComponent {
);
}
let height = 0;
if (statusBarHeight > 20) {
height = (statusBarHeight - 20) + keyboardVerticalOffset;
} else {
height = keyboardVerticalOffset;
}
return (
<KeyboardAvoidingView
behaviour={behaviour}
keyboardVerticalOffset={keyboardVerticalOffset}
keyboardVerticalOffset={height}
{...otherProps}
>
{children}

View file

@ -8,7 +8,6 @@ import {addReaction, createPost, deletePost, removePost} from 'mattermost-redux/
import {getPost} from 'mattermost-redux/selectors/entities/posts';
import {getCurrentUserId, getCurrentUserRoles} from 'mattermost-redux/selectors/entities/users';
import {setPostTooltipVisible} from 'app/actions/views/channel';
import {getTheme} from 'app/selectors/preferences';
import Post from './post';
@ -19,7 +18,6 @@ function makeMapStateToProps() {
const {config, license} = state.entities.general;
const roles = getCurrentUserId(state) ? getCurrentUserRoles(state) : '';
const {tooltipVisible} = state.views.channel;
return {
...ownProps,
@ -29,8 +27,7 @@ function makeMapStateToProps() {
highlight: ownProps.post.highlight,
license,
roles,
theme: getTheme(state),
tooltipVisible
theme: getTheme(state)
};
};
}
@ -41,8 +38,7 @@ function mapDispatchToProps(dispatch) {
addReaction,
createPost,
deletePost,
removePost,
setPostTooltipVisible
removePost
}, dispatch)
};
}

View file

@ -25,21 +25,22 @@ import EventEmitter from 'mattermost-redux/utils/event_emitter';
import {canDeletePost, canEditPost, isPostEphemeral, isPostPendingOrFailed, isSystemMessage} from 'mattermost-redux/utils/post_utils';
import {isAdmin, isSystemAdmin} from 'mattermost-redux/utils/user_utils';
import {isToolTipShowing} from 'react-native-tooltip';
class Post extends PureComponent {
static propTypes = {
actions: PropTypes.shape({
addReaction: PropTypes.func.isRequired,
createPost: PropTypes.func.isRequired,
deletePost: PropTypes.func.isRequired,
removePost: PropTypes.func.isRequired,
setPostTooltipVisible: PropTypes.func.isRequired
removePost: PropTypes.func.isRequired
}).isRequired,
config: PropTypes.object.isRequired,
currentUserId: PropTypes.string.isRequired,
highlight: PropTypes.bool,
intl: intlShape.isRequired,
style: ViewPropTypes.style,
post: PropTypes.object.isRequired,
post: PropTypes.object,
renderReplies: PropTypes.bool,
isFirstReply: PropTypes.bool,
isLastReply: PropTypes.bool,
@ -50,7 +51,6 @@ class Post extends PureComponent {
roles: PropTypes.string,
shouldRenderReplyButton: PropTypes.bool,
showFullDate: PropTypes.bool,
tooltipVisible: PropTypes.bool,
theme: PropTypes.object.isRequired,
onPress: PropTypes.func,
onReply: PropTypes.func
@ -73,10 +73,12 @@ class Post extends PureComponent {
componentWillReceiveProps(nextProps) {
const {config, license, currentUserId, roles, post} = nextProps;
this.setState({
canEdit: canEditPost(config, license, currentUserId, post, this.editDisableAction),
canDelete: canDeletePost(config, license, currentUserId, post, isAdmin(roles), isSystemAdmin(roles))
});
if (post) {
this.setState({
canEdit: canEditPost(config, license, currentUserId, post, this.editDisableAction),
canDelete: canDeletePost(config, license, currentUserId, post, isAdmin(roles), isSystemAdmin(roles))
});
}
}
componentWillUnmount() {
@ -228,8 +230,8 @@ class Post extends PureComponent {
};
handlePress = () => {
const {post, onPress, tooltipVisible} = this.props;
if (!tooltipVisible) {
const {post, onPress} = this.props;
if (!isToolTipShowing) {
if (onPress && post.state !== Posts.POST_DELETED && !isSystemMessage(post) && !isPostPendingOrFailed(post)) {
preventDoubleTap(onPress, null, post);
} else if (isPostEphemeral(post)) {
@ -239,8 +241,8 @@ class Post extends PureComponent {
};
handleReply = () => {
const {post, onReply, tooltipVisible} = this.props;
if (!tooltipVisible && onReply) {
const {post, onReply} = this.props;
if (!isToolTipShowing && onReply) {
return preventDoubleTap(onReply, null, post);
}
@ -281,18 +283,17 @@ class Post extends PureComponent {
};
viewUserProfile = () => {
const {isSearchResult, tooltipVisible} = this.props;
const {isSearchResult} = this.props;
if (!isSearchResult && !tooltipVisible) {
if (!isSearchResult && !isToolTipShowing) {
preventDoubleTap(this.goToUserProfile, this);
}
};
toggleSelected = (selected, tooltip) => {
if (tooltip) {
this.props.actions.setPostTooltipVisible(selected);
toggleSelected = (selected) => {
if (!isToolTipShowing) {
this.setState({selected});
}
this.setState({selected});
};
render() {
@ -307,6 +308,11 @@ class Post extends PureComponent {
showFullDate,
theme
} = this.props;
if (!post) {
return null;
}
const style = getStyleSheet(theme);
const selected = this.state && this.state.selected ? style.selected : null;
const highlighted = highlight ? style.highlight : null;

View file

@ -1,7 +1,6 @@
// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
import {Posts} from 'mattermost-redux/constants';
import keyMirror from 'mattermost-redux/utils/key_mirror';
const ViewTypes = keyMirror({
@ -35,15 +34,15 @@ const ViewTypes = keyMirror({
SET_CHANNEL_LOADER: null,
SET_CHANNEL_REFRESHING: null,
SET_CHANNEL_RETRY_FAILED: null,
SET_CHANNEL_DISPLAY_NAME: null,
POST_TOOLTIP_VISIBLE: null,
SET_LAST_CHANNEL_FOR_TEAM: null,
GITLAB: null,
SAML: null,
SET_INITIAL_POST_VISIBILITY: null,
INCREASE_POST_VISIBILITY: null,
RECEIVED_FOCUSED_POST: null,
LOADING_POSTS: null,
@ -53,7 +52,7 @@ const ViewTypes = keyMirror({
export default {
...ViewTypes,
POST_VISIBILITY_CHUNK_SIZE: Posts.POST_CHUNK_SIZE / 2,
POST_VISIBILITY_CHUNK_SIZE: 15,
FEATURE_TOGGLE_PREFIX: 'feature_enabled_',
EMBED_PREVIEW: 'embed_preview'
};

View file

@ -4,8 +4,7 @@
import {combineReducers} from 'redux';
import {
ChannelTypes,
FileTypes,
PostTypes
FileTypes
} from 'mattermost-redux/action_types';
import {ViewTypes} from 'app/constants';
@ -185,9 +184,6 @@ function loading(state = false, action) {
function refreshing(state = false, action) {
switch (action.type) {
case PostTypes.GET_POSTS_SUCCESS:
case PostTypes.GET_POSTS_FAILURE:
return false;
case ViewTypes.SET_CHANNEL_REFRESHING:
return action.loading;
default:
@ -195,10 +191,10 @@ function refreshing(state = false, action) {
}
}
function tooltipVisible(state = false, action) {
function retryFailed(state = false, action) {
switch (action.type) {
case ViewTypes.POST_TOOLTIP_VISIBLE:
return action.visible;
case ViewTypes.SET_CHANNEL_RETRY_FAILED:
return action.failed;
default:
return state;
}
@ -206,7 +202,7 @@ function tooltipVisible(state = false, action) {
function postVisibility(state = {}, action) {
switch (action.type) {
case ChannelTypes.SELECT_CHANNEL: {
case ViewTypes.SET_INITIAL_POST_VISIBILITY: {
const nextState = {...state};
nextState[action.data] = ViewTypes.POST_VISIBILITY_CHUNK_SIZE;
return nextState;
@ -221,14 +217,6 @@ function postVisibility(state = {}, action) {
nextState[action.channelId] = ViewTypes.POST_VISIBILITY_CHUNK_SIZE;
return nextState;
}
case PostTypes.RECEIVED_POST: {
if (action.data && state[action.data.channel_id]) {
const nextState = {...state};
nextState[action.data.channel_id] += 1;
return nextState;
}
return state;
}
default:
return state;
}
@ -264,8 +252,8 @@ export default combineReducers({
drafts,
loading,
refreshing,
tooltipVisible,
postVisibility,
loadingPosts,
lastGetPosts
lastGetPosts,
retryFailed
});

View file

@ -50,7 +50,6 @@ class Channel extends PureComponent {
currentChannelId: PropTypes.string,
theme: PropTypes.object.isRequired,
webSocketRequest: PropTypes.object,
statusBarHeight: PropTypes.number,
channelsRequestStatus: PropTypes.string
};
@ -173,7 +172,6 @@ class Channel extends PureComponent {
currentChannelId,
intl,
navigator,
statusBarHeight,
theme
} = this.props;
@ -195,11 +193,6 @@ class Channel extends PureComponent {
);
}
let height = 0;
if (statusBarHeight > 20) {
height = statusBarHeight - 20;
}
return (
<ChannelDrawer
blurPostTextBox={this.blurPostTextBox}
@ -220,7 +213,6 @@ class Channel extends PureComponent {
<KeyboardLayout
behavior='padding'
style={style.keyboardLayout}
keyboardVerticalOffset={height}
>
<View style={style.postList}>
<ChannelPostList

View file

@ -47,7 +47,7 @@ class ChannelPostList extends PureComponent {
static defaultProps = {
posts: [],
loadingPosts: false,
postVisibility: 30
postVisibility: 15
};
constructor(props) {
@ -61,19 +61,11 @@ class ChannelPostList extends PureComponent {
}
componentDidMount() {
const {channelId} = this.props;
this.mounted = true;
this.loadPosts(channelId);
}
componentWillReceiveProps(nextProps) {
const {channelId: currentChannelId} = this.props;
const {channelId: nextChannelId, channelRefreshingFailed: nextChannelRefreshingFailed, posts: nextPosts} = nextProps;
if (currentChannelId !== nextChannelId) {
// Load the posts when the channel actually changes
this.loadPosts(nextChannelId);
}
const {channelRefreshingFailed: nextChannelRefreshingFailed, posts: nextPosts} = nextProps;
if (nextChannelRefreshingFailed && nextPosts.length) {
this.toggleRetryMessage();
@ -155,12 +147,9 @@ class ChannelPostList extends PureComponent {
}
};
loadPosts = (channelId) => {
this.props.actions.loadPostsIfNecessaryWithRetry(channelId);
};
loadPostsRetry = () => {
this.loadPosts(this.props.channelId);
const {actions, channelId} = this.props;
actions.loadPostsIfNecessaryWithRetry(channelId);
};
render() {

View file

@ -11,7 +11,6 @@ import {
} from 'app/actions/views/channel';
import {connection} from 'app/actions/device';
import {selectFirstAvailableTeam} from 'app/actions/views/select_team';
import {getStatusBarHeight} from 'app/selectors/device';
import {getTheme} from 'app/selectors/preferences';
import {viewChannel, markChannelAsRead} from 'mattermost-redux/actions/channels';
@ -36,7 +35,6 @@ function mapStateToProps(state, ownProps) {
currentChannelId: getCurrentChannelId(state),
theme: getTheme(state),
webSocketRequest: websocket,
statusBarHeight: getStatusBarHeight(state),
channelsRequestStatus: channelsRequest.status
};
}

View file

@ -1,25 +1,16 @@
// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
import {bindActionCreators} from 'redux';
import {connect} from 'react-redux';
import {getTheme} from 'app/selectors/preferences';
import Code from './code';
function mapStateToProps(state, ownProps) {
function mapStateToProps(state) {
return {
theme: getTheme(state),
...ownProps
theme: getTheme(state)
};
}
function mapDispatchToProps(dispatch) {
return {
actions: bindActionCreators({
}, dispatch)
};
}
export default connect(mapStateToProps, mapDispatchToProps)(Code);
export default connect(mapStateToProps)(Code);

View file

@ -4,7 +4,6 @@
import {bindActionCreators} from 'redux';
import {connect} from 'react-redux';
import {getStatusBarHeight} from 'app/selectors/device';
import {getTheme} from 'app/selectors/preferences';
import {selectPost} from 'mattermost-redux/actions/posts';
@ -22,12 +21,10 @@ function makeMapStateToProps() {
const posts = getPostsForThread(state, ownProps);
return {
...ownProps,
channelId: ownProps.channelId,
myMember: getMyCurrentChannelMembership(state),
rootId: ownProps.rootId,
posts,
statusBarHeight: getStatusBarHeight(state),
theme: getTheme(state)
};
};

View file

@ -20,8 +20,7 @@ export default class Thread extends PureComponent {
myMember: PropTypes.object.isRequired,
rootId: PropTypes.string.isRequired,
theme: PropTypes.object.isRequired,
posts: PropTypes.array.isRequired,
statusBarHeight: PropTypes.number
posts: PropTypes.array.isRequired
};
state = {};
@ -43,21 +42,15 @@ export default class Thread extends PureComponent {
navigator,
posts,
rootId,
statusBarHeight,
theme
} = this.props;
const style = getStyle(theme);
let height = 0;
if (statusBarHeight > 20) {
height = statusBarHeight - 20;
}
return (
<KeyboardLayout
behavior='padding'
style={style.container}
keyboardVerticalOffset={65 + height}
keyboardVerticalOffset={65}
>
<StatusBar/>
<PostList

View file

@ -48,7 +48,7 @@ export default function configureAppStore(initialState) {
['typing']
);
const channelViewBlackList = {loading: true, refreshing: true, tooltipVisible: true, loadingPosts: true};
const channelViewBlackList = {loading: true, refreshing: true, loadingPosts: true, postVisibility: true, retryFailed: true};
const channelViewBlackListFilter = createTransform(
(inboundState) => {
const channel = {};

View file

@ -5012,8 +5012,8 @@ react-native-svg@5.4.1:
lodash "^4.16.6"
react-native-tooltip@enahum/react-native-tooltip:
version "5.0.0"
resolved "https://codeload.github.com/enahum/react-native-tooltip/tar.gz/97d58d19636df8d8df66d6b5737154c9fab727c8"
version "5.1.1"
resolved "https://codeload.github.com/enahum/react-native-tooltip/tar.gz/eb47f93a728813b58c01f23ec84f0b8b1bd70bd0"
react-native-vector-icons@4.3.0:
version "4.3.0"