Change profile view's display name and detailed status's timestamp to link to canonical URL #20991
Conversation
| <div className='account__header__tabs__name'> | ||
| <a className='account__header__tabs__name' href={`${account.get('url')}`} target='_blank' rel='noopener noreferrer'> | ||
| <h1> | ||
| <span dangerouslySetInnerHTML={displayNameHtml} /> {badge} | ||
| <small>@{acct} {lockedIcon}</small> | ||
| </h1> | ||
| </div> | ||
| </a> |
There was a problem hiding this comment.
I don't know if I'd have that be a link.
There was a problem hiding this comment.
The reasoning behind this was to give users an easy way to open the remote profile without menu diving, but I'm open to alternative approaches too
| externalLink = ( | ||
| <React.Fragment> | ||
| <React.Fragment> </React.Fragment> | ||
| <Icon id='external-link' /> |
There was a problem hiding this comment.
Maybe a title and/or aria-label?
There was a problem hiding this comment.
Something like “External link”, “External source” or “Original source”?
There was a problem hiding this comment.
Added, in what I assume is the correct way but idk really
| @@ -276,8 +288,9 @@ class DetailedStatus extends ImmutablePureComponent { | |||
| {media} | |||
|
|
|||
| <div className='detailed-status__meta'> | |||
| <a className='detailed-status__datetime' href={`/@${status.getIn(['account', 'acct'])}\/${status.get('id')}`} target='_blank' rel='noopener noreferrer'> | |||
| <a className='detailed-status__datetime' href={`${status.get('url')}`} target='_blank' rel='noopener noreferrer' title='${intl.formatMessage(messages.canonical_link)}' aria-label='${intl.formatMessage(messages.canonical_link)}'> | |||
There was a problem hiding this comment.
Hmmm I'm not too sure about adding a label/title here.
There was a problem hiding this comment.
My thinking is that you can hover over the timestamp, so you should be able to see a hint about what it leads to? The external icon is just part of the link at the end
There was a problem hiding this comment.
I really don't think aria-label is appropriate here, the thing's primary purpose is that it's a timestamp, it's not a visual-only thing either, I think aria-label would incorrectly obscure the timestamp.
There was a problem hiding this comment.
Would a title but no aria-label be appropriate, or should there be a different aria-label?
|
This pull request has merge conflicts that must be resolved before it can be merged. |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
Follow-up to #20540 -- since these links are in essence "terminal", it can be argued that users may not expect them to open a new tab on their local server, but instead to open the canonical URL.