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

[NEW] Command /hide to hide channels #10727

Merged
merged 8 commits into from
Jun 11, 2018
Merged

[NEW] Command /hide to hide channels #10727

merged 8 commits into from
Jun 11, 2018

Conversation

mikaelmello
Copy link
Contributor

@mikaelmello mikaelmello commented May 9, 2018

Closes #10713
Closes #8239

Simple feature requested in #10713, when an user executes the slash command /hide, they hide the channel and are redirected to the homepage.

image

I've decided to redirect the user to the homepage after hiding because that is the current behavior when you hide the current room you are in using the GUI.

@mikaelmello
Copy link
Contributor Author

@karlprieb @ggazzo I've been recommended to ask one of you to review it, if possible

Copy link
Member

@ggazzo ggazzo left a comment

Choose a reason for hiding this comment

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

what do you think about allow hide another room typing /hide #room_name ? I dont think will take much more time.

'rocketchat:lib'
]);

api.addFiles('hide.js', 'client');
Copy link
Member

Choose a reason for hiding this comment

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

change to api.addFiles('client/hide.js', 'client'); (and move the file =) )

return;
}

try {
Copy link
Member

Choose a reason for hiding this comment

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

can you explain why you are using this try catch? Meteor.call doest throw exceptions on client side, you should use

Meteor.call('hideRoom', item.rid, err => {
  if(err) {
    return RocketChat.Notifications.notifyUser(Meteor.userId(), 'mes'.......
  } 
  if (['channel', 'group', 'direct'].includes(FlowRouter.getRouteName()) && (Session.get('openedRoom') === item.rid)) {
			FlowRouter.go('home');
			Session.delete('openedRoom');
		}
}) ```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the slashcommands-leave as a base, there's a try catch there so I figured I should do the same too. Will update it soon, along with the other suggestions

Copy link
Contributor

@graywolf336 graywolf336 left a comment

Choose a reason for hiding this comment

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

We need a server implementation of this, otherwise the mobile applications won't have access to this command.

- Move client function to client/ directory
- Change error handling of the method call
@mikaelmello
Copy link
Contributor Author

@ggazzo and @graywolf336, all done.

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10727 May 25, 2018 19:04 Inactive
@ggazzo
Copy link
Member

ggazzo commented May 28, 2018

@mikaelmello I made some changes could test and see if you agree?

@mikaelmello
Copy link
Contributor Author

@ggazzo Much better. I had written Hide_given_room to match the join command, but if you prefer it that way alright. About Room_doesnt_exist, it was because of direct messages.

@ggazzo
Copy link
Member

ggazzo commented May 28, 2018

Actually, the sentence 'Hide_given_room' is better but, we have a lot of strings, and probably we will change in the future

ggazzo
ggazzo previously approved these changes May 28, 2018
@ggazzo ggazzo added this to Desireable in June/2018 via automation May 28, 2018
@ggazzo ggazzo added this to the 0.66.0 milestone May 28, 2018
@ggazzo ggazzo moved this from Desireable to Ready to merge in June/2018 May 29, 2018
@engelgabriel engelgabriel dismissed graywolf336’s stale review June 4, 2018 08:20

moved to server side

@ggazzo ggazzo added the ui/ux label Jun 10, 2018
@ggazzo ggazzo merged commit acc671e into RocketChat:develop Jun 11, 2018
June/2018 automation moved this from Ready to merge to Closed Jun 11, 2018
@rodrigok rodrigok mentioned this pull request Jun 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
June/2018
  
Closed
4 participants