Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MM-34585 Use removeClippedSubviews from Lists to improve scroll perf #5199

Merged
merged 5 commits into from
Apr 8, 2021
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
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -550,14 +550,14 @@ workflows:
- test
filters:
branches:
only: /^build-pr-.*/
only: /^(build|android)-pr-.*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this PR be reverted at some point? If so, consider having these changes in a separate PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to keep this, that way if a PR fails for a particular platform we can just build the platform that we need

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yea, I meant that this specific change can be in a separate PR if you foresee the FPS changes being reverted in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe ;)

- build-ios-pr:
context: mattermost-mobile-ios-pr
requires:
- test
filters:
branches:
only: /^build-pr-.*/
only: /^(build|ios)-pr-.*/

- build-android-unsigned:
context: mattermost-mobile-unsigned
Expand Down
11 changes: 6 additions & 5 deletions app/components/autocomplete/at_mention/at_mention.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,15 +251,16 @@ export default class AtMention extends PureComponent {

return (
<SectionList
testID='at_mention_suggestion.list'
keyboardShouldPersistTaps='always'
keyExtractor={this.keyExtractor}
style={[style.listView, {maxHeight: maxListHeight}]}
sections={sections}
renderItem={this.renderItem}
renderSectionHeader={this.renderSectionHeader}
initialNumToRender={10}
nestedScrollEnabled={nestedScrollEnabled}
removeClippedSubviews={true}
renderItem={this.renderItem}
renderSectionHeader={this.renderSectionHeader}
style={[style.listView, {maxHeight: maxListHeight}]}
sections={sections}
testID='at_mention_suggestion.list'
/>
);
}
Expand Down
11 changes: 6 additions & 5 deletions app/components/autocomplete/channel_mention/channel_mention.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,15 +225,16 @@ export default class ChannelMention extends PureComponent {

return (
<SectionList
testID='channel_mention_suggestion.list'
keyboardShouldPersistTaps='always'
keyExtractor={this.keyExtractor}
style={[style.listView, {maxHeight: maxListHeight}]}
sections={sections}
renderItem={this.renderItem}
renderSectionHeader={this.renderSectionHeader}
initialNumToRender={10}
nestedScrollEnabled={nestedScrollEnabled}
removeClippedSubviews={true}
renderItem={this.renderItem}
renderSectionHeader={this.renderSectionHeader}
style={[style.listView, {maxHeight: maxListHeight}]}
sections={sections}
testID='channel_mention_suggestion.list'
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3036,7 +3036,7 @@ exports[`components/autocomplete/emoji_suggestion should match snapshot 2`] = `
numColumns={1}
onEndReachedThreshold={2}
pageSize={10}
removeClippedSubviews={false}
removeClippedSubviews={true}
renderItem={[Function]}
scrollEventThrottle={50}
style={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ export default class EmojiSuggestion extends PureComponent {
extraData={this.state}
data={this.state.dataSource}
keyExtractor={this.keyExtractor}
removeClippedSubviews={true}
renderItem={this.renderItem}
pageSize={10}
initialListSize={10}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ exports[`components/autocomplete/slash_suggestion should match snapshot 1`] = `
nestedScrollEnabled={false}
numColumns={1}
onEndReachedThreshold={2}
removeClippedSubviews={false}
removeClippedSubviews={true}
renderItem={[Function]}
scrollEventThrottle={50}
style={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ export default class SlashSuggestion extends PureComponent<Props, State> {
extraData={this.state}
data={this.state.dataSource}
keyExtractor={this.keyExtractor}
removeClippedSubviews={true}
renderItem={this.renderItem}
nestedScrollEnabled={nestedScrollEnabled}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ exports[`components/emoji_picker/emoji_picker.ios should match snapshot 1`] = `
onScroll={[Function]}
onScrollToIndexFailed={[Function]}
pageSize={50}
removeClippedSubviews={false}
removeClippedSubviews={true}
renderItem={[Function]}
renderSectionHeader={[Function]}
scrollEventThrottle={50}
Expand Down
9 changes: 5 additions & 4 deletions app/components/emoji_picker/emoji_picker_base.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,15 +264,16 @@ export default class EmojiPicker extends PureComponent {

listComponent = (
<FlatList
contentContainerStyle={contentContainerStyle}
data={filteredEmojis}
initialListSize={10}
keyboardShouldPersistTaps='always'
keyExtractor={this.flatListKeyExtractor}
initialListSize={10}
ListEmptyComponent={this.renderEmptyList}
nativeID={SCROLLVIEW_NATIVE_ID}
pageSize={10}
renderItem={this.flatListRenderItem}
ListEmptyComponent={this.renderEmptyList}
contentContainerStyle={contentContainerStyle}
removeClippedSubviews={true}
style={styles.flatList}
/>
);
Expand All @@ -292,7 +293,7 @@ export default class EmojiPicker extends PureComponent {
onScroll={this.onScroll}
onScrollToIndexFailed={this.handleScrollToSectionFailed}
pageSize={50}
removeClippedSubviews={false}
removeClippedSubviews={true}
renderItem={this.renderItem}
renderSectionHeader={this.renderSectionHeader}
sections={emojis}
Expand Down
6 changes: 3 additions & 3 deletions app/components/post_list/__snapshots__/post_list.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ exports[`PostList setting channel deep link 1`] = `
tintColor="#3d3c40"
/>
}
removeClippedSubviews={false}
removeClippedSubviews={true}
renderItem={[Function]}
scrollEventThrottle={60}
style={
Expand Down Expand Up @@ -134,7 +134,7 @@ exports[`PostList setting permalink deep link 1`] = `
tintColor="#3d3c40"
/>
}
removeClippedSubviews={false}
removeClippedSubviews={true}
renderItem={[Function]}
scrollEventThrottle={60}
style={
Expand Down Expand Up @@ -211,7 +211,7 @@ exports[`PostList should match snapshot 1`] = `
tintColor="#3d3c40"
/>
}
removeClippedSubviews={false}
removeClippedSubviews={true}
renderItem={[Function]}
scrollEventThrottle={60}
style={
Expand Down
2 changes: 1 addition & 1 deletion app/components/post_list/post_list.js
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ export default class PostList extends PureComponent {
onScrollToIndexFailed={this.handleScrollToIndexFailed}
ref={this.flatListRef}
refreshControl={refreshControl}
removeClippedSubviews={false}
removeClippedSubviews={true}
renderItem={this.renderItem}
scrollEventThrottle={60}
style={styles.flex}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ class FilteredList extends Component {
<View style={styles.container}>
<SectionList
sections={dataSource}
removeClippedSubviews={true}
renderItem={this.renderItem}
renderSectionHeader={this.renderSectionHeader}
keyExtractor={this.keyExtractor}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ exports[`ChannelsList List should match snapshot 1`] = `
onEndReachedThreshold={2}
onScrollBeginDrag={[Function]}
onViewableItemsChanged={[Function]}
removeClippedSubviews={true}
renderItem={[Function]}
renderSectionHeader={[Function]}
scrollEventThrottle={50}
Expand Down
1 change: 1 addition & 0 deletions app/components/sidebars/main/channels_list/list/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ export default class List extends PureComponent {
ref={this.setListRef}
sections={sections}
contentContainerStyle={{paddingBottom}}
removeClippedSubviews={true}
renderItem={this.renderItem}
renderSectionHeader={this.renderSectionHeader}
keyboardShouldPersistTaps={'always'}
Expand Down
1 change: 1 addition & 0 deletions app/components/sidebars/main/teams_list/teams_list.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ export default class TeamsList extends PureComponent {
extraData={this.state.serverUrl}
contentContainerStyle={this.listContentPadding()}
data={teamIds}
removeClippedSubviews={true}
renderItem={this.renderItem}
keyExtractor={this.keyExtractor}
viewabilityConfig={VIEWABILITY_CONFIG}
Expand Down
1 change: 1 addition & 0 deletions app/screens/pinned_posts/pinned_posts.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export default class PinnedPosts extends PureComponent {
keyExtractor={this.keyExtractor}
keyboardShouldPersistTaps='always'
keyboardDismissMode='interactive'
removeClippedSubviews={true}
renderItem={this.renderPost}
onViewableItemsChanged={this.onViewableItemsChanged}
/>
Expand Down
1 change: 1 addition & 0 deletions app/screens/recent_mentions/recent_mentions.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export default class RecentMentions extends PureComponent {
keyExtractor={this.keyExtractor}
keyboardShouldPersistTaps='always'
keyboardDismissMode='interactive'
removeClippedSubviews={true}
renderItem={this.renderPost}
onViewableItemsChanged={this.onViewableItemsChanged}
/>
Expand Down
1 change: 1 addition & 0 deletions app/screens/saved_posts/saved_posts.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ export default class SavedPosts extends PureComponent {
keyExtractor={this.keyExtractor}
keyboardShouldPersistTaps='always'
keyboardDismissMode='interactive'
removeClippedSubviews={true}
renderItem={this.renderPost}
onViewableItemsChanged={this.onViewableItemsChanged}
/>
Expand Down
1 change: 1 addition & 0 deletions app/screens/search/__snapshots__/search.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ exports[`Search should match snapshot 1`] = `
onLayout={[Function]}
onScroll={[Function]}
onViewableItemsChanged={[Function]}
removeClippedSubviews={true}
renderSectionHeader={[Function]}
scrollEventThrottle={60}
sections={
Expand Down
1 change: 1 addition & 0 deletions app/screens/search/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,7 @@ export default class Search extends PureComponent {
<SectionList
ref={this.setListRef}
style={style.sectionList}
removeClippedSubviews={true}
renderSectionHeader={this.renderSectionHeader}
sections={sections}
keyboardShouldPersistTaps='always'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ exports[`Settings SelectTimezone should match snapshot 1`] = `
maxToRenderPerBatch={15}
numColumns={1}
onEndReachedThreshold={2}
removeClippedSubviews={false}
removeClippedSubviews={true}
renderItem={[Function]}
scrollEventThrottle={50}
updateCellsBatchingPeriod={50}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export default class Timezone extends PureComponent {
</View>
<FlatList
data={this.filteredTimezones(value)}
removeClippedSubviews={true}
renderItem={this.renderItem}
keyExtractor={this.keyExtractor}
getItemLayout={this.getItemLayout}
Expand Down
27 changes: 21 additions & 6 deletions app/utils/images.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,31 @@ export function openGalleryAtIndex(index, files) {
push: {
waitForRender: true,
sharedElementTransitions,
...Platform.select({ios: {
content: contentPush,
}}),
},
pop: {
content: contentPop,
},
},
};

if (Object.keys(contentPush).length) {
migbot marked this conversation as resolved.
Show resolved Hide resolved
options.animations.push = {
...options.animations.push,
...Platform.select({
android: contentPush,
ios: {
content: contentPush,
},
}),
};
}

if (Object.keys(contentPop).length) {
options.animations.pop = Platform.select({
android: contentPop,
ios: {
content: contentPop,
},
});
}

goToScreen(screen, '', passProps, options);
});
}
Expand Down
1 change: 1 addition & 0 deletions share_extension/screens/channel_list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ const ChannelList = ({intl}: ChannnelListProps) => {
style={styles.flex}
sections={sections}
ItemSeparatorComponent={renderItemSeparator}
removeClippedSubviews={true}
renderItem={renderItem}
renderSectionHeader={renderSectionHeader}
keyExtractor={keyExtractor}
Expand Down
1 change: 1 addition & 0 deletions share_extension/screens/team_list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const TeamList = () => {
testID='share_extension.team_list.screen'
data={teams}
ItemSeparatorComponent={renderItemSeparator}
removeClippedSubviews={true}
renderItem={renderItem}
keyExtractor={keyExtractor}
keyboardShouldPersistTaps='always'
Expand Down