Skip to content

External links etc #198

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.

Sign up for GitHub

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

Merged
rosemcc merged 15 commits into from
Dec 21, 2021
Merged

External links etc #198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
bf6df15
delete unused components
rosemcc Dec 14, 2021
04c2542
show external links icons
rosemcc Dec 14, 2021
0ef015d
tweaks to card titles for consistency
rosemcc Dec 14, 2021
deb5149
navigateTo changed to window.open, fix typo
rosemcc Dec 14, 2021
f2176af
minor fixes
rosemcc Dec 14, 2021
bf32313
close search autocomplete panel on enter keypress
rosemcc Dec 15, 2021
bd909a7
Merge branch 'master' into external-links-etc
rosemcc Dec 16, 2021
310bbd1
readme
rosemcc Dec 19, 2021
eb62ccf
external links icon not on phone numbers
rosemcc Dec 19, 2021
9e59ba5
Merge branch 'master' into external-links-etc
rosemcc Dec 19, 2021
2c4c5dd
fix link card external icon
rosemcc Dec 19, 2021
427e5e7
cards external links functionality and accessibility
rosemcc Dec 19, 2021
9085f05
fix tabbing on contact card
rosemcc Dec 20, 2021
02302fa
open assets in new tab
rosemcc Dec 20, 2021
2fc03b8
dont show external link icon on home page banner content
rosemcc Dec 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ A [Serverless Framework](https://www.serverless.com/) ([AWS Lambda](https://aws.
### 4. [terraform/](terraform/)
Terraform infrastructure as code to create the Research Hub AWS infrastructure.

### 4. [subhub-link-checker/](subhub-link-checker/)
### 5. [subhub-link-checker/](subhub-link-checker/)
A [Contentful App Framework app](https://www.contentful.com/developers/docs/extensibility/app-framework/) that checks to prevent cyclical content references in SubHubs.
## Deployment
CI/CD is achieved via the monorepo's [Jenkinsfile](Jenkinsfile) which is triggered when code is pushed to one of 4 Git branches (`sandbox`, `dev`, `test`, `prod`) corresponding to environments across the three [UoA AWS accounts](http://aws.auckland.ac.nz/) (`sandbox`, `nonprod`, `prod`).
Expand Down
1 change: 1 addition & 0 deletions research-hub-web/src/app/components/articles/article/article.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
<div *ngIf="article.banner">
<a
href="{{ article.callToAction }}"
target="_blank"
mat-flat-button
class="standard-button-banner"
>
Expand Down
2 changes: 1 addition & 1 deletion research-hub-web/src/app/components/cards/cards-common.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*/
.mat-card-title {
line-height: 20px;
a {
span {
color: $dark-grey;
&:hover {
border-bottom: solid 2px $dark-grey;
Expand Down
3 changes: 2 additions & 1 deletion research-hub-web/src/app/components/cards/contact-card/contact-card.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
<div style="margin-top: 12px"></div>

<mat-card-title>
<a class="card-title">
<a *ngIf="person.link" class="card-title" tabindex="0" (keydown.enter)="navigateTo(person.link)">
<span *ngIf="person.name">{{ person.name }}</span>
</a>
<span *ngIf="!person.link && person.name" class="card-title" tabindex="0">{{ person.name }}</span>
</mat-card-title>

<mat-card-content>
Expand Down
2 changes: 1 addition & 1 deletion research-hub-web/src/app/components/cards/contact-card/contact-card.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class ContactCardComponent {
constructor() { }

public navigateTo(url: string) {
location.href = url;
window.open(url, "_blank");
}

}
8 changes: 2 additions & 6 deletions research-hub-web/src/app/components/cards/document-card/document-card.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,8 @@
<div style="margin-top: 12px"></div>

<mat-card-title>
<a
class="card-title"
[attr.href]="document.url ?? document.document?.url"
Copy link
Copy Markdown
Contributor

@Trombach Trombach Dec 21, 2021

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Contributor Author

@rosemcc rosemcc Dec 21, 2021

Choose a reason for hiding this comment

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

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.

target="_blank"
>
{{ document.title }}
<a class="card-title" tabindex="0" (keydown.enter)="navigateTo(document.url)">
<span>{{ document.title }}</span>
</a>
</mat-card-title>
<mat-card-content>
Expand Down
2 changes: 1 addition & 1 deletion research-hub-web/src/app/components/cards/document-card/document-card.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ export class DocumentCardComponent {
constructor() { }

public navigateTo(url: Maybe<string> | undefined) {
if (url) location.href = url;
if (url) window.open(url, "_blank");
}
}
2 changes: 1 addition & 1 deletion research-hub-web/src/app/components/cards/org-unit-card/org-unit-card.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<div style="margin-top: 12px"></div>

<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">
Copy link
Copy Markdown
Collaborator

@robmint robmint Dec 20, 2021

Choose a reason for hiding this comment

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

So here the title is clickable but does no action?

Copy link
Copy Markdown
Contributor Author

@rosemcc rosemcc Dec 20, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@rosemcc rosemcc Dec 20, 2021

Choose a reason for hiding this comment

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

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 :)

Copy link
Copy Markdown
Contributor

@Trombach Trombach Dec 21, 2021

Choose a reason for hiding this comment

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

@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

<span *ngIf="orgUnit.name">{{ orgUnit.name }}</span>
<span *ngIf="orgUnit.maoriName && orgUnit.name">, </span>
<span *ngIf="orgUnit.maoriName">{{ orgUnit.maoriName }}</span>
Expand Down
2 changes: 1 addition & 1 deletion research-hub-web/src/app/components/cards/org-unit-card/org-unit-card.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class OrgUnitCardComponent {
constructor() { }

public navigateTo(url: string) {
location.href = url;
window.open(url, "_blank");
}

}
12 changes: 7 additions & 5 deletions research-hub-web/src/app/components/cards/standard-card/standard-card.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@
contentItem.slug
]"
>
{{ contentItem.title }}
<span>{{ contentItem.title }}
&nbsp;
<mat-icon *ngIf="contentItem.ssoProtected" matTooltip="SSO Login Required">
Copy link
Copy Markdown
Collaborator

@robmint robmint Dec 20, 2021

Choose a reason for hiding this comment

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

Suggest we remove SSO - as this is not really necessary and might be confusing to some people

Copy link
Copy Markdown
Contributor Author

@rosemcc rosemcc Dec 20, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@rosemcc rosemcc Dec 20, 2021

Choose a reason for hiding this comment

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

@cameronmclean what do you think?

lock
</mat-icon>
</span>
</a>
&nbsp;
<mat-icon *ngIf="contentItem.ssoProtected" matTooltip="SSO Login Required">
lock
</mat-icon>
</mat-card-title>

<mat-card-content
><span class="card-summary">
<p>{{ contentItem.summary }}</p>
Expand Down
3 changes: 2 additions & 1 deletion research-hub-web/src/app/components/cards/standard-card/standard-card.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
background: rgb(255, 255, 255);
}
.mat-card-title {
>a {
span {
color: white;
&:hover {
border-bottom: solid 2px white;
Expand All @@ -28,6 +28,7 @@ mat-icon {
font-size: 16px;
height: 16px;
width: 16px;
color: $dark-grey;
}

img {
Expand Down
3 changes: 2 additions & 1 deletion research-hub-web/src/app/components/layout/search-bar/search-bar.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
id="search"
#searchBox
type="search"
(keydown.enter)="search()"
#trigger="matAutocompleteTrigger"
(keydown.enter)="search(); trigger.closePanel()"
Copy link
Copy Markdown
Collaborator

@robmint robmint Dec 20, 2021

Choose a reason for hiding this comment

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

I like this!

placeholder="Search for anything"
[matAutocomplete]="auto"
[formControl]="searchText"
Expand Down
2 changes: 1 addition & 1 deletion research-hub-web/src/app/components/layout/search-bar/search-bar.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { filter, map, startWith } from 'rxjs/operators';
export class SearchBarComponent implements OnInit, OnDestroy {
@ViewChild('searchBarContainer') searchBarContainer: ElementRef;
@ViewChild('searchBox') searchBox: ElementRef;
@ViewChild('filterContent') filterContent: ElementRef
@ViewChild('filterContent') filterContent: ElementRef;

public searchText: FormControl = new FormControl();
public activeFilters: SearchFilters;
Expand Down
5 changes: 0 additions & 5 deletions research-hub-web/src/app/components/shared/app.shared.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import { CommonModule } from '@angular/common';
import { FlexLayoutModule } from '@angular/flex-layout';
import { FormsModule, ReactiveFormsModule } from '@angular/forms';
import { MaterialModule } from '@app/app.material.module';
import { ErrorDialogComponent } from './error-dialog/error-dialog.component';
import { ConfirmDialogComponent } from './confirm-dialog/confirm-dialog.component';
import { HumanCasePipe } from '@pipes/human-case.pipe';
import { ContentTypeDisplayNamePipe } from '@pipes/content-type-display-name.pipe';
import { CollectionListComponent } from './collection-list/collection-list.component';
Expand Down Expand Up @@ -39,8 +37,6 @@ import { ExpandablePagePartComponent } from './body-media/expandable-page-part/e
MarkdownModule.forRoot()
],
declarations: [
ErrorDialogComponent,
ConfirmDialogComponent,
HumanCasePipe,
ContentTypeDisplayNamePipe,
CollectionListComponent,
Expand All @@ -60,7 +56,6 @@ import { ExpandablePagePartComponent } from './body-media/expandable-page-part/e
FormsModule,
ReactiveFormsModule,
MaterialModule,
ErrorDialogComponent,
HumanCasePipe,
ContentTypeDisplayNamePipe,
CollectionListComponent,
Expand Down
6 changes: 3 additions & 3 deletions ...p/components/shared/body-media/blocks-embedded-asset/blocks-embedded-asset.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<span [ngSwitch]="contentItem.contentType">
<ng-template #image>
<div class="embedded-asset-block section-2" ngClass.xs="embedded-asset-block-mobile">
<a *ngIf="contentItem.url" href="{{ contentItem.url }}">
<a *ngIf="contentItem.url" href="{{ contentItem.url }}" target="_blank">
<picture>
<source srcset="{{ contentItem.url }}?w=1500&fm=webp" type="image/webp">
<img src="{{ contentItem.url }}?w=1500" alt="{{ contentItem.title }}" />
Expand All @@ -15,7 +15,7 @@

<ng-template #svg>
<div class="embedded-asset-block section-2" ngClass.xs="embedded-asset-block-mobile">
<a *ngIf="contentItem.url" href="{{ contentItem.url }}">
<a *ngIf="contentItem.url" href="{{ contentItem.url }}" target="_blank">
<img src="{{ contentItem.url }}?w=1500" alt="{{ contentItem.title }}" />
</a><br>{{ contentItem.title }}
<small class="asset-summary" *ngIf="contentItem.description">&nbsp;—&nbsp;{{ contentItem.description
Expand Down Expand Up @@ -43,7 +43,7 @@
<!-- Asset File -->
<span *ngSwitchDefault>
<div class="embedded-asset-block file" ngClass.xs="embedded-asset-block-mobile">
<a *ngIf="contentItem.url" href="{{ contentItem.url }}">
<a *ngIf="contentItem.url" href="{{ contentItem.url }}" target="_blank">
<mat-card>
<mat-card-title class="card-title">
<span>{{ contentItem.title }}</span>
Expand Down
2 changes: 1 addition & 1 deletion ...p/components/shared/body-media/blocks-embedded-entry/blocks-embedded-entry.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<span [ngSwitch]="contentItem.__typename">
<!-- Link Card -->
<span *ngSwitchCase="'LinkCard'">
<div>
<div class="link-card">
<a
href="{{
contentItem.document ? contentItem.document.url : contentItem.url
Expand Down
9 changes: 0 additions & 9 deletions research-hub-web/src/app/components/shared/confirm-dialog/confirm-dialog.component.html
View file
Open in desktop

This file was deleted.

Empty file removed research-hub-web/src/app/components/shared/confirm-dialog/confirm-dialog.component.scss
View file
Open in desktop
Empty file.
22 changes: 0 additions & 22 deletions research-hub-web/src/app/components/shared/confirm-dialog/confirm-dialog.component.ts
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions research-hub-web/src/app/components/shared/error-dialog/error-dialog.component.html
View file
Open in desktop

This file was deleted.

Empty file removed research-hub-web/src/app/components/shared/error-dialog/error-dialog.component.scss
View file
Open in desktop
Empty file.
26 changes: 0 additions & 26 deletions research-hub-web/src/app/components/shared/error-dialog/error-dialog.component.ts
View file
Open in desktop

This file was deleted.

35 changes: 24 additions & 11 deletions research-hub-web/src/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,20 +134,33 @@ a {
color: $link-light-bg-color;
font-family: $font-family;
}

// show icon for external links
// page below is good for checking
// http://localhost:4200/research-impact/planning-for-impact
// a:not([href^="/"]):after {
// font-family: "Material Icons";
// content: " \e895";
// font-size: 0.8em;
// }

span {
font-family: $font-family;
}

/**
* External Links Icon
* Show the icon for any anchor tags in the main-content area where:
* - href does not start with '/' (our domain)
* - href does not contain @ (emails)
* - do not apply to those with .standard-button class
* - do not apply to descendents of element app-blocks-embedded-asset
* - 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 *):not(.banner-container-home *):after {
font-family: "Material Icons";
content: " \e895";
font-size: 0.8em;
color: $dark-grey;
}
ngx-contentful-rich-text a[href^="mailto"]:after {
font-family: "Material Icons";
content: " \e0e1";
font-size: 0.8em;
color: $dark-grey;
}

/*
* app-markdown related styles
*/
Expand All @@ -168,7 +181,7 @@ span {
}

/*
* Gloabl styles and Material overrides
* Global styles and Material overrides
*/
mat-chip.mat-chip {
font-size: mat-font-size($app-chips-typography, button);
Expand Down