Skip to content

Commit

Permalink
MessageReactionList: Wrap rogue tab nav in createNavigationContainer.
Browse files Browse the repository at this point in the history
This will prevent a crashing error on iOS and Android that would
otherwise be introduced in the upcoming react-navigation v2 -> v3
upgrade, on opening the message-reaction list. See discussion [1].

[1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Dynamic.20routes.20in.20react-navigation/near/1008268
  • Loading branch information
chrisbobbe committed Sep 11, 2020
1 parent 9176f7a commit b8fa279
Showing 1 changed file with 28 additions and 15 deletions.
43 changes: 28 additions & 15 deletions src/reactions/MessageReactionList.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* @flow strict-local */
import React, { PureComponent } from 'react';
import { View } from 'react-native';
import { createMaterialTopTabNavigator } from 'react-navigation';
import { createNavigationContainer, createMaterialTopTabNavigator } from 'react-navigation';
import type { NavigationScreenProp } from 'react-navigation';

import * as logging from '../utils/logging';
Expand Down Expand Up @@ -62,23 +62,32 @@ const getReactionsTabs = (
]),
);

return createMaterialTopTabNavigator(reactionsTabs, {
backBehavior: 'none',
// It's not feasible to set up our newly created tab navigator as
// part of the entire app's navigation (see the note at
// `getReactionsTabs`'s call site). Given that, it seems we can use
// `createNavigationContainer` (soon to be called
// `createAppContainer`) so our violation of the "only explicitly
// render one navigator" rule doesn't become a crashing error in
// `react-navigation` v3.
return createNavigationContainer(
createMaterialTopTabNavigator(reactionsTabs, {
backBehavior: 'none',

// The user may have originally navigated here to look at a reaction
// that's since been removed. Ignore the nav hint in that case.
...(reactionName !== undefined && reactionsTabs[reactionName]
? { initialRouteName: reactionName }
: {}),
// The user may have originally navigated here to look at a reaction
// that's since been removed. Ignore the nav hint in that case.
...(reactionName !== undefined && reactionsTabs[reactionName]
? { initialRouteName: reactionName }
: {}),

...tabsOptions({
showLabel: true,
showIcon: false,
style: {
borderWidth: 0.15,
},
...tabsOptions({
showLabel: true,
showIcon: false,
style: {
borderWidth: 0.15,
},
}),
}),
});
);
};

type SelectorProps = $ReadOnly<{|
Expand Down Expand Up @@ -135,6 +144,10 @@ class MessageReactionList extends PureComponent<Props> {
);
} else {
const aggregatedReactions = aggregateReactions(message.reactions, ownUserId);
// TODO: React Navigation doesn't want us to explicitly render
// more than one navigator in the app, and the recommended
// workaround isn't feasible; see discussion at
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Dynamic.20routes.20in.20react-navigation/near/1008268.
const TabView = getReactionsTabs(aggregatedReactions, reactionName, allUsersById);
return (
<View style={styles.flexed}>
Expand Down

0 comments on commit b8fa279

Please sign in to comment.