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

Modified for FlatList #629

Closed
wants to merge 84 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
84 commits
Select commit Hold shift + click to select a range
1d9ca95
Turn off inverted scroll view
magfurulabeer Jun 22, 2017
11d91ad
Update GiftedChat.js
magfurulabeer Jun 22, 2017
10b8e6a
Modified for FlatList
Nov 6, 2017
33dc31f
Removed invertible-scroll-view from the dependencies
gavin-gmlab Nov 21, 2017
6aa5c86
add Invert GiftedChat prop
apparition47 Dec 7, 2017
e177c39
loadEarlier Component always at top irregardless of props.inverted value
apparition47 Dec 7, 2017
ca4c700
Add docs for inverted prop
apparition47 Dec 9, 2017
7157e8e
Merge pull request #489 from magfurulabeer/master
brunocascio Dec 9, 2017
6a7c4bd
Merge branch 'master' of https://github.com/FaridSafi/react-native-gi…
apparition47 Dec 9, 2017
528aa0e
Update GiftedChat.js
xcarpentier Dec 15, 2017
45f3763
Merge pull request #658 from apparition47/inverted-prop
brunocascio Dec 15, 2017
8efd338
fix(inverted): change style according inverted new props
xcarpentier Dec 15, 2017
1518e21
fix(append/prepend): add inverted in signature
xcarpentier Dec 18, 2017
28cb467
chore(linting): linting all
xcarpentier Dec 18, 2017
5b67203
fix(error): on runtime
xcarpentier Dec 18, 2017
fd63b02
chore(ci): add circle-ci conf
xcarpentier Dec 18, 2017
43e30a6
feat(Colors.js): use it everywhere
xcarpentier Dec 18, 2017
6dff834
fix(console): removed missed warn
xcarpentier Dec 18, 2017
aefde5c
fix(staless component): missed props
xcarpentier Dec 18, 2017
961c5d8
chore(ci): yarn cache path
xcarpentier Dec 18, 2017
e5f0cb2
fix(naming): Color and Constant
xcarpentier Dec 18, 2017
3f09d38
fix(Color): naming
xcarpentier Dec 18, 2017
4571dda
fix(lint): after upgradeC
xcarpentier Dec 18, 2017
ce91bf3
chore(lint): de-order main file method
xcarpentier Dec 18, 2017
587319e
fix(lint): LoaderEarlier scrunched
xcarpentier Dec 18, 2017
1cf028d
fix(lint): GiftedChat
xcarpentier Dec 18, 2017
da1cf48
chore(hook): add husky
xcarpentier Dec 18, 2017
ca47d62
doc(badge): add badge and align content on readme
xcarpentier Dec 18, 2017
20d8bd0
Added nodejs & react versions
brunocascio Dec 18, 2017
8fe384a
chore(circleci): modify node version
xcarpentier Dec 18, 2017
9745e81
fix(circleci): version to 8.2.0
xcarpentier Dec 18, 2017
0c046de
Merge pull request #668 from FaridSafi/linting
xcarpentier Dec 18, 2017
3c38612
Update README.md
xcarpentier Dec 18, 2017
ebe5a62
Update README.md
xcarpentier Dec 19, 2017
37762b0
Update README.md
xcarpentier Dec 19, 2017
163f731
chore(package): update dependencies
greenkeeper Dec 19, 2017
7136a60
docs(readme): add Greenkeeper badge
greenkeeper Dec 19, 2017
86d90e9
Merge pull request #670 from FaridSafi/greenkeeper/initial
xcarpentier Dec 19, 2017
da43d5d
Move greenkeeper badge in better place
xcarpentier Dec 19, 2017
0ddacde
Add contributors section
xcarpentier Dec 19, 2017
6ea779b
Add demo on snack / expo
xcarpentier Dec 19, 2017
940ff59
Update README.md
xcarpentier Dec 19, 2017
c0281d5
.vscode
xcarpentier Dec 20, 2017
5e94773
Merge branch 'master' of https://github.com/FaridSafi/react-native-gi…
xcarpentier Dec 20, 2017
e054648
Add Jest tests and Codecov reports (#673)
xcarpentier Dec 20, 2017
de89d2c
Update README.md add codecov badge
xcarpentier Dec 20, 2017
ed765b4
fix(typos): carot => carrot
xcarpentier Dec 20, 2017
86046e3
Merge branch 'master' of https://github.com/FaridSafi/react-native-gi…
xcarpentier Dec 20, 2017
349a4a0
Add questions sections to README.md
xcarpentier Dec 21, 2017
4def71f
Add a questions in section README.md
xcarpentier Dec 21, 2017
d50392a
Add questions in section README.md
xcarpentier Dec 21, 2017
806b0de
chore(package): update babel-jest to version 22.0.4
greenkeeper Dec 22, 2017
f7f99b2
chore(package): update jest to version 22.0.4
greenkeeper Dec 22, 2017
f877a96
Merge pull request #675 from FaridSafi/greenkeeper/babel-jest-22.0.4
xcarpentier Dec 22, 2017
7538d4a
Merge pull request #676 from FaridSafi/greenkeeper/jest-22.0.4
xcarpentier Dec 22, 2017
14562a1
Add a question
xcarpentier Dec 22, 2017
edf8fff
Add example "Slack" UI (#649)
cooperka Dec 23, 2017
a476408
Unnecessary semicolon removed (#677)
devshekhawat Dec 24, 2017
68a0c3c
Update .eslintignore
xcarpentier Dec 25, 2017
b388d9f
fix(onPressActionButton): no default (#684)
xcarpentier Dec 29, 2017
9fe28b5
Update README.md
xcarpentier Jan 8, 2018
fd3bfef
Update README.md
xcarpentier Jan 9, 2018
eec38d8
Update README.md
xcarpentier Jan 9, 2018
ee21489
chore(package): update jest to version 22.0.5 (#693)
greenkeeper Jan 11, 2018
6358a14
chore(package): update eslint-plugin-react-native to version 3.2.1 (#…
greenkeeper Jan 11, 2018
0ca7358
chore(package): update jest to version 22.0.6 (#697)
greenkeeper Jan 11, 2018
483d288
chore(package): update babel-jest to version 22.0.6 (#696)
greenkeeper Jan 11, 2018
36ca1a3
Update README.md
xcarpentier Jan 11, 2018
31c3bde
Update README.md
xcarpentier Jan 11, 2018
142055a
Update README.md
xcarpentier Jan 11, 2018
19069ce
chore(screenshot): add screen border
xcarpentier Jan 11, 2018
82d9329
Merge branch 'master' of https://github.com/FaridSafi/react-native-gi…
xcarpentier Jan 11, 2018
07ad31b
Update README.md
xcarpentier Jan 11, 2018
cbd70c2
chore(screenshot): add screen border for 2
xcarpentier Jan 11, 2018
1c60140
Update README.md
xcarpentier Jan 12, 2018
2146795
Update README.md
xcarpentier Jan 12, 2018
3b0fdd4
Update README.md
xcarpentier Jan 12, 2018
907205f
doc(readme): add screenshot
xcarpentier Jan 12, 2018
e281b8a
Merge branch 'master' of https://github.com/FaridSafi/react-native-gi…
xcarpentier Jan 12, 2018
a72daee
chore(package): update babel-jest to version 22.1.0 (#701)
greenkeeper Jan 15, 2018
db937d0
chore(package): update jest to version 22.1.0 (#702)
greenkeeper Jan 15, 2018
22072ea
chore(package): update jest to version 22.1.1 (#703)
greenkeeper Jan 15, 2018
1ea1775
chore(lint): merge some new lint
xcarpentier Jan 15, 2018
8ff8f3a
chore(test): update jest snapshot
xcarpentier Jan 15, 2018
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+47 −62
Diff settings

Always

Just for now

Copy path View file
@@ -2,13 +2,12 @@ import PropTypes from 'prop-types';
import React from 'react';

import {
ListView,
FlatList,
View,
StyleSheet,
} from 'react-native';

import shallowequal from 'shallowequal';
import InvertibleScrollView from 'react-native-invertible-scroll-view';
import md5 from 'md5';
import LoadEarlier from './LoadEarlier';
import Message from './Message';
@@ -20,37 +19,22 @@ export default class MessageContainer extends React.Component {
this.renderRow = this.renderRow.bind(this);
this.renderFooter = this.renderFooter.bind(this);
this.renderLoadEarlier = this.renderLoadEarlier.bind(this);
this.renderScrollComponent = this.renderScrollComponent.bind(this);

const dataSource = new ListView.DataSource({
rowHasChanged: (r1, r2) => {
return r1.hash !== r2.hash;
}
});

const messagesData = this.prepareMessages(props.messages);
this.state = {
dataSource: dataSource.cloneWithRows(messagesData.blob, messagesData.keys)
};
messagesData: this.prepareMessages(props.messages)
};
}

prepareMessages(messages) {
return {
keys: messages.map(m => m._id),
blob: messages.reduce((o, m, i) => {
const previousMessage = messages[i + 1] || {};
const nextMessage = messages[i - 1] || {};
// add next and previous messages to hash to ensure updates
const toHash = JSON.stringify(m) + previousMessage._id + nextMessage._id;
o[m._id] = {
...m,
previousMessage,
nextMessage,
hash: md5(toHash)
};
return o;
}, {})
};
return output = messages.reduce((o,m,i) => {
const previousMessage = messages[i + 1] || {}
const nextMessage = messages[i - 1] || {}
o.push( {

This comment has been minimized.

Copy link
@dralletje

dralletje Nov 26, 2017

Two small questions here:

  1. It seems to me this is a lot simpler done using .map, maybe even more performant? (Dunno if that even matters here)

  2. Because you merge previousMessage and nextMessage props onto the existing message, that message will be "invalidated" when matching it against existing messages (causing PureComponent or shouldComponentUpdate to always see change). I'd recommend setting { currentMessage, previousMessage, nextMessage } and properly using item.currentMessage instead of just item in https://github.com/FaridSafi/react-native-gifted-chat/pull/629/files#diff-bd8395a0a94eca8b61a34ec66f8a605cR102

It might be that I miss something why this is necessary, let me know :)

This comment has been minimized.

Copy link
@dralletje

dralletje Nov 26, 2017

About the second one, I see that Message isn't PureComponent yet, but I think it is still good to these optimisations already, if we want Message to be an effective PureComponent later ;)

https://github.com/gavin-gmlab/react-native-gifted-chat/blob/33dc31f41d29be98e441c4b27ad74e453b9801b5/src/Message.js#L16

...m,
previousMessage,
nextMessage
})
return o
},[])
}

shouldComponentUpdate(nextProps, nextState) {
@@ -67,10 +51,9 @@ export default class MessageContainer extends React.Component {
if (this.props.messages === nextProps.messages) {
return;
}
const messagesData = this.prepareMessages(nextProps.messages);
this.setState({
dataSource: this.state.dataSource.cloneWithRows(messagesData.blob, messagesData.keys)
});
messagesData: this.prepareMessages(nextProps.messages)

This comment has been minimized.

Copy link
@sondremare

sondremare Nov 6, 2017

This will cause all chat messages to re-render every time a new message is added.

This comment has been minimized.

Copy link
@redwind

redwind Nov 8, 2017

i think we should remove prepareMessages and move set value for nextMessage and previousMessage into renderItem function

This comment has been minimized.

Copy link
@gavin-gmlab

gavin-gmlab Nov 21, 2017

Author

My thoughts:
Its not worse than what it was with the ListView approach. Correct me if I am wrong since the old approach was rerendering the whole ListView anyway. FlatList is supposed to render only what is actually viewable anyway, and not those components that are outside the view, so we are getting an improvement right there.

I was more thinking of getting a quick fix in for now, and we can always have perf improvements later? But just my opinion.

This comment has been minimized.

Copy link
@dralletje

dralletje Nov 26, 2017

It is not the prepareMessages that will cause everything (in the view) to rerender, that will happen anyway when a new message arrives. What we can do about that is make the components being rendered pure, and allow them to be effective PureComponent-s

})
}

renderFooter() {
@@ -99,66 +82,68 @@ export default class MessageContainer extends React.Component {
}

scrollTo(options) {
this._invertibleScrollViewRef.scrollTo(options);
this.refs.flatListRef.scrollToOffset(options)
}

renderRow(message, sectionId, rowId) {
if (!message._id && message._id !== 0) {
console.warn('GiftedChat: `_id` is missing for message', JSON.stringify(message));
renderRow({item,index}) {
if (!item._id && item._id !== 0) {
console.warn('GiftedChat: `_id` is missing for message', JSON.stringify(item));
}
if (!message.user) {
if (!message.system) {
console.warn("GiftedChat: `user` is missing for message", JSON.stringify(message));
if (!item.user) {
if (!item.system) {
console.warn("GiftedChat: `user` is missing for message", JSON.stringify(item));
}
message.user = {};
item.user = {};
}

const messageProps = {
...this.props,
key: message._id,
currentMessage: message,
previousMessage: message.previousMessage,
nextMessage: message.nextMessage,
position: message.user._id === this.props.user._id ? 'right' : 'left',
key: item._id,
currentMessage: item,
previousMessage: item.previousMessage,
nextMessage: item.nextMessage,
position: item.user._id === this.props.user._id ? 'right' : 'left',
};

if (this.props.renderMessage) {
return this.props.renderMessage(messageProps);

This comment has been minimized.

Copy link
@dralletje

dralletje Nov 26, 2017

Is it on purpose that this is outside of the { transform: [{ scaleY: -1 }] } ? I don't think people will want to have to set that themselves, and when we go to use invert={true} on the container later, the people that do have their own { transform: [{ scaleY: -1 }] } will have to remove it again and ughhhhh hahaha :)

}
return <Message {...messageProps}/>;
}

renderScrollComponent(props) {
const invertibleScrollViewProps = this.props.invertibleScrollViewProps;
return (
<InvertibleScrollView
{...props}
{...invertibleScrollViewProps}
ref={component => this._invertibleScrollViewRef = component}
/>
);
<View style={{ transform: [{ scaleY: -1 },{perspective: 1280}]}}>
<Message {...messageProps}/>
</View>
)
}

renderHeaderWrapper = () => {
return <View style={{ flex: 1, transform: [{ scaleY: -1 },{perspective: 1280}] }}>{this.renderLoadEarlier()}</View>;
};

_keyExtractor = (item, index) => item._id+" "+index

render() {
return (
<View
ref='container'
style={styles.container}
>
<ListView
<FlatList
enableEmptySections={true}
automaticallyAdjustContentInsets={false}
initialListSize={20}
pageSize={20}

ref='flatListRef'
keyExtractor={this._keyExtractor}
{...this.props.listViewProps}

dataSource={this.state.dataSource}
data={this.state.messagesData}

renderRow={this.renderRow}
renderItem={this.renderRow}
renderHeader={this.renderFooter}
renderFooter={this.renderLoadEarlier}
renderScrollComponent={this.renderScrollComponent}
renderFooter={this.renderLoadEarlier()}
style={{transform: [{scaleY: -1 },{perspective: 1280}]}}

This comment has been minimized.

Copy link
@dralletje

dralletje Nov 26, 2017

One last thing, what is the reasoning behind using this transform ourselves, and not using inverted={true}? I'm not sure why it not in the documentation (so you may very well know something I don't) :-)

But this would solve the problem of us having to set the transform on all the items, and it would also spare us a view per item (FlatList applies inverted styles on the view it would create anyway, we introduce a new one :-/)

I understand if it is for version support: I think inverted only is supported in a couple of last version

https://github.com/facebook/react-native/blob/master/Libraries/Lists/FlatList.js#L142

This comment has been minimized.

Copy link
@sondremare

sondremare Nov 26, 2017

Inverted works only on RN 0.49 and above. Atleast for Huawei phones running Android 7

This comment has been minimized.

Copy link
@dralletje

dralletje Nov 26, 2017

Then maybe it is even a combination of the fact that inverted got added later + the perspective fix... Maybe good to wait some version before we use inverted, but still worth adding a TODO/NOTE?

{...this.props.invertibleScrollViewProps}
ListFooterComponent={this.renderHeaderWrapper}
/>
</View>
);
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.