Page MenuHomePhabricator
Differential D7672

[native] Create a new screen to display the thread's pinned messages
ClosedPublic
Actions

Authored by rohan on Apr 27 2023, 11:10 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 23, 12:49 AM
Unknown Object (File)
Tue, Mar 18, 12:05 PM
Unknown Object (File)
Mon, Mar 17, 12:11 PM
Unknown Object (File)
Mon, Mar 17, 12:11 PM
Unknown Object (File)
Mon, Mar 17, 12:11 PM
Unknown Object (File)
Mon, Mar 17, 12:11 PM
Unknown Object (File)
Mon, Mar 17, 12:11 PM
Unknown Object (File)
Mon, Mar 17, 12:10 PM
View All Files
Subscribers
ashoat
inka

Details

Reviewers
ginsu
tomek
ashoat
inka
atul
Commits
rCOMM2e1c72c3d557: [native] Create a new screen to display the thread's pinned messages
Summary

Create a very basic new screen to show the thraed's pinned messages (this screen
will be re-used by the message search work on native).

Implementation of the screen will come in the next diff, to make this one easier to
review.

Depends on D7614

Test Plan

Navigate to the screen by clicking on the pinned count banner from the previous
diff and make sure the screen is navigated to.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan created this revision.Apr 27 2023, 11:10 AM
Herald added a subscriber: ashoat. · View Herald TranscriptApr 27 2023, 11:10 AM
rohan added a child revision: D7673: [native] Display the pinned messages for the thread in the new screen.Apr 27 2023, 11:15 AM
Harbormaster completed remote builds in B18890: Diff 25874.Apr 27 2023, 11:27 AM
rohan requested review of this revision.Apr 27 2023, 11:27 AM
rohan updated this revision to Diff 26028.May 2 2023, 5:21 PM

Simplify headerTitle since the screen will be exclusive to pinned
messages

Harbormaster completed remote builds in B19008: Diff 26028.May 2 2023, 5:39 PM
atul resigned from this revision.May 3 2023, 8:12 AM
atul added a reviewer: ashoat.
ashoat accepted this revision.May 3 2023, 8:15 AM
ashoat added a reviewer: inka.
ashoat added a subscriber: inka.

Looks good, but should @inka be on this diff given that MessagesResult will be used for message search as well?

This revision is now accepted and ready to land.May 3 2023, 8:15 AM
rohan updated this revision to Diff 26136.May 5 2023, 4:26 PM

Rebase

Harbormaster completed remote builds in B19088: Diff 26136.May 5 2023, 4:43 PM
rohan updated this revision to Diff 26226.May 8 2023, 12:07 PM

Rebase

Harbormaster completed remote builds in B19155: Diff 26226.May 8 2023, 12:30 PM
Closed by commit rCOMM2e1c72c3d557: [native] Create a new screen to display the thread's pinned messages (authored by RohanK6 <rohan.khanderia@gmail.com>, committed by rohan). · Explain WhyMay 8 2023, 12:38 PM
This revision was automatically updated to reflect the committed changes.
rohan added a commit: rCOMM2e1c72c3d557: [native] Create a new screen to display the thread's pinned messages.

Revision Contents
Changeset List

PathSize
native/
chat/
11 lines
23 lines
24 lines
navigation/
4 lines

Diff 26234

View Options

native/chat/chat.react.js

// @flow // @flow
import Icon from '@expo/vector-icons/FontAwesome5.js';
import { useNavigationState } from '@react-navigation/native'; import { useNavigationState } from '@react-navigation/native';
import invariant from 'invariant'; import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View, Text } from 'react-native';
import genesis from 'lib/facts/genesis.js'; import genesis from 'lib/facts/genesis.js';
import { threadInfoSelector } from 'lib/selectors/thread-selectors.js'; import { threadInfoSelector } from 'lib/selectors/thread-selectors.js';
import { import {
userInfoSelectorForPotentialMembers, userInfoSelectorForPotentialMembers,
userSearchIndexForPotentialMembers, userSearchIndexForPotentialMembers,
} from 'lib/selectors/user-selectors.js'; } from 'lib/selectors/user-selectors.js';
import { getPotentialMemberItems } from 'lib/shared/search-utils.js'; import { getPotentialMemberItems } from 'lib/shared/search-utils.js';
▲ Show 20 Lines • Show All 189 Lines • ▼ Show 20 Lines if (this.frozen) {
this.pendingListDataWithHeights = listDataWithHeights; this.pendingListDataWithHeights = listDataWithHeights;
} else { } else {
this.setState({ listDataWithHeights }); this.setState({ listDataWithHeights });
} }
}; };
} }
const unboundStyles = { const unboundStyles = {
pinnedCountBanner: {
backgroundColor: 'panelForeground',
height: 30,
flexDirection: 'row',
textAlign: 'center',
justifyContent: 'center',
alignItems: 'center',
},
pinnedCountText: {
color: 'panelBackgroundLabel',
marginRight: 5,
},
container: { container: {
backgroundColor: 'listBackground', backgroundColor: 'listBackground',
flex: 1, flex: 1,
}, },
threadContent: { threadContent: {
flex: 1, flex: 1,
}, },
hiddenThreadContent: { hiddenThreadContent: {
▲ Show 20 Lines • Show All 124 Lines • ▼ Show 20 Lines ) {
const styles = useStyles(unboundStyles); const styles = useStyles(unboundStyles);
const overlayContext = React.useContext(OverlayContext); const overlayContext = React.useContext(OverlayContext);
const measureMessages = useHeightMeasurer(); const measureMessages = useHeightMeasurer();
const genesisThreadInfo = useSelector( const genesisThreadInfo = useSelector(
state => threadInfoSelector(state)[genesis.id], state => threadInfoSelector(state)[genesis.id],
); );
const bannerText = React.useMemo(() => {
if (!threadInfo.pinnedCount || threadInfo.pinnedCount === 0) {
return '';
}
const messageNoun = threadInfo.pinnedCount === 1 ? 'message' : 'messages';
return `${threadInfo.pinnedCount} pinned ${messageNoun}`;
}, [threadInfo.pinnedCount]);
atulUnsubmitted
Not Done
Inline Actions

You don't need to use React.useMemo(...) here.

The useMemo() hook is helpful for maintaining referential equality so that objects will be considered "shallowly equal" (== in JS) and we can avoid re-renders. This is helpful for objects (including Map(), Set(), etc), arrays (which are objects), and functions (which are objects).

On the other hand, strings in JS are considered shallowly equal if they have the same contents, so we don't have to worry about re-renders if the "content" stays the same.

See below:

875403.png (620×810 px, 104 KB)

(In like C++, which you've been working w/ recently, std::strings can be allocated on the heap (unlike integers, booleans, etc) which may have been part of your reasoning that shallow equality would check reference instead of contents?)

atul: You don't need to use `React.useMemo(...)` here. The `useMemo()` hook is helpful for…
const pinnedCountBanner = React.useMemo(() => {
if (!bannerText) {
return null;
}
return (
<View style={styles.pinnedCountBanner}>
<Text style={styles.pinnedCountText}>{bannerText}</Text>
<Icon
name="chevron-right"
size={12}
color={colors.panelBackgroundLabel}
/>
</View>
);
}, [
bannerText,
styles.pinnedCountBanner,
styles.pinnedCountText,
colors.panelBackgroundLabel,
]);
return ( return (
<MessageListContextProvider threadInfo={threadInfo}> <MessageListContextProvider threadInfo={threadInfo}>
{pinnedCountBanner}
<MessageListContainer <MessageListContainer
{...props} {...props}
usernameInputText={usernameInputText} usernameInputText={usernameInputText}
updateUsernameInput={updateUsernameInput} updateUsernameInput={updateUsernameInput}
userInfoInputArray={userInfoInputArray} userInfoInputArray={userInfoInputArray}
updateTagInput={updateTagInput} updateTagInput={updateTagInput}
resolveToUser={resolveToUser} resolveToUser={resolveToUser}
otherUserInfos={otherUserInfos} otherUserInfos={otherUserInfos}
Show All 14 Lines
View Options

native/chat/message-list-container.react.js

// @flow // @flow
import Icon from '@expo/vector-icons/FontAwesome5.js';
import { useNavigationState } from '@react-navigation/native'; import { useNavigationState } from '@react-navigation/native';
import invariant from 'invariant'; import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View, Text } from 'react-native';
import genesis from 'lib/facts/genesis.js'; import genesis from 'lib/facts/genesis.js';
import { threadInfoSelector } from 'lib/selectors/thread-selectors.js'; import { threadInfoSelector } from 'lib/selectors/thread-selectors.js';
import { import {
userInfoSelectorForPotentialMembers, userInfoSelectorForPotentialMembers,
userSearchIndexForPotentialMembers, userSearchIndexForPotentialMembers,
} from 'lib/selectors/user-selectors.js'; } from 'lib/selectors/user-selectors.js';
import { getPotentialMemberItems } from 'lib/shared/search-utils.js'; import { getPotentialMemberItems } from 'lib/shared/search-utils.js';
▲ Show 20 Lines • Show All 189 Lines • ▼ Show 20 Lines if (this.frozen) {
this.pendingListDataWithHeights = listDataWithHeights; this.pendingListDataWithHeights = listDataWithHeights;
} else { } else {
this.setState({ listDataWithHeights }); this.setState({ listDataWithHeights });
} }
}; };
} }
const unboundStyles = { const unboundStyles = {
pinnedCountBanner: {
backgroundColor: 'panelForeground',
height: 30,
flexDirection: 'row',
textAlign: 'center',
justifyContent: 'center',
alignItems: 'center',
},
pinnedCountText: {
color: 'panelBackgroundLabel',
marginRight: 5,
},
container: { container: {
backgroundColor: 'listBackground', backgroundColor: 'listBackground',
flex: 1, flex: 1,
}, },
threadContent: { threadContent: {
flex: 1, flex: 1,
}, },
hiddenThreadContent: { hiddenThreadContent: {
▲ Show 20 Lines • Show All 124 Lines • ▼ Show 20 Lines ) {
const styles = useStyles(unboundStyles); const styles = useStyles(unboundStyles);
const overlayContext = React.useContext(OverlayContext); const overlayContext = React.useContext(OverlayContext);
const measureMessages = useHeightMeasurer(); const measureMessages = useHeightMeasurer();
const genesisThreadInfo = useSelector( const genesisThreadInfo = useSelector(
state => threadInfoSelector(state)[genesis.id], state => threadInfoSelector(state)[genesis.id],
); );
const bannerText = React.useMemo(() => {
if (!threadInfo.pinnedCount || threadInfo.pinnedCount === 0) {
return '';
}
const messageNoun = threadInfo.pinnedCount === 1 ? 'message' : 'messages';
return `${threadInfo.pinnedCount} pinned ${messageNoun}`;
}, [threadInfo.pinnedCount]);
atulUnsubmitted
Not Done
Inline Actions

You don't need to use React.useMemo(...) here.

The useMemo() hook is helpful for maintaining referential equality so that objects will be considered "shallowly equal" (== in JS) and we can avoid re-renders. This is helpful for objects (including Map(), Set(), etc), arrays (which are objects), and functions (which are objects).

On the other hand, strings in JS are considered shallowly equal if they have the same contents, so we don't have to worry about re-renders if the "content" stays the same.

See below:

875403.png (620×810 px, 104 KB)

(In like C++, which you've been working w/ recently, std::strings can be allocated on the heap (unlike integers, booleans, etc) which may have been part of your reasoning that shallow equality would check reference instead of contents?)

atul: You don't need to use `React.useMemo(...)` here. The `useMemo()` hook is helpful for…
const pinnedCountBanner = React.useMemo(() => {
if (!bannerText) {
return null;
}
return (
<View style={styles.pinnedCountBanner}>
<Text style={styles.pinnedCountText}>{bannerText}</Text>
<Icon
name="chevron-right"
size={12}
color={colors.panelBackgroundLabel}
/>
</View>
);
}, [
bannerText,
styles.pinnedCountBanner,
styles.pinnedCountText,
colors.panelBackgroundLabel,
]);
return ( return (
<MessageListContextProvider threadInfo={threadInfo}> <MessageListContextProvider threadInfo={threadInfo}>
{pinnedCountBanner}
<MessageListContainer <MessageListContainer
{...props} {...props}
usernameInputText={usernameInputText} usernameInputText={usernameInputText}
updateUsernameInput={updateUsernameInput} updateUsernameInput={updateUsernameInput}
userInfoInputArray={userInfoInputArray} userInfoInputArray={userInfoInputArray}
updateTagInput={updateTagInput} updateTagInput={updateTagInput}
resolveToUser={resolveToUser} resolveToUser={resolveToUser}
otherUserInfos={otherUserInfos} otherUserInfos={otherUserInfos}
Show All 14 Lines
View Options

native/chat/message-results-screen.react.js

// @flow // @flow
import Icon from '@expo/vector-icons/FontAwesome5.js';
import { useNavigationState } from '@react-navigation/native'; import { useNavigationState } from '@react-navigation/native';
import invariant from 'invariant'; import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View, Text } from 'react-native';
import genesis from 'lib/facts/genesis.js'; import genesis from 'lib/facts/genesis.js';
import { threadInfoSelector } from 'lib/selectors/thread-selectors.js'; import { threadInfoSelector } from 'lib/selectors/thread-selectors.js';
import { import {
userInfoSelectorForPotentialMembers, userInfoSelectorForPotentialMembers,
userSearchIndexForPotentialMembers, userSearchIndexForPotentialMembers,
} from 'lib/selectors/user-selectors.js'; } from 'lib/selectors/user-selectors.js';
import { getPotentialMemberItems } from 'lib/shared/search-utils.js'; import { getPotentialMemberItems } from 'lib/shared/search-utils.js';
▲ Show 20 Lines • Show All 189 Lines • ▼ Show 20 Lines if (this.frozen) {
this.pendingListDataWithHeights = listDataWithHeights; this.pendingListDataWithHeights = listDataWithHeights;
} else { } else {
this.setState({ listDataWithHeights }); this.setState({ listDataWithHeights });
} }
}; };
} }
const unboundStyles = { const unboundStyles = {
pinnedCountBanner: {
backgroundColor: 'panelForeground',
height: 30,
flexDirection: 'row',
textAlign: 'center',
justifyContent: 'center',
alignItems: 'center',
},
pinnedCountText: {
color: 'panelBackgroundLabel',
marginRight: 5,
},
container: { container: {
backgroundColor: 'listBackground', backgroundColor: 'listBackground',
flex: 1, flex: 1,
}, },
threadContent: { threadContent: {
flex: 1, flex: 1,
}, },
hiddenThreadContent: { hiddenThreadContent: {
▲ Show 20 Lines • Show All 124 Lines • ▼ Show 20 Lines ) {
const styles = useStyles(unboundStyles); const styles = useStyles(unboundStyles);
const overlayContext = React.useContext(OverlayContext); const overlayContext = React.useContext(OverlayContext);
const measureMessages = useHeightMeasurer(); const measureMessages = useHeightMeasurer();
const genesisThreadInfo = useSelector( const genesisThreadInfo = useSelector(
state => threadInfoSelector(state)[genesis.id], state => threadInfoSelector(state)[genesis.id],
); );
const bannerText = React.useMemo(() => {
if (!threadInfo.pinnedCount || threadInfo.pinnedCount === 0) {
return '';
}
const messageNoun = threadInfo.pinnedCount === 1 ? 'message' : 'messages';
return `${threadInfo.pinnedCount} pinned ${messageNoun}`;
}, [threadInfo.pinnedCount]);
atulUnsubmitted
Not Done
Inline Actions

You don't need to use React.useMemo(...) here.

The useMemo() hook is helpful for maintaining referential equality so that objects will be considered "shallowly equal" (== in JS) and we can avoid re-renders. This is helpful for objects (including Map(), Set(), etc), arrays (which are objects), and functions (which are objects).

On the other hand, strings in JS are considered shallowly equal if they have the same contents, so we don't have to worry about re-renders if the "content" stays the same.

See below:

875403.png (620×810 px, 104 KB)

(In like C++, which you've been working w/ recently, std::strings can be allocated on the heap (unlike integers, booleans, etc) which may have been part of your reasoning that shallow equality would check reference instead of contents?)

atul: You don't need to use `React.useMemo(...)` here. The `useMemo()` hook is helpful for…
const pinnedCountBanner = React.useMemo(() => {
if (!bannerText) {
return null;
}
return (
<View style={styles.pinnedCountBanner}>
<Text style={styles.pinnedCountText}>{bannerText}</Text>
<Icon
name="chevron-right"
size={12}
color={colors.panelBackgroundLabel}
/>
</View>
);
}, [
bannerText,
styles.pinnedCountBanner,
styles.pinnedCountText,
colors.panelBackgroundLabel,
]);
return ( return (
<MessageListContextProvider threadInfo={threadInfo}> <MessageListContextProvider threadInfo={threadInfo}>
{pinnedCountBanner}
<MessageListContainer <MessageListContainer
{...props} {...props}
usernameInputText={usernameInputText} usernameInputText={usernameInputText}
updateUsernameInput={updateUsernameInput} updateUsernameInput={updateUsernameInput}
userInfoInputArray={userInfoInputArray} userInfoInputArray={userInfoInputArray}
updateTagInput={updateTagInput} updateTagInput={updateTagInput}
resolveToUser={resolveToUser} resolveToUser={resolveToUser}
otherUserInfos={otherUserInfos} otherUserInfos={otherUserInfos}
Show All 14 Lines
View Options

native/navigation/route-names.js

// @flow // @flow
import Icon from '@expo/vector-icons/FontAwesome5.js';
import { useNavigationState } from '@react-navigation/native'; import { useNavigationState } from '@react-navigation/native';
import invariant from 'invariant'; import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View, Text } from 'react-native';
import genesis from 'lib/facts/genesis.js'; import genesis from 'lib/facts/genesis.js';
import { threadInfoSelector } from 'lib/selectors/thread-selectors.js'; import { threadInfoSelector } from 'lib/selectors/thread-selectors.js';
import { import {
userInfoSelectorForPotentialMembers, userInfoSelectorForPotentialMembers,
userSearchIndexForPotentialMembers, userSearchIndexForPotentialMembers,
} from 'lib/selectors/user-selectors.js'; } from 'lib/selectors/user-selectors.js';
import { getPotentialMemberItems } from 'lib/shared/search-utils.js'; import { getPotentialMemberItems } from 'lib/shared/search-utils.js';
▲ Show 20 Lines • Show All 189 Lines • ▼ Show 20 Lines if (this.frozen) {
this.pendingListDataWithHeights = listDataWithHeights; this.pendingListDataWithHeights = listDataWithHeights;
} else { } else {
this.setState({ listDataWithHeights }); this.setState({ listDataWithHeights });
} }
}; };
} }
const unboundStyles = { const unboundStyles = {
pinnedCountBanner: {
backgroundColor: 'panelForeground',
height: 30,
flexDirection: 'row',
textAlign: 'center',
justifyContent: 'center',
alignItems: 'center',
},
pinnedCountText: {
color: 'panelBackgroundLabel',
marginRight: 5,
},
container: { container: {
backgroundColor: 'listBackground', backgroundColor: 'listBackground',
flex: 1, flex: 1,
}, },
threadContent: { threadContent: {
flex: 1, flex: 1,
}, },
hiddenThreadContent: { hiddenThreadContent: {
▲ Show 20 Lines • Show All 124 Lines • ▼ Show 20 Lines ) {
const styles = useStyles(unboundStyles); const styles = useStyles(unboundStyles);
const overlayContext = React.useContext(OverlayContext); const overlayContext = React.useContext(OverlayContext);
const measureMessages = useHeightMeasurer(); const measureMessages = useHeightMeasurer();
const genesisThreadInfo = useSelector( const genesisThreadInfo = useSelector(
state => threadInfoSelector(state)[genesis.id], state => threadInfoSelector(state)[genesis.id],
); );
const bannerText = React.useMemo(() => {
if (!threadInfo.pinnedCount || threadInfo.pinnedCount === 0) {
return '';
}
const messageNoun = threadInfo.pinnedCount === 1 ? 'message' : 'messages';
return `${threadInfo.pinnedCount} pinned ${messageNoun}`;
}, [threadInfo.pinnedCount]);
atulUnsubmitted
Not Done
Inline Actions

You don't need to use React.useMemo(...) here.

The useMemo() hook is helpful for maintaining referential equality so that objects will be considered "shallowly equal" (== in JS) and we can avoid re-renders. This is helpful for objects (including Map(), Set(), etc), arrays (which are objects), and functions (which are objects).

On the other hand, strings in JS are considered shallowly equal if they have the same contents, so we don't have to worry about re-renders if the "content" stays the same.

See below:

875403.png (620×810 px, 104 KB)

(In like C++, which you've been working w/ recently, std::strings can be allocated on the heap (unlike integers, booleans, etc) which may have been part of your reasoning that shallow equality would check reference instead of contents?)

atul: You don't need to use `React.useMemo(...)` here. The `useMemo()` hook is helpful for…
const pinnedCountBanner = React.useMemo(() => {
if (!bannerText) {
return null;
}
return (
<View style={styles.pinnedCountBanner}>
<Text style={styles.pinnedCountText}>{bannerText}</Text>
<Icon
name="chevron-right"
size={12}
color={colors.panelBackgroundLabel}
/>
</View>
);
}, [
bannerText,
styles.pinnedCountBanner,
styles.pinnedCountText,
colors.panelBackgroundLabel,
]);
return ( return (
<MessageListContextProvider threadInfo={threadInfo}> <MessageListContextProvider threadInfo={threadInfo}>
{pinnedCountBanner}
<MessageListContainer <MessageListContainer
{...props} {...props}
usernameInputText={usernameInputText} usernameInputText={usernameInputText}
updateUsernameInput={updateUsernameInput} updateUsernameInput={updateUsernameInput}
userInfoInputArray={userInfoInputArray} userInfoInputArray={userInfoInputArray}
updateTagInput={updateTagInput} updateTagInput={updateTagInput}
resolveToUser={resolveToUser} resolveToUser={resolveToUser}
otherUserInfos={otherUserInfos} otherUserInfos={otherUserInfos}
Show All 14 Lines