diff --git a/src/account/__tests__/accountSelectors-test.js b/src/account/__tests__/accountSelectors-test.js index 5b2feae3c57..8a592dbfe3b 100644 --- a/src/account/__tests__/accountSelectors-test.js +++ b/src/account/__tests__/accountSelectors-test.js @@ -12,21 +12,6 @@ test('getAuth returns an empty object when no accounts', () => { }); }); -test('getAuth returns only relevant fields', () => { - const state = { - account: [{ - realm: 'https://realm1.com', - otherField: 'someValue', - }], - }; - const auth = getAuth(state); - expect(auth).toEqual({ - realm: 'https://realm1.com', - email: undefined, - apiKey: undefined, - }); -}); - test('getAuth returns the auth information from the first account', () => { const state = { account: [ diff --git a/src/account/accountSelectors.js b/src/account/accountSelectors.js index 0279830391c..fd07288e337 100644 --- a/src/account/accountSelectors.js +++ b/src/account/accountSelectors.js @@ -12,9 +12,7 @@ export const getAuth = (state) => { }; } - return { - apiKey: account.apiKey, - email: account.email, - realm: account.realm, - }; + // Returning a copy here (instead of the original object) + // causes all components in the app to re-render + return account; }; diff --git a/src/api/apiFetch.js b/src/api/apiFetch.js index 346fa01f8fb..2116a1f89ca 100644 --- a/src/api/apiFetch.js +++ b/src/api/apiFetch.js @@ -35,7 +35,7 @@ export const apiCall = async ( try { if (!noTimeout) { timeout = setTimeout(() => { - // TODO (nw): the throw below does not get caught and crashes the app + // TODO: the throw below does not get caught and crashes the app // throw Error(`Request timed out @ ${route}`); // send APP_OFFLINE }, 5000); diff --git a/src/chat/Chat.js b/src/chat/Chat.js index 1d5bd31a8f2..60d8cca8ebb 100644 --- a/src/chat/Chat.js +++ b/src/chat/Chat.js @@ -12,21 +12,23 @@ import ComposeBox from '../compose/ComposeBox'; export default class MainScreen extends React.Component { render() { - const { auth, messages, narrow, isFetching, subscriptions, - isOnline, twentyFourHourTime, doNarrow, fetchOlder } = this.props; + const { auth, messages, narrow, fetching, caughtUp, subscriptions, + isOnline, twentyFourHourTime, doNarrow, fetchOlder, fetchNewer } = this.props; return ( {!isOnline && } - {messages.length === 0 && !isFetching && } + {messages.length === 0 && caughtUp[0] && caughtUp[1] && } {canSendToNarrow(narrow) && } diff --git a/src/chat/__tests__/chatReducers-test.js b/src/chat/__tests__/chatReducers-test.js index 526be160bd4..a32cbcc0391 100644 --- a/src/chat/__tests__/chatReducers-test.js +++ b/src/chat/__tests__/chatReducers-test.js @@ -31,13 +31,18 @@ describe('chatReducers', () => { test('changes current narrow', () => { const initialState = { narrow: [], + caughtUp: [false, false], + fetching: [false, false], }; const action = { type: SWITCH_NARROW, narrow: streamNarrow('some stream'), + fetching: [true, true], }; const expectedState = { narrow: streamNarrow('some stream'), + caughtUp: [false, false], + fetching: [true, true], }; const newState = chatReducers(initialState, action); @@ -52,16 +57,20 @@ describe('chatReducers', () => { const initialState = { messages: { [homeNarrowStr]: [{ id: 1 }, { id: 2 }], - } + }, + fetching: [false, false], + caughtUp: [false, false], }; const action = { type: EVENT_NEW_MESSAGE, - message: { id: 3 } + message: { id: 3 }, }; const expectedState = { messages: { [homeNarrowStr]: [{ id: 1 }, { id: 2 }, { id: 3 }], - } + }, + fetching: [false, false], + caughtUp: [false, false], }; const newState = chatReducers(initialState, action); @@ -245,7 +254,9 @@ describe('chatReducers', () => { { id: 2 }, { id: 3 }, ], - } + }, + fetching: [false, false], + caughtUp: [false, false], }; const action = { type: MESSAGE_FETCH_SUCCESS, @@ -255,6 +266,8 @@ describe('chatReducers', () => { { id: 3 }, { id: 4 }, ], + fetching: [false, false], + caughtUp: [false, false], }; const expectedState = { messages: { @@ -265,6 +278,8 @@ describe('chatReducers', () => { { id: 3 }, ], }, + fetching: [false, false], + caughtUp: [false, false], }; const newState = chatReducers(initialState, action); @@ -280,7 +295,9 @@ describe('chatReducers', () => { { id: 1, timestamp: 3 }, { id: 2, timestamp: 4 }, ], - } + }, + fetching: [false, false], + caughtUp: [false, false], }; const action = { type: MESSAGE_FETCH_SUCCESS, @@ -289,6 +306,8 @@ describe('chatReducers', () => { { id: 3, timestamp: 2 }, { id: 4, timestamp: 1 }, ], + fetching: [false, false], + caughtUp: [false, false], }; const expectedState = { messages: { @@ -298,47 +317,14 @@ describe('chatReducers', () => { { id: 1, timestamp: 3 }, { id: 2, timestamp: 4 }, ], - } - }; - - const newState = chatReducers(initialState, action); - - expect(newState.messages).toEqual(expectedState.messages); - expect(newState).not.toBe(initialState); - }); - - test('tracks startReached per narrow', () => { - const initialState = { - messages: { - [homeNarrowStr]: [ - { id: 1 }, - ], }, - startReached: [], - }; - const action = { - type: MESSAGE_FETCH_SUCCESS, - narrow: [], - messages: [ - { id: 2 }, - ], - startReached: true, - }; - const expectedState = { - messages: { - [homeNarrowStr]: [ - { id: 1 }, - { id: 2 }, - ], - }, - startReached: [ - '[]', - ] + fetching: [false, false], + caughtUp: [false, false], }; const newState = chatReducers(initialState, action); - expect(newState.startReached).toEqual(expectedState.startReached); + expect(newState.messages).toEqual(expectedState.messages); expect(newState).not.toBe(initialState); }); }); diff --git a/src/chat/chatReducers.js b/src/chat/chatReducers.js index 5bd78286b82..5e04c9cea92 100644 --- a/src/chat/chatReducers.js +++ b/src/chat/chatReducers.js @@ -10,26 +10,40 @@ import { isMessageInNarrow } from '../utils/narrow'; const getInitialState = () => ({ - fetching: 0, + /* + `fetching` and `caughtUp` are tuples representing (top, bottom) respectively. + + `fetching` is true if we're in the process of fetching data in that direction. + `caughtUp` is true if we know that we're at the last message in that direction. + */ + fetching: [false, false], + caughtUp: [false, false], narrow: [], messages: {}, - startReached: [], }); export default (state = getInitialState(), action) => { switch (action.type) { case ACCOUNT_SWITCH: return getInitialState(); + case SWITCH_NARROW: return { ...state, - narrow: action.narrow + narrow: action.narrow, + fetching: action.fetching, + caughtUp: [false, false], }; + case MESSAGE_FETCH_START: return { ...state, - fetching: state.fetching + 1, - narrow: action.narrow + narrow: action.narrow, + fetching: [ + action.fetching[0] || state.fetching[0], + action.fetching[1] || state.fetching[1], + ], + caughtUp: action.caughtUp ? action.caughtUp : state.caughtUp, }; case MESSAGE_FETCH_SUCCESS: { @@ -40,18 +54,20 @@ export default (state = getInitialState(), action) => { .concat(messages) .sort((a, b) => a.timestamp - b.timestamp); - const newStartReached = action.startReached && !state.startReached.includes(key); - return { ...state, - fetching: state.fetching - 1, + fetching: [ + action.fetching[0] && state.fetching[0], + action.fetching[1] && state.fetching[1], + ], messages: { ...state.messages, [key]: newMessages, }, - startReached: newStartReached ? - state.startReached.concat(key) : - state.startReached, + caughtUp: [ + action.caughtUp[0] || state.caughtUp[0], + action.caughtUp[1] || state.caughtUp[1], + ], }; } diff --git a/src/common/Loading.js b/src/common/Loading.js new file mode 100644 index 00000000000..50e8970266d --- /dev/null +++ b/src/common/Loading.js @@ -0,0 +1,58 @@ +import React from 'react'; +import { + Animated, + Easing, + StyleSheet, +} from 'react-native'; + +const styles = StyleSheet.create({ + loading: { + width: 30, + height: 30, + margin: 10, + alignSelf: 'center', + overflow: 'hidden', + }, +}); + +export default class Loading extends React.Component { + constructor() { + super(); + this.state = { + rotation: new Animated.Value(0), + }; + } + + componentDidMount() { + this.rotate(); + } + + rotate() { + this.state.rotation.setValue(0); + Animated.timing( + this.state.rotation, + { + toValue: 1, + duration: 2000, + easing: Easing.linear, + } + ).start(() => this.rotate()); + } + + render() { + const rotation = this.state.rotation.interpolate({ + inputRange: [0, 1], + outputRange: ['0deg', '360deg'], + }); + return ( + + ); + } +} diff --git a/src/common/index.js b/src/common/index.js index 91a6b7ba1cd..5eb2895d50a 100644 --- a/src/common/index.js +++ b/src/common/index.js @@ -3,6 +3,7 @@ export { default as Button } from './Button'; export { default as ErrorMsg } from './ErrorMsg'; export { default as ImageAvatar } from './ImageAvatar'; export { default as Input } from './Input'; +export { default as Loading } from './Loading'; export { default as Logo } from './Logo'; export { default as OfflineNotice } from './OfflineNotice'; export { default as Popup } from './Popup'; diff --git a/src/main/MainScreen.js b/src/main/MainScreen.js index 79632789fbd..52a46f7eb31 100644 --- a/src/main/MainScreen.js +++ b/src/main/MainScreen.js @@ -15,7 +15,7 @@ export default class MainScreen extends React.Component { fetchEvents(auth); fetchUsersAndStatus(auth); - fetchMessages(auth, Number.MAX_SAFE_INTEGER, 20, 0, []); + fetchMessages(auth, Number.MAX_SAFE_INTEGER, 20, 0, [], [true, true]); focusPing(auth, true, false); } diff --git a/src/main/MainScreenContainer.js b/src/main/MainScreenContainer.js index 799e35c17f8..bbf00f4464d 100644 --- a/src/main/MainScreenContainer.js +++ b/src/main/MainScreenContainer.js @@ -9,9 +9,16 @@ import MainScreen from './MainScreen'; class MainScreenContainer extends React.Component { fetchOlder = () => { - const { auth, isFetching, startReached, narrow, pointer, fetchMessages } = this.props; - if (!isFetching && !startReached.includes(JSON.stringify(narrow))) { - fetchMessages(auth, pointer[0], 20, 0, narrow); + const { auth, fetching, caughtUp, pointer, narrow, fetchMessages } = this.props; + if (!fetching[0] && !caughtUp[0]) { + fetchMessages(auth, pointer[0], 20, 0, narrow, [true, false]); + } + } + + fetchNewer = () => { + const { auth, fetching, caughtUp, pointer, narrow, fetchMessages } = this.props; + if (!fetching[1] && !caughtUp[1]) { + fetchMessages(auth, pointer[1], 0, 20, narrow, [false, true]); } } @@ -21,7 +28,7 @@ class MainScreenContainer extends React.Component { if (allMessages[JSON.stringify(newNarrow)]) { switchNarrow(newNarrow); } else { - fetchMessages(auth, pointer, 20, 0, newNarrow); + fetchMessages(auth, pointer, 10, 10, newNarrow, [true, true], [false, false]); } } @@ -29,6 +36,7 @@ class MainScreenContainer extends React.Component { return ( @@ -42,9 +50,9 @@ const mapStateToProps = (state) => ({ subscriptions: state.subscriptions, messages: getMessagesInActiveNarrow(state), allMessages: state.chat.messages, - isFetching: state.chat.fetching > 0, + fetching: state.chat.fetching, + caughtUp: state.chat.caughtUp, narrow: state.chat.narrow, - startReached: state.chat.startReached, pointer: getPointer(state), streamlistOpened: state.nav.opened, }); diff --git a/src/message-list/InfiniteScrollView.js b/src/message-list/InfiniteScrollView.js index bfff0d24dd8..2c139b9c4ed 100644 --- a/src/message-list/InfiniteScrollView.js +++ b/src/message-list/InfiniteScrollView.js @@ -25,6 +25,7 @@ class InfiniteScrollView extends React.Component { _onContentSizeChanged(contentWidth, contentHeight) { const oldContentHeight = this._contentHeight; this._contentHeight = contentHeight; + this._maybeCallOnStartOrEndReached(); } _onScrollViewLayout(e) { @@ -51,8 +52,7 @@ class InfiniteScrollView extends React.Component { } } - _onScroll(e) { - this._scrollOffset = e.nativeEvent.contentOffset['y']; + _maybeCallOnStartOrEndReached() { const distFromStart = this._scrollOffset; const distFromEnd = this._contentHeight - this._scrollViewHeight - this._scrollOffset; @@ -67,6 +67,11 @@ class InfiniteScrollView extends React.Component { distFromEnd > this.props.onEndReachedThreshold) { this._sentEndForContentHeight = null; } + } + + _onScroll(e) { + this._scrollOffset = e.nativeEvent.contentOffset['y']; + this._maybeCallOnStartOrEndReached(); this.props.onScroll(e.nativeEvent); } diff --git a/src/message-list/MessageList.js b/src/message-list/MessageList.js index 25593c5f0e8..f4959405ca5 100644 --- a/src/message-list/MessageList.js +++ b/src/message-list/MessageList.js @@ -1,6 +1,9 @@ import React from 'react'; import { StyleSheet } from 'react-native'; +import { Loading } from '../common'; +import MessageLoading from '../message/MessageLoading'; + import InfiniteScrollView from './InfiniteScrollView'; import renderMessages from './renderMessages'; @@ -8,7 +11,7 @@ const styles = StyleSheet.create({ list: { backgroundColor: 'white', }, - container: { + centerContainer: { flexGrow: 1, justifyContent: 'space-around', }, @@ -32,8 +35,25 @@ export default class MessageList extends React.PureComponent { } render() { - const { fetchOlder, fetchNewer } = this.props; - const messageList = renderMessages(this.props); + const { messages, caughtUp, fetching, fetchOlder, fetchNewer } = this.props; + let messageList = []; + let containerStyle = {}; + + if (messages.length === 0) { + if (!caughtUp[0] || !caughtUp[1]) { + // Show placeholder messages if we're loading the screen + messageList = [...Array(6).keys()].map((i) => + + ); + } + containerStyle = styles.centerContainer; + } else { + messageList = [ + , + ...renderMessages(this.props), + , + ]; + } // `headerIndices` tell the scroll view which components are headers // and are eligible to be docked at the top of the view. @@ -70,14 +90,14 @@ export default class MessageList extends React.PureComponent { return ( {}} > {messageList} diff --git a/src/message-list/__tests__/renderMessages-test.js b/src/message-list/__tests__/renderMessages-test.js index 40b8887180b..82f4fe86c1b 100644 --- a/src/message-list/__tests__/renderMessages-test.js +++ b/src/message-list/__tests__/renderMessages-test.js @@ -12,11 +12,6 @@ describe('renderMessages', () => { expect(messageList).toEqual([]); }); - test('if fetching in progress, one or more loading messages are shown', () => { - const messageList = renderMessages({ messages: [], isFetching: true }); - expect(messageList[0].type.name).toEqual('MessageLoading'); - }); - test('renders time, header and message for a single input', () => { const messages = [{ timestamp: 123, diff --git a/src/message-list/messagesActions.js b/src/message-list/messagesActions.js index 63276302281..ffa28d00f1f 100644 --- a/src/message-list/messagesActions.js +++ b/src/message-list/messagesActions.js @@ -8,6 +8,8 @@ import { export const switchNarrow = (narrow) => ({ type: SWITCH_NARROW, narrow, + fetching: [false, false], + caughtUp: [false, false], }); export const fetchMessages = ( @@ -16,18 +18,31 @@ export const fetchMessages = ( numBefore: number, numAfter: number, narrow, + fetching = [false, false], + caughtUp, ) => async (dispatch) => { - dispatch({ type: MESSAGE_FETCH_START, narrow }); + dispatch({ type: MESSAGE_FETCH_START, narrow, fetching, caughtUp }); const messages = await getMessages(auth, anchor, numBefore, numAfter, narrow); + // Find the anchor in the results (or set it past the end of the list) + // We can use the position of the anchor to determine if we're caught up + // in both directions. + let anchorIndex = messages.findIndex((msg) => msg.id === anchor); + if (anchorIndex < 0) anchorIndex = messages.length; + const newCaughtUp = [ + anchorIndex + 1 < numBefore, + messages.length - anchorIndex - 1 < numAfter, + ]; + dispatch({ type: MESSAGE_FETCH_SUCCESS, auth, messages, anchor, narrow, - startReached: numAfter === 0 && numBefore > messages.length, + fetching: [!fetching[0], !fetching[1]], + caughtUp: newCaughtUp, }); }; diff --git a/src/message-list/renderMessages.js b/src/message-list/renderMessages.js index 586816febfe..7f355401b21 100644 --- a/src/message-list/renderMessages.js +++ b/src/message-list/renderMessages.js @@ -6,23 +6,17 @@ import { isPrivateNarrow, isGroupNarrow, } from '../utils/narrow'; + import MessageHeader from '../message/headers/MessageHeader'; import MessageContainer from '../message/MessageContainer'; -import MessageLoading from '../message/MessageLoading'; import TimeRow from '../message/TimeRow'; import { isSameRecipient } from '../utils/message'; import { isSameDay } from '../utils/date'; -export default ({ auth, subscriptions, messages, isFetching, narrow, doNarrow }) => { +export default ({ auth, subscriptions, messages, narrow, doNarrow }) => { const list = []; let prevItem; - if (isFetching) { - for (let i = 0; i < 6; i++) { - list.push(); - } - } - for (const item of messages) { const diffDays = prevItem && !isSameDay(new Date(prevItem.timestamp * 1000), new Date(item.timestamp * 1000)); @@ -75,6 +69,5 @@ export default ({ auth, subscriptions, messages, isFetching, narrow, doNarrow }) prevItem = item; } - return list; }; diff --git a/static/img/message-loading.png b/static/img/message-loading.png new file mode 100644 index 00000000000..a13f2d137ea Binary files /dev/null and b/static/img/message-loading.png differ