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

added name/link of channel/room in email notifications #3814

Merged

Conversation

danielrohers
Copy link
Contributor

@RocketChat/core

Closes E-Mail notifications: include name/link of channel/room #3704

Hello, add link to the channel that was mentioned...

screen shot 2016-07-19 at 13 48 31

@sampaiodiego
Copy link
Member

thanks for the contribution @danielrohers

I don't like the idea of repating the subject on email body, specially because it adds the "server name" (which is already in the body header)..

I think we should change the translations for Offline_Mention_Email and Offline_DM_Email to not include the [__site__].. So we can use it in both places (subject and body) but adding the "server name" where it's needed. What you think?

@danielrohers
Copy link
Contributor Author

I think it's a good idea @sampaiodiego

Is there any way to make this change with i18n files or I need to do manually?

I saw not treated the DM situation, I will adjust it too.

@sampaiodiego
Copy link
Member

Is there any way to make this change with i18n files or I need to do manually?

@danielrohers have to do manually 😞 but a replace in all files should do the trick 😃

@danielrohers
Copy link
Contributor Author

@sampaiodiego preview messages. Site_Name believe to be interesting in the subject to easily see the contexts...

screen shot 2016-07-20 at 20 20 54

screen shot 2016-07-20 at 20 21 10

@sampaiodiego
Copy link
Member

cool @danielrohers

can you please rebase and fix the conflicts?

@rodrigok
Copy link
Member

Hi @danielrohers, what do you think about adding a center (big and green?) button bellow all text with text "Go to channel" instead of that small link?

Another thing, can you remove that > on the top of the email?

@danielrohers
Copy link
Contributor Author

danielrohers commented Jul 21, 2016

Hi @rodrigok,

Great! I better keep the "RC blue" color and with "Go to message" since it can use in both cases (DM/Channel).

What do you think?

@rodrigok
Copy link
Member

@danielrohers Ok, better, go ahead 😄

@danielrohers
Copy link
Contributor Author

Hey guys

I made the changes, I believe that really got much better :)

@sampaiodiego @rodrigok

screen shot 2016-07-25 at 11 47 34
screen shot 2016-07-25 at 11 47 43

'margin: auto;',
'margin-bottom: 8px;'
].join(' ');
return `<a style="${ style }" href="${ process.env.ROOT_URL }${ path }">GO TO MESSAGE</a>`;
Copy link
Member

Choose a reason for hiding this comment

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

@danielrohers can you change this to use TAPi18n to GO TO MESSAGE and add this text to en.i18n.json?

@rodrigok
Copy link
Member

Can you solve the conflicts too?

@rodrigok
Copy link
Member

rodrigok commented Aug 3, 2016

@danielrohers Awesome man :)

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