External links etc#198
Conversation
|
|
||
| <mat-card-title> | ||
| <a class="card-title" [attr.href]="orgUnit.link ? orgUnit.link : null"> | ||
| <a class="card-title" tabindex="0" (keydown.enter)="orgUnit.link ? navigateTo(orgUnit.link) : null"> |
There was a problem hiding this comment.
So here the title is clickable but does no action?
There was a problem hiding this comment.
The whole card is clickable, so the title doesn't need to have the link - I realise this seems kind of odd, but the problem is that if the title does have the link then you get a new tab opening the link as well as the hub navigating to the link (double navigation). The keydown.enter is needed to navigate when the title is focused.
There was a problem hiding this comment.
Alternative is that the external links open in the current tab, but this feels a bit weird to me? What do you think? I feel like we should keep the user on the ResearchHub and open links in new tabs. But maybe that's just my personal preference making me biased :)
There was a problem hiding this comment.
@rosemcc An alternative, and possibly better, way to handle accessibility would be to make tabbing focus the whole card and then let aria-label read out the title of the card. Therefore the titles wouldn't need to be anchors any longer
This works fine though
robmint
left a comment
robmint
left a comment
There was a problem hiding this comment.
Looks good, and deleting of stuff good!
| {{ contentItem.title }} | ||
| <span>{{ contentItem.title }} | ||
| | ||
| <mat-icon *ngIf="contentItem.ssoProtected" matTooltip="SSO Login Required"> |
There was a problem hiding this comment.
Suggest we remove SSO - as this is not really necessary and might be confusing to some people
There was a problem hiding this comment.
Might need to discuss with team? For users that don't have access I'd say it's needed so they can understand why they can't access this content. But we haven't collected specific user feedback about it so can't be sure.
| type="search" | ||
| (keydown.enter)="search()" | ||
| #trigger="matAutocompleteTrigger" | ||
| (keydown.enter)="search(); trigger.closePanel()" |
|
Question: @robmint @cameronmclean should asset hyperlinks be indicated as external links? Or are they technically internal links? Example towards the bottom of this page: https://research-hub-dev.connect.test.amazon.auckland.ac.nz/article/open-access |
| <mat-card-title> | ||
| <a | ||
| class="card-title" | ||
| [attr.href]="document.url ?? document.document?.url" |
There was a problem hiding this comment.
I don't know why there are 2 URL fields in the first place, but this ternary expression existed before the refactor, therefore I kept it. Are you sure we can remove this logic?
There was a problem hiding this comment.
The two url fields are because some documents are hosted on Contentful, and some are just linked to externally, and the respective urls are shown in different fields because of that.
| * - do not apply to link cards or funding application docs | ||
| * - show the mail icon for email links within rich text | ||
| */ | ||
| #main-content a:not([href^="/"]):not([href*="//research-hub"]):not([href*="@"]):not([href^="tel"]):not(.standard-button):not(app-blocks-embedded-asset *):not(.link-card *):not(.application-doc *):after { |
There was a problem hiding this comment.
I didn't know you could make css look like perl. I like it! 😄
Trombach
left a comment
Trombach
left a comment
There was a problem hiding this comment.
Apart from some minor comments, this LGTM!
3 participants
This PR contains the following changes:
Feature
Improvements