Skip to content

Fix for keeping a room hidden after activity#6299

Closed
kable-wilmoth wants to merge 1 commit intoRocketChat:developfrom
kable-wilmoth:HidenRoom
Closed

Fix for keeping a room hidden after activity#6299
kable-wilmoth wants to merge 1 commit intoRocketChat:developfrom
kable-wilmoth:HidenRoom

Conversation

@kable-wilmoth
Copy link

@RocketChat/core

Currently you can mark a room as hidden and it will be removed from your channel list, even after a browser refresh. BUT, if any activity occurs in this hidden channel, it no longer remains hidden.

This fix keeps the channel hidden w/ normal channel activity. Other things (notification settings) will make the room visible (mentions, etc.) but that is a different concern.

update =
$set:
alert: true
open: true
Copy link
Author

Choose a reason for hiding this comment

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

setAlertForRoomIdExcludingUserId is only called in one place and there was no expectation of the side effect of updating the room 'open' property.

Copy link
Member

Choose a reason for hiding this comment

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

This was not a bug :) it was intended. If you want a different behaviour, we need to make it configurable.

@engelgabriel
Copy link
Member

This was not a bug :) it was intended. If you want a different behaviour, we need to make it configurable.

@kable-wilmoth
Copy link
Author

Oh, I had no idea. I couldn't find any docs about it and the code and comments didn't make it obvious. So the current behavior just hides it until there is activity. This was the intent? So I assume people probably hid rooms that they were a member of but are not very active?
I will look into end user configurations for this. Any pointers on where to learn about this in the code? Any suggestions for the name of this configuration?
Show hidden rooms when unread messages? (default true)

@engelgabriel
Copy link
Member

We copied the behavior from Skype at the time. Maybe we could change the wording to CLOSE rather than HIDE?

@engelgabriel
Copy link
Member

Hummm.. On a second thought, CLOSE may be even more confusing...

@kable-wilmoth
Copy link
Author

My specific use case for our team.
Our company has standardized on Rocket, but a lot of teams still use Slack (especially for the mobile client/speed). So these teams have their channels 'bridged' so that Rocket users can also participate in them.
So for these teams, their primary usage is through Slack, but they need to check Rocket throughout the day for other corporate communications out their team.
Their slack channels are bridged into rocket, so even though they have 'read' all the team channels throughout the day, in Rocket, they have all their channels flagged w/ messages and it is really annoying.
So they have tried to use the 'hide' channel feature, because they really don't interact with those channels via Rocket. This works, until someone posts a message, then the channel shows back up w/ lots of messages marked as un-read.
With my change, this use case works great, but I do understand it wasn't as designed and the SlackBridge is a corner case.

What did you think of my suggested setting?
Show hidden rooms when unread messages? (default true)

@marceloschmidt
Copy link
Member

@kable-wilmoth I like your proposal of "Show hidden rooms when unread messages"

Copy link
Member

@marceloschmidt marceloschmidt left a comment

Choose a reason for hiding this comment

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

I would love to see the suggested changes for "Show hidden rooms when unread messages"

@kable-wilmoth kable-wilmoth deleted the HidenRoom branch September 1, 2017 04:40
@engelgabriel
Copy link
Member

Hi @kable-wilmoth

I know your latest PRs were open for far too long, but we were busy with implementing the new UX and were trying to catch up, so I am sorry for that. Having said that, is there any reason why you closed all the open PRs?

Copy link
Contributor

@lindoelio lindoelio left a comment

Choose a reason for hiding this comment

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

I thought it was already implemented in the current version, but I was wrong :-) I can increment the parameter in the settings.

@lindoelio
Copy link
Contributor

lindoelio commented Sep 6, 2017

Hey guys! Let me know what you think about my suggestions for this feature on PR #8053.
@kable-wilmoth @marceloschmidt @engelgabriel

@engelgabriel engelgabriel added this to the 0.60.0 milestone Sep 6, 2017
@lindoelio lindoelio self-assigned this Sep 14, 2017
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.

4 participants

Comments