Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
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[bot] Dec 19, 2017
7136a60
docs(readme): add Greenkeeper badge
greenkeeper[bot] 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[bot] Dec 22, 2017
f7f99b2
chore(package): update jest to version 22.0.4
greenkeeper[bot] 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[bot] Jan 11, 2018
6358a14
chore(package): update eslint-plugin-react-native to version 3.2.1 (#…
greenkeeper[bot] Jan 11, 2018
0ca7358
chore(package): update jest to version 22.0.6 (#697)
greenkeeper[bot] Jan 11, 2018
483d288
chore(package): update babel-jest to version 22.0.6 (#696)
greenkeeper[bot] 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
cbd70c2
chore(screenshot): add screen border for 2
xcarpentier Jan 11, 2018
07ad31b
Update README.md
xcarpentier Jan 11, 2018
1c60140
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
2146795
Update README.md
xcarpentier Jan 12, 2018
a72daee
chore(package): update babel-jest to version 22.1.0 (#701)
greenkeeper[bot] Jan 15, 2018
db937d0
chore(package): update jest to version 22.1.0 (#702)
greenkeeper[bot] Jan 15, 2018
22072ea
chore(package): update jest to version 22.1.1 (#703)
greenkeeper[bot] 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 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
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
"moment": "^2.19.0",
"prop-types": "15.5.10",
"react-native-communications": "2.2.1",
"react-native-invertible-scroll-view": "^1.1.0",
"react-native-lightbox": "^0.7.0",
"react-native-parsed-text": "^0.0.19",
"shallowequal": "1.0.2",
Expand Down
109 changes: 47 additions & 62 deletions src/MessageContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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( {
Copy link

@dralletje dralletje Nov 26, 2017

Choose a reason for hiding this comment

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

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 :)

Choose a reason for hiding this comment

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

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) {
Expand All @@ -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)

Choose a reason for hiding this comment

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

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

Copy link

@redwind redwind Nov 8, 2017

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

Choose a reason for hiding this comment

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

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() {
Expand Down Expand Up @@ -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);

Choose a reason for hiding this comment

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

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}]}}
Copy link

@dralletje dralletje Nov 26, 2017

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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>
);
Expand Down