-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Display @ before mentions #7324
Display @ before mentions #7324
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.
LGTM
Thanks! :) |
Just upgraded d-fr, I find weird that the @ is not part of the link to the user profile. |
So we have two different ways to do it: as part of the link (like github or twitter does it), or before the link (as google+ does it (with a "+" instead of an "@"), or as friendica did it, when sending mentions as markdown-link in the past). The difference between github/twitter and diaspora/google+ is, that github/twitter display usernames, and diaspora/google+ displays the realname. Because the realname also could contain (or start with) an "@" and the realname can contain spaces, I thought it would be better to not include the "@" in the link and stick it to the first part of the realname. For #tags we add the "#" also to the link, but #tags also don't allow special chars or spaces. But since @denschub also told me, that he would include it in the link, we should probably change it, and add the "@" to the link? I can do it this evening or we can wait if there are other opinions? |
Oh okay, nice to know the thinking behind the choice. Well it looks better in my opinion but I understand the point "display name" vs "username". |
So what do you think? Should we change it? Or should we keep it for a week as it is now, and wait if people get used to it, or if there is more feedback about it? For me both ways are OK, but I preferred the current solution because of the reasons above. |
This has been merged to the develop branch, so I'd say it's ok to keep it for a while and re-evaluate later. |
Yeah, it's landed, let's leave it there for a bit. If it turns out it is particularly annoying, we can always change it later on. |
Fixes #7269.