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

Typing notification in main thread #70

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

maxime-netsyst
Copy link

Hello,

I added the typing effect to the main thread when the bot is requested.
To activate it, use the env variable MATTERMOST_TYPE_IN_MAIN_THREAD=true.
The default value is false.

@yGuy
Copy link
Owner

yGuy commented Mar 19, 2024

Thanks!

I think this was always intended to be the way it should work, but it didn't because this is wrong?

 const typing = () => wsClient.userTyping(msgData.post.channel_id, (msgData.post.root_id || msgData.post.id) ?? "")

So for the case when we are in main, we should just use "" ?

Then let's just fix that and make that unconditional - I don't think anyone wants to have this configurable - and the diff will be much smaller, too!

@maxime-netsyst
Copy link
Author

To have the typing notification only in the main thread yes.
But maybe you should put the notification in the main and the other thread.

@yGuy
Copy link
Owner

yGuy commented Mar 21, 2024

But showing the typing both in the main as well as in the thread is not what you would expect. Only for the first reply...

So wouldn't it suffice to just change the original line to:

 const typing = () => wsClient.userTyping(msgData.post.channel_id, msgData.post.id ?? "")

or is it

 const typing = () => wsClient.userTyping(msgData.post.channel_id, msgData.post.root_id ?? "")

and that's it? No other change required and you get the typing notification where the bot is actually responding to?

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.

2 participants