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

Added "alwaysRenderAvatar" prop to "Avatar.js" #297

Closed
wants to merge 3 commits into from

Conversation

eomine
Copy link

@eomine eomine commented Dec 5, 2016

Currently, the Avatar component hides the avatar of a message if it's from the same day and from the same user of the previous message.

This prop allows users to disable this behavior, so that the avatar is rendered on all messages.

The default value is false, which keeps the current behavior.

In order to render the avatar on all messages, one should set the alwaysRenderAvatar prop to true when placing the GiftedChat component.

Copy link
Contributor

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Nice @eomine!

Could you please update also the Readme Props section?

BTW, it seems that this PR should be merged after #292.

@eomine eomine changed the title Added "hideAvatarWhenSameDayAndUser" prop to "Avatar.js" Added "alwaysRenderAvatar" prop to "Avatar.js" Dec 5, 2016
@eomine
Copy link
Author

eomine commented Dec 5, 2016

@dgdavid Thanks!

Changed the prop to a shorter name. Its value must be set to true now, which seems more straightforward.

Updated the Readme.

@@ -63,6 +67,7 @@ const styles = {
};

Avatar.defaultProps = {
alwaysRenderAvatar: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool! Looks better now. I was thinking a shorter name but no idea came to my mind ;)

README.md Outdated
@@ -118,6 +118,7 @@ See [example/App.js](example/App.js)
- **`renderSend`** _(Function)_ - render the send button
- **`renderAccessory`** _(Function)_ - renders a second line of actions below the message composer
- **`bottomOffset`** _(Integer)_ - distance of the chat from the bottom of the screen, useful if you display a tab bar
- **`alwaysRenderAvatar`** _(Bool)_ - always render the message avatar, even when consecutive messages are from same day and same user
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be interesting to indicate the default value.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, updated the readme with the default value.

@prithsharma
Copy link

@dgdavid since #292 is merged now, is there anything else blocking this PR right now?
Thanks for the update, in advance.

@kfiroo
Copy link
Collaborator

kfiroo commented Mar 28, 2017

@prithsharma Looking at the code I'm not sure it will help you after all :(
This only overrides the avatar visibility for received message.

@dgdavid
Copy link
Contributor

dgdavid commented Mar 28, 2017

@prithsharma It would be nice to use the utils functions after import them instead rely on props in order to follow the way introduced by @scarmuega in #292.

For example, replacing this.props.isSameUser with isSameUser after import { isSameUser } from './utils'.

But these days I'm a little out of the loop to think/discuss the viability of this PR, so once again we need the @kfiroo support.

@cooperka cooperka force-pushed the master branch 5 times, most recently from 092db6b to 730bab3 Compare July 16, 2017 23:58
@zachrnolan
Copy link

Hey @eomine @dgdavid @kfiroo any updates on this? Would love to see this feature merged.

@dgdavid
Copy link
Contributor

dgdavid commented Dec 24, 2017

@xcarpentier

Since you have now a good knowledte of project, could please take a look to this PR and give your opinion about what to do in order to fix and merge or reject it?

Thanks!

@hironarita
Copy link

Will this eventually be merged?

@apparition47
Copy link
Contributor

@hironarita #729 was merged

@eomine eomine closed this Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants