Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions src/diagnostics/TimingScreen.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* @flow strict-local */
import React, { PureComponent } from 'react';
import React from 'react';
import { FlatList } from 'react-native';

import type { RouteProp } from '../react-navigation';
Expand All @@ -13,18 +13,16 @@ type Props = $ReadOnly<{|
route: RouteProp<'timing', void>,
|}>;

export default class TimingScreen extends PureComponent<Props> {
render() {
return (
<Screen title="Timing" scrollEnabled={false}>
<FlatList
data={timing.log}
keyExtractor={(item, index) => index.toString()}
renderItem={({ item }) => (
<TimeItem text={item.text} startMs={item.startMs} endMs={item.endMs} />
)}
/>
</Screen>
);
}
export default function TimingScreen(props: Props) {
return (
<Screen title="Timing" scrollEnabled={false}>
<FlatList
data={timing.log}
keyExtractor={(item, index) => index.toString()}
renderItem={({ item }) => (
<TimeItem text={item.text} startMs={item.startMs} endMs={item.endMs} />
)}
/>
</Screen>
);
}
44 changes: 1 addition & 43 deletions src/main/MainTabsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
import { useSafeAreaInsets } from 'react-native-safe-area-context';

import type { RouteProp, RouteParamsOf } from '../react-navigation';
import type { Dispatch } from '../types';
import type { AppNavigationProp } from '../nav/AppNavigator';
import type { GlobalParamList } from '../nav/globalTypes';
import { bottomTabNavigatorConfig } from '../styles/tabs';
Expand All @@ -20,10 +19,7 @@ import { IconInbox, IconSettings, IconStream } from '../common/Icons';
import { OwnAvatar, OfflineNotice, ZulipStatusBar } from '../common';
import IconUnreadConversations from '../nav/IconUnreadConversations';
import ProfileScreen from '../account-info/ProfileScreen';
import { connect } from '../react-redux';
import { getHaveServerData } from '../selectors';
import styles, { ThemeContext } from '../styles';
import FullScreenLoading from '../common/FullScreenLoading';

export type MainTabsNavigatorParamList = {|
home: RouteParamsOf<typeof HomeScreen>,
Expand All @@ -43,35 +39,16 @@ const Tab = createBottomTabNavigator<
MainTabsNavigationProp<>,
>();

type SelectorProps = $ReadOnly<{|
haveServerData: boolean,
|}>;

type Props = $ReadOnly<{|
navigation: AppNavigationProp<'main-tabs'>,
route: RouteProp<'main-tabs', void>,

dispatch: Dispatch,
...SelectorProps,
|}>;

function MainTabsScreen(props: Props) {
export default function MainTabsScreen(props: Props) {
const { backgroundColor } = useContext(ThemeContext);
const { haveServerData } = props;

const insets = useSafeAreaInsets();

if (!haveServerData) {
// Show a full-screen loading indicator while waiting for the
// initial fetch to complete, if we don't have potentially stale
// data to show instead. Also show it for the duration of the nav
// transition just after the user logs out (see our #4275).
//
// And avoid rendering any of our main UI, to maintain the
// guarantee that it can all rely on server data existing.
return <FullScreenLoading />;
}

return (
<View style={[styles.flexed, { backgroundColor }]}>
<View style={{ height: insets.top }} />
Expand Down Expand Up @@ -128,22 +105,3 @@ function MainTabsScreen(props: Props) {
</View>
);
}

// `connect` does something useful for us that `useSelector` doesn't
// do: it interposes a new `ReactReduxContext.Provider` component,
// which proxies subscriptions so that the descendant components only
// rerender if this one continues to say their subtree should be kept
// around. See
// https://github.com/zulip/zulip-mobile/pull/4454#discussion_r578140524
// and some discussion around
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/converting.20to.20Hooks/near/1111970
// where we describe some limits of our understanding.
//
// We found these things out while investigating an annoying crash: we
// found that `mapStateToProps` on a descendant of `MainTabsScreen`
// was running -- and throwing an uncaught error -- on logout, and
// `MainTabsScreen`'s early return on `!haveServerData` wasn't
// preventing that from happening.
export default connect(state => ({
haveServerData: getHaveServerData(state),
}))(MainTabsScreen);
65 changes: 41 additions & 24 deletions src/nav/AppNavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import EmojiPickerScreen from '../emoji/EmojiPickerScreen';
import LegalScreen from '../settings/LegalScreen';
import UserStatusScreen from '../user-status/UserStatusScreen';
import SharingScreen from '../sharing/SharingScreen';
import withHaveServerDataGate from '../withHaveServerDataGate';

export type AppNavigatorParamList = {|
'account-pick': RouteParamsOf<typeof AccountPickScreen>,
Expand Down Expand Up @@ -104,39 +105,55 @@ export default function AppNavigator(props: Props) {
}),
}}
>
{/* These screens expect server data in order to function normally. */}
<Stack.Screen
name="account-details"
component={withHaveServerDataGate(AccountDetailsScreen)}
/>
<Stack.Screen name="group-details" component={withHaveServerDataGate(GroupDetailsScreen)} />
<Stack.Screen name="chat" component={withHaveServerDataGate(ChatScreen)} />
<Stack.Screen name="emoji-picker" component={withHaveServerDataGate(EmojiPickerScreen)} />
<Stack.Screen name="main-tabs" component={withHaveServerDataGate(MainTabsScreen)} />
<Stack.Screen
name="message-reactions"
component={withHaveServerDataGate(MessageReactionsScreen)}
/>
<Stack.Screen
name="search-messages"
component={withHaveServerDataGate(SearchMessagesScreen)}
/>
<Stack.Screen name="users" component={withHaveServerDataGate(UsersScreen)} />
<Stack.Screen name="language" component={withHaveServerDataGate(LanguageScreen)} />
<Stack.Screen name="lightbox" component={withHaveServerDataGate(LightboxScreen)} />
<Stack.Screen name="create-group" component={withHaveServerDataGate(CreateGroupScreen)} />
<Stack.Screen name="invite-users" component={withHaveServerDataGate(InviteUsersScreen)} />
<Stack.Screen name="diagnostics" component={withHaveServerDataGate(DiagnosticsScreen)} />
<Stack.Screen name="variables" component={withHaveServerDataGate(VariablesScreen)} />
<Stack.Screen name="timing" component={withHaveServerDataGate(TimingScreen)} />
<Stack.Screen name="storage" component={withHaveServerDataGate(StorageScreen)} />
<Stack.Screen name="debug" component={withHaveServerDataGate(DebugScreen)} />
<Stack.Screen
name="stream-settings"
component={withHaveServerDataGate(StreamSettingsScreen)}
/>
<Stack.Screen name="edit-stream" component={withHaveServerDataGate(EditStreamScreen)} />
<Stack.Screen name="create-stream" component={withHaveServerDataGate(CreateStreamScreen)} />
<Stack.Screen name="topic-list" component={withHaveServerDataGate(TopicListScreen)} />
<Stack.Screen name="notifications" component={withHaveServerDataGate(NotificationsScreen)} />
<Stack.Screen name="legal" component={withHaveServerDataGate(LegalScreen)} />
<Stack.Screen name="user-status" component={withHaveServerDataGate(UserStatusScreen)} />

{/* These screens do not expect server data in order to function
normally. */}
<Stack.Screen name="account-pick" component={AccountPickScreen} />
<Stack.Screen name="account-details" component={AccountDetailsScreen} />
<Stack.Screen name="group-details" component={GroupDetailsScreen} />
<Stack.Screen name="auth" component={AuthScreen} />
<Stack.Screen name="chat" component={ChatScreen} />
<Stack.Screen name="dev-auth" component={DevAuthScreen} />
<Stack.Screen name="emoji-picker" component={EmojiPickerScreen} />
<Stack.Screen name="main-tabs" component={MainTabsScreen} />
<Stack.Screen name="message-reactions" component={MessageReactionsScreen} />
<Stack.Screen name="password-auth" component={PasswordAuthScreen} />
<Stack.Screen
name="realm-input"
component={RealmInputScreen}
initialParams={initialRouteName === 'realm-input' ? initialRouteParams : undefined}
/>
<Stack.Screen name="search-messages" component={SearchMessagesScreen} />
<Stack.Screen name="users" component={UsersScreen} />
<Stack.Screen name="language" component={LanguageScreen} />
<Stack.Screen name="lightbox" component={LightboxScreen} />
<Stack.Screen name="create-group" component={CreateGroupScreen} />
<Stack.Screen name="invite-users" component={InviteUsersScreen} />
<Stack.Screen name="diagnostics" component={DiagnosticsScreen} />
<Stack.Screen name="variables" component={VariablesScreen} />
<Stack.Screen name="timing" component={TimingScreen} />
<Stack.Screen name="storage" component={StorageScreen} />
<Stack.Screen name="debug" component={DebugScreen} />
<Stack.Screen name="stream-settings" component={StreamSettingsScreen} />
<Stack.Screen name="edit-stream" component={EditStreamScreen} />
<Stack.Screen name="create-stream" component={CreateStreamScreen} />
<Stack.Screen name="topic-list" component={TopicListScreen} />
<Stack.Screen name="notifications" component={NotificationsScreen} />
<Stack.Screen name="legal" component={LegalScreen} />
<Stack.Screen name="user-status" component={UserStatusScreen} />
<Stack.Screen name="sharing" component={SharingScreen} />
</Stack.Navigator>
);
Expand Down
63 changes: 63 additions & 0 deletions src/withHaveServerDataGate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/* @flow strict-local */
import React, { type ComponentType, type ElementConfig } from 'react';

import { connect } from './react-redux';
import type { Dispatch } from './types';
import { getHaveServerData } from './selectors';
import FullScreenLoading from './common/FullScreenLoading';

/**
* A HOC for any server-data-dependent screen component that might be
* mounted when we don't have server data.
*
* Prevents rerendering of the component's subtree unless we have
* server data.
*
* The implementation uses props named `dispatch` and `haveServerData`; the
* inner component shouldn't try to accept props with those names, and the
* caller shouldn't try to pass them in.
*/
// It sure seems like Flow should catch the `dispatch` / `haveServerData`
// thing and reflect it in the types; it's not clear why it doesn't.
export default function withHaveServerDataGate<P: { ... }, C: ComponentType<$Exact<P>>>(
Comp: C,
): ComponentType<$Exact<ElementConfig<C>>> {
// `connect` does something useful for us that `useSelector` doesn't
// do: it interposes a new `ReactReduxContext.Provider` component,
// which proxies subscriptions so that the descendant components only
// rerender if this one continues to say their subtree should be kept
// around. See
// https://github.com/zulip/zulip-mobile/pull/4454#discussion_r578140524
// and some discussion around
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/converting.20to.20Hooks/near/1111970
// where we describe some limits of our understanding.
//
// We found these things out while investigating an annoying crash: we
// found that `mapStateToProps` on a descendant of `MainTabsScreen`
// was running -- and throwing an uncaught error -- on logout, and
// `MainTabsScreen`'s early return on `!haveServerData` wasn't
// preventing that from happening.
return connect(state => ({ haveServerData: getHaveServerData(state) }))(
({
dispatch,
haveServerData,
...props
}: {|
dispatch: Dispatch,
haveServerData: boolean,
...$Exact<P>,
|}) =>
haveServerData ? (
<Comp {...props} />
) : (
// Show a full-screen loading indicator while waiting for the
// initial fetch to complete, if we don't have potentially stale
// data to show instead. Also show it for the duration of the nav
// transition just after the user logs out (see our #4275).
//
// And avoid rendering any of our main UI, to maintain the
// guarantee that it can all rely on server data existing.
<FullScreenLoading />
),
);
}