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

Support threaded messages and reactions #91

Closed
yangmillstheory opened this issue Mar 2, 2018 · 38 comments
Closed

Support threaded messages and reactions #91

yangmillstheory opened this issue Mar 2, 2018 · 38 comments

Comments

@yangmillstheory
Copy link

https://get.slack.help/hc/en-us/articles/115000769927-Message-threads

@jpbruinsslot
Copy link
Owner

I'll have a look if this is possible to implement, however I'm afraid that it will mess up the chat view.

@jpbruinsslot jpbruinsslot changed the title Support threaded messages Support threaded messages and reactions Mar 18, 2018
@jpbruinsslot
Copy link
Owner

Unfortunately, this is not going to work. The only way I can get replies/reactions is from the RTM events which will send the message, and an extra message, which mentions that a reply was made. However, the api (when switching channels it will get messages from the api) doesn't relay that a message was in reply to another message. As far as I can see, it will only say if a message has replies or not, but not the id to that message. (https://godoc.org/github.com/nlopes/slack#Msg)

That would mean that I'm able to gather, from incoming message events (RTM), that a reply has been made to a message, but when changing the channel it will clear this information and is lost. This will also happen when restarting the application.

This will only work when I'm able to get the id's of the replies, or the id of the message that the replies are made to. When this is the case I can do the following: message structs from nlopes/slack need to be kept in memory (as is done with channels). This will allows updating of message and the subsequent replies to be rendered correctly in the chat view.

@peyloride
Copy link

@erroneousboat any update on this?

@jpbruinsslot
Copy link
Owner

I'll have a look if anything has changed

@Ferenc-
Copy link

Ferenc- commented Dec 12, 2018

This feature would be highly appreciated,
as in my organization 90% of the communication happens in threaded messages,
and this way I can't fully switch to the otherwise very nice slack-term.

@jpbruinsslot
Copy link
Owner

Ok, it is possible, but I need to rethink how messages are handled in slack-term. I'll put it on the road map.

@jpbruinsslot jpbruinsslot self-assigned this Dec 22, 2018
@jpbruinsslot jpbruinsslot added this to the v0.4.2 milestone Dec 22, 2018
@egegunes
Copy link

Ok, it is possible, but I need to rethink how messages are handled in slack-term.

If you explain a bit, I can start working on it.

@jpbruinsslot
Copy link
Owner

Hey, thank you for the offer, but it isn't necessary. I'm working on it on the branch: threads

@jpbruinsslot
Copy link
Owner

A basic version is now available in the threads, if you'd like you can test it out.

@Ferenc-
Copy link

Ferenc- commented Jan 11, 2019

It"s great that now the threaded messages are visible.
is it just me who didn't find out how to reply in a thread?
My responses ended up in the channel.

@jpbruinsslot
Copy link
Owner

No it's not you, I haven't implemented that functionality. At the moment it is a basic version and I try to figure out how to best handle threads.

@Ferenc-
Copy link

Ferenc- commented Jan 12, 2019

Keep up the good work! Tell me if I can help with testing.

@jpbruinsslot
Copy link
Owner

jpbruinsslot commented Jan 15, 2019

Can I ask several of you how you typically use threads with slack, because at the moment I'm at bit of an impasse on this.

At the moment (in the threads branch) I've added threads inside the main chat view, so that will mean that new message received, the thread gets updated and the message gets inline with all the other messages. However, I can imagine that this gets confusing when messages get appended that are not in view. Replying on threads also gets complicated, because you probably need to reply to a specific thread in a channel, which can have multiple threads. That would mean something like typing /reply {thread-num} hello, world.

A potential solution could be, when you navigate to a channel you would retrieve all threads in a channel. A new column beneath the channels list will appear with a list of threads which you then can select and reply to.

So I would like to know if this is something that might be sufficient for those that use threads intensively, and I would thus like to know how you typically use and navigate threads at the moment. It might give me some insight on how to implement this.

Thanks in advance

@Ferenc-
Copy link

Ferenc- commented Jan 25, 2019

I didn't want to do all the talking, but if nobody else has comments, then I guess I can provide some feedback.

So, your proposed potential solution, the new column with the list of the threads in the selected channel
sounds good and sufficient at this point.

Perhaps in the distant future, and in a new feature request the "My Threads" feature could also be implemented, which would list all the threads that the user replied to at least once, regardless of the channel.

For some reason this view is labelled as "All Threads" in the electron based Slack client,
but I think that the "All" is a bit misleading here.

@xescugc
Copy link

xescugc commented Jan 25, 2019

IMO what @erroneousboat mentions is enough (something is better that nothing hehe), but I don't see where do you mean A new column beneath the channels list exactly.

And also which will be the "name" of the thread? or they'll have numbers (based on the /reply {thread-num} hello, world that you mentioned) because maybe it's difficult to know the "scope" of that thread. If possible it would be good to know which is the message that this thread is attached to.

IDK just some ideas hehe.

@jpbruinsslot
Copy link
Owner

Hey I just wanted to thank you both for your suggestions. I'll review and start implementing when I'm less bogged down with other responsibilities.

@jpbruinsslot
Copy link
Owner

Hey, I've updated the branch with some reply to thread functionality. You will see a thread id in front of the thread, and that id you can use to reply to it. Change to insert mode and type the following:

/reply [enter-thread-id-here] hello, world

It is not extensively tested and perhaps rough around the edges, but I wanted to get it out to you guys.

@davidjahn
Copy link

Hi, I just compiled the thread branch and I'm testing it out. Works great!

My main feedback (from UX point of view) is that it's a bit unwieldy to type out the entire thread ID (eg /reply thread_5c6d75a6 my message... One possible UX solution would be to display/rewrite the internal thread IDs as incrementing integers (where 1 is most recent thread, 2 is next oldest thread, etc... so then the command to reply to most recent thread would become

/reply thread-1 my response

or even easier:

/thread 1 my response

@kitplummer
Copy link

Progress. Would it be possible to redraw the thread with a new message to the bottom of the view? Thanks for diggin in on this.

@jpbruinsslot
Copy link
Owner

@davidjahn Cool, I'm glad it is workable at the moment! Good idea about the thread number and command, I'll implement that.

@kitplummer You're welcome of course. I'll have a look if that would be something that could work. I'm not sure if it will be more easier to follow though, but I understand where you're coming from. That said, I'm still looking into making a separate list of threads that will be available when you select a channel that contains threads, which you then can select and reply to. I think that might even be better than the implemented solution at the moment.

jpbruinsslot added a commit that referenced this issue Mar 16, 2019
* threads:
  Update comment
  Update threads reply functionality
  Add reply to threads functionality
  Fix thread timestamp checking
  Fix pagination problem
  Update handling thread replies in event handler
  Update message event
  Implement new Message setup
  Start with thread support

Reference #91
@jpbruinsslot
Copy link
Owner

jpbruinsslot commented Mar 16, 2019

I've merged the current functionality into master, and I'm developing a different setup in threads-v2

@moertel
Copy link

moertel commented Apr 17, 2019

@erroneousboat thanks a lot! I'll give it a shot. Just for convenience: probably worth publishing this as a new release on GitHub.

@xescugc
Copy link

xescugc commented Jun 3, 2019

Just to report a bug related to threads hehe. The "ID/Code" of the thread only appears if you "refresh" the view (move up and down on the channels to refresh the channel) if not, the Thread appears where it should be but it's impossible to respond to it.

If you want I can open a separated issue for this one, and IDK if it's solved ton the threads-v2 though.

@jpbruinsslot
Copy link
Owner

@xescugc yeah it is one of the reason for the threads-v2 implementation, it would have a new panel on the right with the thread id's that appear in the specific channel

@jpbruinsslot
Copy link
Owner

jpbruinsslot commented Jun 29, 2019

I've made an update to the threads-v2 branch. You'll be able to try it out. When changing channels from the main chat view, and threads are present in that channel. A pane to the right side will show. From there you can select the thread by using Shift+j or Shift+k. When you've selected a thread you'll be able to reply to it by sending a message.

Note that threads will continue to show inline with conversations in the main chat view and the initial functionality of replying to a thread with /reply is still there.

This is still rough around the edges, and there will be potential bugs. But if you want to test it out, you can. It has been rebased to the latest version of master.

@egegunes
Copy link

egegunes commented Jul 1, 2019

Thank you for the update. I got a new build from threads-v2 and will try today. At first look everything seems OK but I couldn't figure out how to deselect a thread except by changing channels. Also can I configure thread bar width?

@xescugc
Copy link

xescugc commented Jul 1, 2019

Also can I configure thread bar width?

I would say yes c5e2d5d#diff-3baf47c64847a8fb8aaa8cc2e088513bR25

@jpbruinsslot
Copy link
Owner

Thank you for the update. I got a new build from threads-v2 and will try today. At first look everything seems OK but I couldn't figure out how to deselect a thread except by changing channels. Also can I configure thread bar width?

Deselecting is indeed done by changing channels. And as @xescugc mentioned you can change the width of the thread by by adding threads_width and a value.

@xescugc
Copy link

xescugc commented Jul 1, 2019

@erroneousboat if it's ok with you I'll report the things I found here, if it's not the right place indicate me the right one :)

  • Each time I move to a new channel, all the UI refreshed instead of just the Body as it was before (makes a weird flashy effect)
  • If you go to a channel without threads, the threads column appears and after that disappears immediately
  • Already commented, but be able to return to the root conversation, maybe have the first "thread" be the channel itself, as if you don't move the first one(thread) is selected but you're not on the thread itself (when you move to a new channel) 😄
  • It's faster to move up on the threads than down (feels like down is fetching before moving and up moving and then fetching)

For now I've found this 😄 If I find something else I'll report it.

Amazing BTW!

@jpbruinsslot
Copy link
Owner

@erroneousboat if it's ok with you I'll report the things I found here, if it's not the right place indicate me the right one :)

  • Each time I move to a new channel, all the UI refreshed instead of just the Body as it was before (makes a weird flashy effect)
  • If you go to a channel without threads, the threads column appears and after that disappears immediately
  • Already commented, but be able to return to the root conversation, maybe have the first "thread" be the channel itself, as if you don't move the first one(thread) is selected but you're not on the thread itself (when you move to a new channel) smile
  • It's faster to move up on the threads than down (feels like down is fetching before moving and up moving and then fetching)

For now I've found this If I find something else I'll report it.

Amazing BTW!

Yeah no problem if you report it here.

  1. True, has to do with that I need to redraw the interface in order to get a new pane to show :( But, I might have an idea how to optimize it.
  2. Ok, haven't seen that yet, but I keep that in mind when optimizing point 1.
  3. Good idea, I'll see if I can implement that!
  4. Yeah you're right haven't noticed that before, I'll check that out.

And of course thanks for testing it out!

@xescugc
Copy link

xescugc commented Jul 1, 2019

Another one, if you move to the 4th position on a thread, that position is the same one on each channel you go, and if there is no 4th position no thread is selected by default and if you move:

panic: runtime error: index out of range

goroutine 929 [running]:
github.com/erroneousboat/slack-term/handlers.actionChangeThread(0xc00020c000)
	/go/src/github.com/erroneousboat/slack-term/handlers/event.go:543 +0x30d
github.com/erroneousboat/slack-term/handlers.actionMoveCursorDownThreads.func1(0xc00020c000)
	/go/src/github.com/erroneousboat/slack-term/handlers/event.go:588 +0x124
created by github.com/erroneousboat/slack-term/handlers.actionMoveCursorDownThreads
	/go/src/github.com/erroneousboat/slack-term/handlers/event.go:576 +0x3f

Basically the selected thread position is not reseted when switching channels, that would go together with the 3rd item on the list I mentioned before :)

jpbruinsslot added a commit that referenced this issue Sep 7, 2019
jpbruinsslot added a commit that referenced this issue Sep 7, 2019
jpbruinsslot added a commit that referenced this issue Sep 7, 2019
@jpbruinsslot
Copy link
Owner

@xescugc It has been a minute. I've updated the branch with fixes of the issues you came across.

jpbruinsslot added a commit that referenced this issue Sep 14, 2019
@xescugc
Copy link

xescugc commented Oct 3, 2019

Ok so I've been testing for a while and I've found this:

  • Each time I move to a new channel, all the UI refreshed instead of just the Body as it was before (makes a weird flashy effect) still
  • When you are inside a thread, if some one writes on the root channel you also see it (without the identation) but it should not be displayed as it's not part of the thread
  • If some one edits a thread happens the same as before, it's displayed as if it was on the root channel.
  • If some one opens a thread you have to reload to see thread on the right (or to see the ID of the thread).

Would be cool to also see the * of new messages on the threads too hehe.

I would say this is all I've found right now

jpbruinsslot added a commit that referenced this issue Oct 5, 2019
@jpbruinsslot
Copy link
Owner

Hey, thanks again for testing it out. I've made some changes on master.

  • Each time I move to a new channel, all the UI refreshed instead of just the Body as it was before (makes a weird flashy effect) still

This is unfortunately going to be present in some form. When changing channels, and the next channel contains a thread. The view needs to get updated in order to add the threads pane to the right. I've made it so that flashing shouldn't occur when changing channels when there is no threads present in the next channel.

  • When you are inside a thread, if some one writes on the root channel you also see it (without the identation) but it should not be displayed as it's not part of the thread

Fixed

  • If some one edits a thread happens the same as before, it's displayed as if it was on the root channel.

Fixed

  • If some one opens a thread you have to reload to see thread on the right (or to see the ID of the thread).

Fixed, although this requires reloading of the interface, which results in a "flash".

Would be cool to also see the * of new messages on the threads too hehe.

I'll have a look for this implementation.

@xescugc
Copy link

xescugc commented Oct 31, 2019

Now when we write on a thread we get redirected to the "root" view of the conversateion, I'm gonna say it has to do with the Fixed, although this requires reloading of the interface, which results in a "flash".

For the rest I think it's ok

I'll have a look for this implementation.

That would be cool hehe because if it's a big implementation you don't see which is the actual update.

@faelin
Copy link

faelin commented Jan 14, 2020

Does the threads-v2 branch still exist? I get a 404 when I follow the link.

@jpbruinsslot
Copy link
Owner

@faelin The functionality has been merged into master

@jpbruinsslot jpbruinsslot removed this from the v0.4.2 milestone Mar 14, 2020
@jpbruinsslot
Copy link
Owner

Closing since functionality has been released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants