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

[Fix] Don't @ mention when doing reply in DM #7347

Merged
merged 5 commits into from
Jun 30, 2017
Merged

Conversation

geekgonecrazy
Copy link
Contributor

@geekgonecrazy geekgonecrazy commented Jun 26, 2017

@RocketChat/core

Previously doing a reply in a DM mentioned them. Which is pointless. You don't need to mention the person you're sending the message directly to :)

There might be a better way of determining if the room type is DM. If so please suggest :)

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7347 June 26, 2017 21:33 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7347 June 26, 2017 21:54 Inactive
@RocketChat RocketChat deleted a comment Jun 26, 2017
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

thanks for this.. ❤️ I always remove the @mention 😂

let text = `[ ](${ url }) `;

if (Session.get('openedRoom').indexOf(Meteor.userId()) === -1) {
text += `@${ message.u.username }`;
Copy link
Member

Choose a reason for hiding this comment

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

you're not adding an additional space after @mention as the previous code did.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops! I was even thinking about it, I think I went back in and cleaned it out by mistake :(

Fixed now 👍

const text = `[ ](${ url }) @${ message.u.username } `;
let text = `[ ](${ url }) `;

if (Session.get('openedRoom').indexOf(Meteor.userId()) === -1) {
Copy link
Member

Choose a reason for hiding this comment

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

I think it's safe doing RocketChat.models.Rooms.findOne(message.rid, { fields: { t: 1 } }) to get room's type =)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a much cleaner way of doing it. Thanks!

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7347 June 27, 2017 16:19 Inactive
@geekgonecrazy
Copy link
Contributor Author

geekgonecrazy commented Jun 27, 2017

@sampaiodiego Yes it always bugged me too! I couldn't take it any more I had to fix it 😂

Could you take another look? 😄

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7347 June 27, 2017 16:51 Inactive
@sampaiodiego
Copy link
Member

what if you reply to a message sent by yourself? 🤔

@marceloschmidt
Copy link
Member

marceloschmidt commented Jun 29, 2017

I think auto-replies should also remove @ my username

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7347 June 29, 2017 22:03 Inactive
@geekgonecrazy
Copy link
Contributor Author

@sampaiodiego @marceloschmidt ok if you reply to self now doesn't mention :)

@rodrigok rodrigok added this to the 0.58.0 milestone Jun 30, 2017
@rodrigok rodrigok merged commit dc795d3 into develop Jun 30, 2017
@rodrigok rodrigok deleted the fix-reply-in-dm branch June 30, 2017 19:39
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.

5 participants