Skip to content

Append read receipt feature for #3411#4047

Closed
dezzzus wants to merge 2 commits into
RocketChat:developfrom
dezzzus:patch-1
Closed

Append read receipt feature for #3411#4047
dezzzus wants to merge 2 commits into
RocketChat:developfrom
dezzzus:patch-1

Conversation

@dezzzus

@dezzzus dezzzus commented Aug 17, 2016

Copy link
Copy Markdown
Contributor

@RocketChat/core

Closes #3411

Append read receipt info to message entry.

Append read receipt info to message entry.
@tgxn

tgxn commented Aug 18, 2016

Copy link
Copy Markdown
Contributor

Hey, this is great, but it looks like it depends on the code in your other PR #4048? (for the setMessageReader function)

@rodrigok

Copy link
Copy Markdown
Member

Hi @alexdorn87, first of all, thanks by this contribution.

I have some questions/suggestions about this PR

  1. Can you merge your 2 PRs in one since they are dependents?
  2. I like the idea, but this should be optional, can you create a setting for that and disabled by default?
  3. You can improve your code by removing your loop on messages and changing the update to affect all desired messages in one single query, I can help you with this if you want.
  4. Will you change the interface to show this list of users?
  5. Maybe we should add a setting to enable this feature to only channels with a certain number of users, we have bad experiences with large arrays, think in rooms with thousands of users.

@RocketChat/core what do you think?

@geekgonecrazy

Copy link
Copy Markdown
Contributor

This would be a great addition!

I agree large rooms this could be very bad for experience. Maybe a setting to only enable for rooms with users <= x ? And let users define it. Then default it to like 10-15 ?

@dezzzus

dezzzus commented Aug 18, 2016

Copy link
Copy Markdown
Contributor Author

hi #rodrigok

Thanks for your detailed message.
1)
Could you help to me to know how to do it, please?

I will make it, but now I am some busy.
If you don't mind, please make it, too.
Or not, I will make it myself
3)
Now I don't know what you mean.
If you can do it, please help me.
thanks
4)
I changed already on my server.
But I don't commit it, because I don't know the design will be accept by your team.
5)
Yes, That's good idea.
I hope to interact with you more and make more completed chat server.
I am appreciate for your open source chat server.
I hope to get more touch with you.

thanks, guys.

@engelgabriel

Copy link
Copy Markdown
Member

@alexdorn87 can you update all the indentation to TABs?

@xavierzwirtz

Copy link
Copy Markdown

If I stepped in and cleaned this pull request up would the RocketChat team be willing to pull it in? This feature is vital for the way my team uses chat, but I don't want to be running a custom version.

@engelgabriel engelgabriel added this to the 0.50.0 milestone Jan 16, 2017
@engelgabriel

Copy link
Copy Markdown
Member

Yes @xavierzwirtz we'd like to add this feature, if you can make the changes, we'd love to merge it.

@xavierzwirtz

Copy link
Copy Markdown

Awesome.

@alexdorn87, could you post the UI modifications you made somewhere?

@dezzzus

dezzzus commented Jan 18, 2017

Copy link
Copy Markdown
Contributor Author

Hi @xavierzwirtz

which UI modification do you need?
Could you give me more details, please?

@xavierzwirtz

Copy link
Copy Markdown

The UI for displaying which users have seen a message. I've got the changes you have pushed cleaned up and rebased on tip. Just need the UI changes.

@engelgabriel engelgabriel modified the milestones: 0.50.0, Short-term Jan 24, 2017
@RichardFoxworthy

Copy link
Copy Markdown

any progress on this one @alexdorn87 and @xavierzwirtz ? Will be a great new piece of functionality!

@xavierzwirtz

Copy link
Copy Markdown

I was able to rebase the changes to the server, and I can see the data getting sent back to the server whenever a message is read. However, I am not familiar with the UI framework that is being used within rocket chat, and have not had time to get familiar with it. I was hoping that @alexdorn87 could post up the UI changes that he made here so that I can get them integrated as well.

@dezzzus

dezzzus commented Feb 21, 2017

Copy link
Copy Markdown
Contributor Author

I posted all files.
if you need more, please tell me more details

@xavierzwirtz

Copy link
Copy Markdown

@alexdorn87, you said that

I changed (the UI) already on my server.
But I don't commit it, because I don't know the design will be accept by your team.

Could you post the changes that you made for that? I can get them into a PR and have the RocketChat review them.

@RichardFoxworthy

Copy link
Copy Markdown

Hi again @alexdorn87 and @xavierzwirtz - is there an easy way forward to complete this PR? Would be a great feature addition to Rocket.chat!

@ArchimedesFM

Copy link
Copy Markdown

This would be a great feature addition to Rocket.chat!

@HammyHavoc

Copy link
Copy Markdown
Contributor

Would love to see this in RC, and completely agree about keeping it disabled by default, and defaulting to only on rooms with a handful of users.

@engelgabriel

Copy link
Copy Markdown
Member

@sampaiodiego can you link this on the PR you are about to create for this feature?

@engelgabriel

Copy link
Copy Markdown
Member

Closed via #9717

@engelgabriel engelgabriel modified the milestones: Short-term, 0.62.0 May 5, 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.

read receipts?

9 participants