From c9fd1ac97db9c4873e71a29e7f323f903c26f523 Mon Sep 17 00:00:00 2001 From: enahum Date: Mon, 13 Nov 2017 13:55:26 -0300 Subject: [PATCH] Scroll to search result in preview (#1129) * Scroll to search result in preview * Fix uni test * Feedback review * Remove listeners for mattermostManaged * Remove specific event listeners for mattermostManaged --- app/components/post_list/post_list.js | 22 +++++++++---- .../mattermost-managed.android.js | 19 ++++++++++-- .../mattermost-managed.ios.js | 8 +++++ app/screens/search/search.js | 26 +++++++++++++++- app/selectors/post_list.js | 5 +-- test/app/selectors/post_list.test.js | 31 ++++++++++--------- 6 files changed, 85 insertions(+), 26 deletions(-) diff --git a/app/components/post_list/post_list.js b/app/components/post_list/post_list.js index bec49293c..956340356 100644 --- a/app/components/post_list/post_list.js +++ b/app/components/post_list/post_list.js @@ -4,6 +4,7 @@ import React, {PureComponent} from 'react'; import PropTypes from 'prop-types'; import { + InteractionManager, StyleSheet, FlatList } from 'react-native'; @@ -53,7 +54,7 @@ export default class PostList extends PureComponent { } componentWillMount() { - mattermostManaged.addEventListener('change', this.setManagedConfig); + this.listenerId = mattermostManaged.addEventListener('change', this.setManagedConfig); } componentDidMount() { @@ -63,13 +64,17 @@ export default class PostList extends PureComponent { componentDidUpdate(prevProps) { const initialPosts = !prevProps.postIds.length && prevProps.postIds !== this.props.postIds; - if ((prevProps.channelId !== this.props.channelId || initialPosts) && this.refs.list) { + if ((prevProps.channelId !== this.props.channelId || initialPosts || this.props.isSearchResult) && this.refs.list) { this.scrollList(); } } + componentWillUnmount() { + mattermostManaged.removeEventListener(this.listenerId); + } + scrollList = () => { - requestAnimationFrame(() => { + InteractionManager.runAfterInteractions(() => { if (this.props.postIds.length && this.newMessagesIndex !== -1) { this.refs.list.scrollToIndex({index: this.newMessagesIndex, viewPosition: 1, viewOffset: -10, animated: true}); this.newMessagesIndex = -1; @@ -131,7 +136,7 @@ export default class PostList extends PureComponent { const previousPostId = index < this.props.postIds.length - 1 ? this.props.postIds[index + 1] : null; const nextPostId = index > 0 ? this.props.postIds[index - 1] : null; - return this.renderPost(postId, previousPostId, nextPostId); + return this.renderPost(postId, previousPostId, nextPostId, index); }; renderDateHeader = (date) => { @@ -143,7 +148,7 @@ export default class PostList extends PureComponent { ); }; - renderPost = (postId, previousPostId, nextPostId) => { + renderPost = (postId, previousPostId, nextPostId, index) => { const { highlightPostId, isSearchResult, @@ -154,12 +159,17 @@ export default class PostList extends PureComponent { } = this.props; const {managedConfig} = this.state; + const highlight = highlightPostId === postId; + if (highlight) { + this.newMessagesIndex = index; + } + return ( { - DeviceEventEmitter.addListener(name, (config) => { + const listener = DeviceEventEmitter.addListener(name, (config) => { localConfig = config; if (callback && typeof callback === 'function') { callback(config); } }); + + listeners.push(listener); + return listener; + }, + clearListeners: () => { + listeners.forEach((listener) => { + listener.remove(); + }); + }, + removeEventListener: (listenerId) => { + const index = listeners.findIndex((listener) => listener === listenerId); + if (index !== -1) { + listenerId.remove(); + listeners.splice(index, 1); + } }, - clearListeners: () => true, authenticate: LocalAuth.authenticate, blurAppScreen: MattermostManaged.blurAppScreen, getConfig: MattermostManaged.getConfig, diff --git a/app/mattermost_managed/mattermost-managed.ios.js b/app/mattermost_managed/mattermost-managed.ios.js index 0fd9643ae..7e7eb5b44 100644 --- a/app/mattermost_managed/mattermost-managed.ios.js +++ b/app/mattermost_managed/mattermost-managed.ios.js @@ -19,12 +19,20 @@ export default { }); listeners.push(listener); + return listener; }, clearListeners: () => { listeners.forEach((listener) => { listener.remove(); }); }, + removeEventListener: (listenerId) => { + const index = listeners.findIndex((listener) => listener === listenerId); + if (index !== -1) { + listenerId.remove(); + listeners.splice(index, 1); + } + }, authenticate: LocalAuth.authenticate, blurAppScreen: BlurAppScreen.enabled, getConfig: MattermostManaged.getConfig, diff --git a/app/screens/search/search.js b/app/screens/search/search.js index 28fe618d2..a55b7bf33 100644 --- a/app/screens/search/search.js +++ b/app/screens/search/search.js @@ -26,6 +26,7 @@ import PostListRetry from 'app/components/post_list_retry'; import SearchBar from 'app/components/search_bar'; import SearchPreview from 'app/components/search_preview'; import StatusBar from 'app/components/status_bar'; +import mattermostManaged from 'app/mattermost_managed'; import {preventDoubleTap} from 'app/utils/tap'; import {changeOpacity, makeStyleSheetFromTheme} from 'app/utils/theme'; @@ -73,11 +74,17 @@ class Search extends PureComponent { isFocused: true, postId: null, preview: false, - value: '' + value: '', + managedConfig: {} }; } + componentWillMount() { + this.listenerId = mattermostManaged.addEventListener('change', this.setManagedConfig); + } + componentDidMount() { + this.setManagedConfig(); if (this.refs.searchBar) { this.refs.searchBar.focus(); } @@ -99,6 +106,10 @@ class Search extends PureComponent { } } + componentWillUnmount() { + mattermostManaged.removeEventListener(this.listenerId); + } + attachAutocomplete = (c) => { this.autocomplete = c; }; @@ -247,6 +258,7 @@ class Search extends PureComponent { renderPost = ({item, index}) => { const {postIds, theme} = this.props; + const {managedConfig} = this.state; const style = getStyleFromTheme(theme); if (item.id) { @@ -270,6 +282,7 @@ class Search extends PureComponent { previewPost={this.previewPost} goToThread={this.goToThread} navigator={this.props.navigator} + managedConfig={managedConfig} /> {separator} @@ -355,6 +368,17 @@ class Search extends PureComponent { this.search(this.state.value.trim()); }; + setManagedConfig = async (config) => { + let nextConfig = config; + if (!nextConfig) { + nextConfig = await mattermostManaged.getLocalConfig(); + } + + this.setState({ + managedConfig: nextConfig + }); + }; + scrollToTop = () => { if (this.refs.list) { this.refs.list._wrapperListRef.getListRef().scrollToOffset({ //eslint-disable-line no-underscore-dangle diff --git a/app/selectors/post_list.js b/app/selectors/post_list.js index b21e1556d..94e0187ff 100644 --- a/app/selectors/post_list.js +++ b/app/selectors/post_list.js @@ -25,9 +25,10 @@ export function makePreparePostIdsForPostList() { return createIdsSelector( (state, props) => getMyPosts(state, props.postIds), (state, props) => props.lastViewedAt, + (state, props) => props.indicateNewMessages, getCurrentUserId, shouldShowJoinLeaveMessages, - (posts, lastViewedAt, currentUserId, showJoinLeave) => { + (posts, lastViewedAt, indicateNewMessages, currentUserId, showJoinLeave) => { if (posts.length === 0) { return []; } @@ -63,7 +64,7 @@ export function makePreparePostIdsForPostList() { // Only add the new messages line if a lastViewedAt time is set const postIsUnread = post.create_at > lastViewedAt && post.user_id !== currentUserId; - if (lastViewedAt !== null && !addedNewMessagesIndicator && postIsUnread) { + if (lastViewedAt !== null && !addedNewMessagesIndicator && postIsUnread && indicateNewMessages) { out.push(START_OF_NEW_MESSAGES); addedNewMessagesIndicator = true; } diff --git a/test/app/selectors/post_list.test.js b/test/app/selectors/post_list.test.js index ae047e836..18ddcd564 100644 --- a/test/app/selectors/post_list.test.js +++ b/test/app/selectors/post_list.test.js @@ -35,9 +35,10 @@ describe('Selectors.PostList', () => { }; const lastViewedAt = Number.POSITIVE_INFINITY; const postIds = ['1002', '1001']; + const indicateNewMessages = true; // Defaults to show post - let now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + let now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages}); assert.deepEqual(removeDateLines(now), ['1002', '1001']); // Show join/leave posts @@ -59,7 +60,7 @@ describe('Selectors.PostList', () => { } }; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages}); assert.deepEqual(removeDateLines(now), ['1002', '1001']); // Hide join/leave posts @@ -81,7 +82,7 @@ describe('Selectors.PostList', () => { } }; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages}); assert.deepEqual(removeDateLines(now), ['1001']); }); @@ -109,11 +110,11 @@ describe('Selectors.PostList', () => { const postIds = ['1010', '1005', '1000']; // Remember that we list the posts backwards // Show new messages indicator before all posts - let now = preparePostIdsForPostList(state, {postIds, lastViewedAt: 0}); + let now = preparePostIdsForPostList(state, {postIds, lastViewedAt: 0, indicateNewMessages: true}); assert.deepEqual(removeDateLines(now), ['1010', '1005', '1000', START_OF_NEW_MESSAGES]); // Show indicator between posts - now = preparePostIdsForPostList(state, {postIds, lastViewedAt: 1003}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt: 1003, indicateNewMessages: true}); assert.deepEqual(removeDateLines(now), ['1010', '1005', START_OF_NEW_MESSAGES, '1000']); // Don't show indicator when all posts are read @@ -156,12 +157,12 @@ describe('Selectors.PostList', () => { let postIds = ['1006', '1004', '1003', '1001']; let lastViewedAt = initialPosts['1001'].create_at + 1; - let now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + let now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.deepEqual(removeDateLines(now), ['1006', '1004', '1003', START_OF_NEW_MESSAGES, '1001']); // No changes let prev = now; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.equal(now, prev); assert.deepEqual(removeDateLines(now), ['1006', '1004', '1003', START_OF_NEW_MESSAGES, '1001']); @@ -169,7 +170,7 @@ describe('Selectors.PostList', () => { lastViewedAt = initialPosts['1001'].create_at + 2; prev = now; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.equal(now, prev); assert.deepEqual(removeDateLines(now), ['1006', '1004', '1003', START_OF_NEW_MESSAGES, '1001']); @@ -177,12 +178,12 @@ describe('Selectors.PostList', () => { lastViewedAt += initialPosts['1003'].create_at + 1; prev = now; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.notEqual(now, prev); assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']); prev = now; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.equal(now, prev); assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']); @@ -190,7 +191,7 @@ describe('Selectors.PostList', () => { postIds = [...postIds]; prev = now; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.equal(now, prev); assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']); @@ -210,7 +211,7 @@ describe('Selectors.PostList', () => { }; prev = now; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.equal(now, prev); assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']); @@ -230,7 +231,7 @@ describe('Selectors.PostList', () => { }; prev = now; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.equal(now, prev); assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']); @@ -254,12 +255,12 @@ describe('Selectors.PostList', () => { }; prev = now; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.notEqual(now, prev); assert.deepEqual(removeDateLines(now), ['1004', START_OF_NEW_MESSAGES, '1003', '1001']); prev = now; - now = preparePostIdsForPostList(state, {postIds, lastViewedAt}); + now = preparePostIdsForPostList(state, {postIds, lastViewedAt, indicateNewMessages: true}); assert.equal(now, prev); assert.deepEqual(removeDateLines(now), ['1004', START_OF_NEW_MESSAGES, '1003', '1001']); });