Improve loading indicators and fix fetching / caught-up logic. (zulip…

…#247)

Fixes zulip#237 and fixes zulip#154

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: [

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;
 };

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);

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 (
       <KeyboardAvoidingView style={styles.screen} behavior="padding">
         {!isOnline && <OfflineNotice />}
-        {messages.length === 0 && !isFetching && <NoMessages narrow={narrow} />}
+        {messages.length === 0 && caughtUp[0] && caughtUp[1] && <NoMessages narrow={narrow} />}
         <MessageList
           messages={messages}
           narrow={narrow}
-          isFetching={isFetching}
+          fetching={fetching}
+          caughtUp={caughtUp}
           twentyFourHourTime={twentyFourHourTime}
           subscriptions={subscriptions}
           auth={auth}
           fetchOlder={fetchOlder}
+          fetchNewer={fetchNewer}
           doNarrow={doNarrow}
         />
         {canSendToNarrow(narrow) && <ComposeBox onSend={this.sendMessage} />}

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);
     });
   });

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],
+        ],
       };
     }
 

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 (
+      <Animated.Image
+        style={[
+          styles.loading,
+          this.props.active ? { transform: [{ rotate: rotation }] } : {},
+        ]}
+        source={require('../../static/img/message-loading.png')}
+        resizeMode="contain"
+      />
+    );
+  }
+}

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';

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);
   }