Skip to content
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

External links open in new tabs #32

Conversation

malloryfreeberg
Copy link
Contributor

@malloryfreeberg malloryfreeberg commented Aug 15, 2022

Description

  • Updated all external links to open in a new tab using embedded html
  • Updated all internal links to be relative and open in same window
  • Fixed typos & improved unclear text
  • Updated any links with text "here" to text describing what the linked material is (better UX, consistent with rest of website)

Motivation and Context

Currently, links to external materials/sites are opened in the same tab that is currently being used. If someone wants to go back to the FEGA pages, they have to navigate back from where they are. A better user experience would be that external links open in a new tab in the same window, so that a user can easily switch back to the FEGA pages without losing access to the external materials they just opened. Internal links to other pages in the FEGA website can stay in the same tab, since it doesn't navigate someone away from the FEGA website. This PR addressed Issue #26.

How has this been tested?

Changes have been tested in my fork by enabling GitHub pages to build a local copy of the website. All website links were then checked (using Mac & Google Chrome) that:

  • External links opened in a new tab
  • Internal links opened in the same tab

NB Links won't open correctly in GitHub markdown; GitHub pages must be built to test.

What should reviewers focus on?

  • External links open in a new tab and internal links open in same window, optionally in another OS/browser
  • Updated text is at least as or more clear than before

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New content (non-breaking change which adds new content)
  • Modified content (non-breaking change which modifies existing content)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I have read the CONTRIBUTING.md guide.
  • My changes follow the code style of this project.
  • I have updated supporting documentation, if required.
  • I have added my details to the CONTRIBUTORS.yaml file.

@malloryfreeberg malloryfreeberg linked an issue Aug 15, 2022 that may be closed by this pull request
@malloryfreeberg malloryfreeberg added the feature New feature or request label Aug 15, 2022
@malloryfreeberg malloryfreeberg changed the title Feature/external links open in tabs External links open in new tabs Aug 15, 2022
Copy link
Collaborator

@M-casado M-casado left a comment

Choose a reason for hiding this comment

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

LGTM. Checked a few links of each page and they work fine (in Mallory's forked GitHub pages).
I have not checked the full set of documents, though, searching for external links that escaped this PR.

| Data Use Ontology (DUO) | Allows users to semantically tag datasets with usage restrictions so datasets can be automatically discoverable based on a researcher's authorization level or intended use. | 2021-02-23 | 2021-02-23 | <a href="https://github.com/EBISPOT/DUO" target="_blank">Specification</a>, <a href="https://ega-archive.org/data-use-conditions" target="_blank">Documentation</a>, <a href="https://www.ebi.ac.uk/ols/api/ontologies" target="_blank">Endpoint</a> | <a href="https://doi.org/10.1016/j.xgen.2021.100028" target="_blank">DOI</a> |
| htsget | Enables secure, efficient, and reliable access to sequencing read and variation data including specific genomic regions. | v1.3.0 | v1.0.0 | <a href="http://samtools.github.io/hts-specs/htsget.html" target="_blank">Specification</a>, <a href="https://github.com/EGA-archive/ega-download-client" target="_blank">Documentation</a>, <a href="https://ega.ebi.ac.uk:8052/elixir/tickets/tickets" target="_blank">Endpoint</a> | <a href="https://doi.org/10.1093/bioinformatics/bty492" target="_blank">DOI</a> |
| refget | Enables access to reference sequences using an identifier derived from the sequence itself. | v1.2.6 | N/A | <a href="http://samtools.github.io/hts-specs/refget.html" target="_blank">Specification</a>, Documentation, Endpoint | <a href="https://doi.org/10.1093/bioinformatics/btab524" target="_blank">DOI</a> |
| Researcher IDs (Passports, Visas) | Specifies the collection of researchers that may access a dataset at any given time, and the credentials they must supply. | v1.0.1 | v1.0.1 | <a href="https://github.com/ga4gh-duri/ga4gh-duri.github.io/blob/master/researcher_ids/ga4gh_passport_v1.md" target="_blank">Specification</a>, <a href="https://docs.google.com/document/d/1FTzUYAfV5d2a0zoDkbY9Iy_L5NbSAnHeWnmY2NIrY8M/edit" target="_blank">Documentation</a>, <a href="https://ega.ebi.ac.uk:8443/ega-permissions/swagger-ui/index.html" target="_blank">Endpoint</a> | <a href="https://doi.org/10.1016/j.xgen.2021.100030" target="_blank">DOI</a> |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, why have Visas in uppercase? I've seen the DOI where they put the "passport" specification with uppercase, but not visas. And wouldn't it be "Passports and visas"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just wanted to highlight line 49, but git highlighted all these. Sorry about that 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to "visas"

@malloryfreeberg malloryfreeberg merged commit 68ac477 into EGA-archive:main Aug 15, 2022
@malloryfreeberg malloryfreeberg deleted the feature/external-links-open-in-tabs branch August 15, 2022 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] External links from webpages should open in new tab
2 participants