-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
Make email address on user profiles a mailto link #3203
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if the tooltip is needed, but ok for me.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like the issue asks, I would also add this to the user profile. For the Dodona admins, this can be useful as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now, the emailaddress is also colored as a link. Personally I think this is a bit too attention-grabbing, especially on the profile pages. @bmesuere opinions?
edit: adding the text-muted class is ok for me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you rebase on latest develop when making changes? That way the checks will run for your changes.
@@ -32,7 +32,7 @@ | |||
<%= link_to user.full_name, course_member_path(@course, user), title: user.full_name, class: "ellipsis-overflow" %> | |||
<% end %> | |||
</td> | |||
<td><%= user.email %></td> | |||
<td><%= mail_to(user.email, nil, {title: t('.send_mail'), class: 'text-muted'}) %></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can fill in this nil
argument, we have the name of the user (here and in the other instances). I would also not add the text-muted class here since the link works visually in our tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced nil
with the user email. I would like to continue displaying the e-mail, because not everyone has their mailto client configured properly. I removed text-muted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not realize filling in the argument would change the shown content of the a
. In that case, leaving it empty seems like the better option. I'll change this, approve and merge.
I had some trouble with the rebase because of multiple laptops, but it should be ok now. |
Closes #3201.