From b7970c3a342874d5c798ded3c82bc76339330371 Mon Sep 17 00:00:00 2001 From: Mattermost Build Date: Fri, 7 Feb 2020 02:46:09 +0100 Subject: [PATCH] Remove ChannelPeek (#3897) Introduced large number of unnecessary re-renders when opening channels, and ideally should not be a part of the channel switching/opening code path. Although this was discovered while trying to investigate an [Android-specific issue](https://mattermost.atlassian.net/browse/MM-22253), this extra code path made it difficult to see what Android is potentially doing differently than iOS. Functionality originally introduced in #1203. Conversation for removal is [here](https://community.mattermost.com/core/pl/hfcogf6pr7rw8k3ryq14c69c7e) --- .../__snapshots__/channel_item.test.js.snap | 1388 ++++++++--------- .../channel_item/channel_item.js | 66 +- .../channel_item/channel_item.test.js | 4 +- .../main/channels_list/channels_list.js | 4 - .../filtered_list/filtered_list.js | 2 - .../sidebars/main/channels_list/list/list.js | 4 +- app/components/sidebars/main/main_sidebar.js | 3 - app/screens/channel/channel.ios.js | 11 +- app/screens/channel/channel_base.js | 3 +- app/screens/channel_peek/channel_peek.js | 103 -- app/screens/channel_peek/index.js | 37 - app/screens/index.js | 1 - assets/base/i18n/en.json | 1 - 13 files changed, 709 insertions(+), 918 deletions(-) delete mode 100644 app/screens/channel_peek/channel_peek.js delete mode 100644 app/screens/channel_peek/index.js diff --git a/app/components/sidebars/main/channels_list/channel_item/__snapshots__/channel_item.test.js.snap b/app/components/sidebars/main/channels_list/channel_item/__snapshots__/channel_item.test.js.snap index 2a65161cd..3d3462503 100644 --- a/app/components/sidebars/main/channels_list/channel_item/__snapshots__/channel_item.test.js.snap +++ b/app/components/sidebars/main/channels_list/channel_item/__snapshots__/channel_item.test.js.snap @@ -1,669 +1,657 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`ChannelItem should match snapshot 1`] = ` - - + - + - - - display_name - - + display_name + - - + + `; exports[`ChannelItem should match snapshot for current user i.e currentUser (you) 1`] = ` - - + + - - - - - {displayname} (you) - - + {displayname} (you) + - - + + `; exports[`ChannelItem should match snapshot for current user i.e currentUser (you) when isSearchResult 1`] = ` - - + + - - - - - {displayname} (you) - - + {displayname} (you) + - - + + `; exports[`ChannelItem should match snapshot for deactivated user and is currentChannel 1`] = ` - - + + - - - - - display_name - - + display_name + - - + + `; exports[`ChannelItem should match snapshot for deactivated user and is searchResult 1`] = ` - - + - + - - - display_name - - + display_name + - - + + `; exports[`ChannelItem should match snapshot for deactivated user and not searchResults or currentChannel 1`] = ` - - + - + - - - display_name - - + display_name + - - + + `; exports[`ChannelItem should match snapshot for no displayName 1`] = `null`; @@ -671,246 +659,242 @@ exports[`ChannelItem should match snapshot for no displayName 1`] = `null`; exports[`ChannelItem should match snapshot for showUnreadForMsgs 1`] = `null`; exports[`ChannelItem should match snapshot with draft 1`] = ` - - + - + - - - display_name - - + display_name + - - + + `; exports[`ChannelItem should match snapshot with mentions and muted 1`] = ` - - + - + - + - - display_name - - - + } + /> - - + + `; diff --git a/app/components/sidebars/main/channels_list/channel_item/channel_item.js b/app/components/sidebars/main/channels_list/channel_item/channel_item.js index 9d42e742f..476982370 100644 --- a/app/components/sidebars/main/channels_list/channel_item/channel_item.js +++ b/app/components/sidebars/main/channels_list/channel_item/channel_item.js @@ -4,13 +4,11 @@ import React, {PureComponent} from 'react'; import PropTypes from 'prop-types'; import { - Animated, TouchableHighlight, Text, View, } from 'react-native'; import {intlShape} from 'react-intl'; -import {Navigation} from 'react-native-navigation'; import {General} from 'mattermost-redux/constants'; import {paddingLeft as padding} from 'app/components/safe_area_view/iphone_x_spacing'; @@ -19,8 +17,6 @@ import ChannelIcon from 'app/components/channel_icon'; import {preventDoubleTap} from 'app/utils/tap'; import {changeOpacity, makeStyleSheetFromTheme} from 'app/utils/theme'; -const {View: AnimatedView} = Animated; - export default class ChannelItem extends PureComponent { static propTypes = { channelId: PropTypes.string.isRequired, @@ -40,7 +36,6 @@ export default class ChannelItem extends PureComponent { unreadMsgs: PropTypes.number.isRequired, isSearchResult: PropTypes.bool, isBot: PropTypes.bool.isRequired, - previewChannel: PropTypes.func, isLandscape: PropTypes.bool.isRequired, }; @@ -61,27 +56,6 @@ export default class ChannelItem extends PureComponent { }); }); - onPreview = ({reactTag}) => { - const {channelId, previewChannel} = this.props; - if (previewChannel) { - const {intl} = this.context; - const passProps = { - channelId, - }; - const options = { - preview: { - reactTag, - actions: [{ - id: 'action-mark-as-read', - title: intl.formatMessage({id: 'mobile.channel.markAsRead', defaultMessage: 'Mark As Read'}), - }], - }, - }; - - previewChannel(passProps, options); - } - }; - showChannelAsUnread = () => { return this.props.mentions > 0 || (this.props.unreadMsgs > 0 && this.props.showUnreadForMsgs); }; @@ -192,29 +166,25 @@ export default class ChannelItem extends PureComponent { ); return ( - - - - {extraBorder} - - {icon} - - {channelDisplayName} - - {badge} - + + + {extraBorder} + + {icon} + + {channelDisplayName} + + {badge} - - + + ); } } diff --git a/app/components/sidebars/main/channels_list/channel_item/channel_item.test.js b/app/components/sidebars/main/channels_list/channel_item/channel_item.test.js index c6dca8ec7..9f825d7f5 100644 --- a/app/components/sidebars/main/channels_list/channel_item/channel_item.test.js +++ b/app/components/sidebars/main/channels_list/channel_item/channel_item.test.js @@ -3,7 +3,7 @@ import React from 'react'; import {shallow} from 'enzyme'; -import {Navigation} from 'react-native-navigation'; +import {TouchableHighlight} from 'react-native'; import Preferences from 'mattermost-redux/constants/preferences'; @@ -217,7 +217,7 @@ describe('ChannelItem', () => { {context: {intl: {formatMessage: jest.fn()}}}, ); - wrapper.find(Navigation.TouchablePreview).simulate('press'); + wrapper.find(TouchableHighlight).simulate('press'); jest.runAllTimers(); const expectedChannelParams = {id: baseProps.channelId, display_name: baseProps.displayName, fake: channel.fake, type: channel.type}; diff --git a/app/components/sidebars/main/channels_list/channels_list.js b/app/components/sidebars/main/channels_list/channels_list.js index 1d1f74e2a..10b38e5e9 100644 --- a/app/components/sidebars/main/channels_list/channels_list.js +++ b/app/components/sidebars/main/channels_list/channels_list.js @@ -34,7 +34,6 @@ export default class ChannelsList extends PureComponent { onShowTeams: PropTypes.func.isRequired, theme: PropTypes.object.isRequired, drawerOpened: PropTypes.bool, - previewChannel: PropTypes.func, isLandscape: PropTypes.bool.isRequired, }; @@ -98,7 +97,6 @@ export default class ChannelsList extends PureComponent { const { onShowTeams, theme, - previewChannel, isLandscape, } = this.props; @@ -112,7 +110,6 @@ export default class ChannelsList extends PureComponent { onSelectChannel={this.onSelectChannel} styles={styles} term={term} - previewChannel={previewChannel} /> ); } else { @@ -120,7 +117,6 @@ export default class ChannelsList extends PureComponent { ); } diff --git a/app/components/sidebars/main/channels_list/filtered_list/filtered_list.js b/app/components/sidebars/main/channels_list/filtered_list/filtered_list.js index 12b818906..094226964 100644 --- a/app/components/sidebars/main/channels_list/filtered_list/filtered_list.js +++ b/app/components/sidebars/main/channels_list/filtered_list/filtered_list.js @@ -50,7 +50,6 @@ class FilteredList extends Component { styles: PropTypes.object.isRequired, term: PropTypes.string, theme: PropTypes.object.isRequired, - previewChannel: PropTypes.func, isLandscape: PropTypes.bool.isRequired, }; @@ -350,7 +349,6 @@ class FilteredList extends Component { isUnread={item.isUnread} mentions={0} onSelectChannel={this.onSelectChannel} - previewChannel={this.props.previewChannel} /> ); }; diff --git a/app/components/sidebars/main/channels_list/list/list.js b/app/components/sidebars/main/channels_list/list/list.js index cf8013531..84968377f 100644 --- a/app/components/sidebars/main/channels_list/list/list.js +++ b/app/components/sidebars/main/channels_list/list/list.js @@ -48,7 +48,6 @@ export default class List extends PureComponent { styles: PropTypes.object.isRequired, theme: PropTypes.object.isRequired, orderedChannelIds: PropTypes.array.isRequired, - previewChannel: PropTypes.func, isLandscape: PropTypes.bool.isRequired, }; @@ -309,7 +308,7 @@ export default class List extends PureComponent { }; renderItem = ({item}) => { - const {favoriteChannelIds, unreadChannelIds, previewChannel} = this.props; + const {favoriteChannelIds, unreadChannelIds} = this.props; return ( ); }; diff --git a/app/components/sidebars/main/main_sidebar.js b/app/components/sidebars/main/main_sidebar.js index f0f02a764..28bc96b92 100644 --- a/app/components/sidebars/main/main_sidebar.js +++ b/app/components/sidebars/main/main_sidebar.js @@ -45,7 +45,6 @@ export default class ChannelSidebar extends Component { currentUserId: PropTypes.string.isRequired, teamsCount: PropTypes.number.isRequired, theme: PropTypes.object.isRequired, - previewChannel: PropTypes.func, }; static contextTypes = { @@ -318,7 +317,6 @@ export default class ChannelSidebar extends Component { const { teamsCount, theme, - previewChannel, } = this.props; const { @@ -372,7 +370,6 @@ export default class ChannelSidebar extends Component { onSearchEnds={this.onSearchEnds} theme={theme} drawerOpened={this.state.drawerOpened} - previewChannel={previewChannel} /> , ); diff --git a/app/screens/channel/channel.ios.js b/app/screens/channel/channel.ios.js index f2b016ba9..9a92542fe 100644 --- a/app/screens/channel/channel.ios.js +++ b/app/screens/channel/channel.ios.js @@ -12,7 +12,6 @@ import PostTextbox from 'app/components/post_textbox'; import SafeAreaView from 'app/components/safe_area_view'; import StatusBar from 'app/components/status_bar'; import {DeviceTypes} from 'app/constants'; -import {peek} from 'app/actions/navigation'; import LocalConfig from 'assets/config'; @@ -25,14 +24,6 @@ const CHANNEL_POST_TEXTBOX_CURSOR_CHANGE = 'onChannelTextBoxCursorChange'; const CHANNEL_POST_TEXTBOX_VALUE_CHANGE = 'onChannelTextBoxValueChange'; export default class ChannelIOS extends ChannelBase { - previewChannel = (passProps, options) => { - const screen = 'ChannelPeek'; - - peek(screen, passProps, options); - }; - - optionalProps = {previewChannel: this.previewChannel}; - render() { const {height} = Dimensions.get('window'); const {currentChannelId} = this.props; @@ -84,6 +75,6 @@ export default class ChannelIOS extends ChannelBase { ); - return this.renderChannel(drawerContent, this.optionalProps); + return this.renderChannel(drawerContent); } } diff --git a/app/screens/channel/channel_base.js b/app/screens/channel/channel_base.js index 45eab4c36..1c7461d38 100644 --- a/app/screens/channel/channel_base.js +++ b/app/screens/channel/channel_base.js @@ -268,7 +268,7 @@ export default class ChannelBase extends PureComponent { } }; - renderChannel(drawerContent, optionalProps = {}) { + renderChannel(drawerContent) { const { channelsRequestFailed, currentChannelId, @@ -318,7 +318,6 @@ export default class ChannelBase extends PureComponent { { - return props.postIds?.slice(0, 15) || []; - }; - - render() { - const { - channelId, - currentUserId, - lastViewedAt, - theme, - } = this.props; - - const {visiblePostIds} = this.state; - const style = getStyle(theme); - - return ( - - - - ); - } -} - -const getStyle = makeStyleSheetFromTheme((theme) => { - return { - container: { - flex: 1, - backgroundColor: theme.centerChannelBg, - }, - }; -}); diff --git a/app/screens/channel_peek/index.js b/app/screens/channel_peek/index.js deleted file mode 100644 index 415f1c0bd..000000000 --- a/app/screens/channel_peek/index.js +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -import {bindActionCreators} from 'redux'; -import {connect} from 'react-redux'; - -import {getPostIdsInChannel} from 'mattermost-redux/selectors/entities/posts'; -import {getMyChannelMember} from 'mattermost-redux/selectors/entities/channels'; -import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users'; -import {loadPostsIfNecessaryWithRetry, markChannelViewedAndRead} from 'app/actions/views/channel'; -import {getTheme} from 'mattermost-redux/selectors/entities/preferences'; - -import ChannelPeek from './channel_peek'; - -function mapStateToProps(state, ownProps) { - const channelId = ownProps.channelId; - const myMember = getMyChannelMember(state, channelId); - - return { - channelId, - currentUserId: getCurrentUserId(state), - postIds: getPostIdsInChannel(state, channelId), - lastViewedAt: myMember && myMember.last_viewed_at, - theme: getTheme(state), - }; -} - -function mapDispatchToProps(dispatch) { - return { - actions: bindActionCreators({ - loadPostsIfNecessaryWithRetry, - markChannelViewedAndRead, - }, dispatch), - }; -} - -export default connect(mapStateToProps, mapDispatchToProps)(ChannelPeek); diff --git a/app/screens/index.js b/app/screens/index.js index a4f5b62a5..2ff1f5b90 100644 --- a/app/screens/index.js +++ b/app/screens/index.js @@ -26,7 +26,6 @@ export function registerScreens(store, Provider) { Navigation.registerComponent('ChannelAddMembers', () => wrapper(require('app/screens/channel_add_members').default), () => require('app/screens/channel_add_members').default); Navigation.registerComponent('ChannelInfo', () => wrapper(require('app/screens/channel_info').default), () => require('app/screens/channel_info').default); Navigation.registerComponent('ChannelMembers', () => wrapper(require('app/screens/channel_members').default), () => require('app/screens/channel_members').default); - Navigation.registerComponent('ChannelPeek', () => wrapper(require('app/screens/channel_peek').default), () => require('app/screens/channel_peek').default); Navigation.registerComponent('ClientUpgrade', () => wrapper(require('app/screens/client_upgrade').default), () => require('app/screens/client_upgrade').default); Navigation.registerComponent('ClockDisplaySettings', () => wrapper(require('app/screens/settings/clock_display').default), () => require('app/screens/settings/clock_display').default); Navigation.registerComponent('Code', () => wrapper(require('app/screens/code').default), () => require('app/screens/code').default); diff --git a/assets/base/i18n/en.json b/assets/base/i18n/en.json index 1c0d7fe0a..84d0639cc 100644 --- a/assets/base/i18n/en.json +++ b/assets/base/i18n/en.json @@ -183,7 +183,6 @@ "mobile.channel_list.not_member": "NOT A MEMBER", "mobile.channel_list.unreads": "UNREADS", "mobile.channel_members.add_members_alert": "You must select at least one member to add to the channel.", - "mobile.channel.markAsRead": "Mark As Read", "mobile.client_upgrade": "Update App", "mobile.client_upgrade.can_upgrade_subtitle": "A new version is available for download.", "mobile.client_upgrade.can_upgrade_title": "Update Available",