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

Allow opening ds-metadata-uri-values links in a new window #2866

Conversation

kunovercammen
Copy link

References

Description

This feature request proposes adding a new optional input parameter to the <ds-metadata-uri-values> component in order to specify the link target, allowing metadata links to open in a new window/tab.

Instructions for Reviewers

List of changes in this PR:

  • Added a new optional input parameter linkTarget to the <ds-metadata-uri-values> component. The default value of this parameter is _blank, this will make sure the link is opened in a new window/tab.
  • Added a test case in the src/app/item-page/field-components/metadata-uri-values/metadata-uri-values.component.spec.ts to verify the correct application of the linkTarget attribute to metadata links.

How to test:

  • Go to an item page, for example a publication
  • Click on the link that's under the URI-title
  • The link now opens in a new tab/window

Reason for changes: If the user clicks a link that leads away from DSpace, we want it to open in a new window.

Checklist

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using yarn lint
  • My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

@kunovercammen kunovercammen self-assigned this Mar 15, 2024
@kunovercammen kunovercammen added new feature low priority claimed: Atmire Atmire team is working on this issue & will contribute back labels Mar 15, 2024
@alanorth
Copy link
Contributor

Thanks @kunovercammen. This patch works well for me on DSpace 7.6.1. Our repository has many metadata fields containing URLs that should be opened in a new window (for example DOIs and other URLs on external sites).

This is a very minor change, but I wonder if @tdonohue will allow it in dspace-7_x since it changes the existing behavior? In any case, the base branch should probably be changed to main and then the port to dspace-7_x tag should be added if Tim agrees.

@alanorth alanorth linked an issue Apr 5, 2024 that may be closed by this pull request
@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge label May 1, 2024
@tdonohue
Copy link
Member

tdonohue commented May 8, 2024

@kunovercammen : This seems reasonable to me for both 7.x and main. However, I cannot get this PR to apply cleanly to the main branch. Could you please create a second PR which can be applied to main? That way this can be more easily tested for 8.x as well.

@tdonohue tdonohue added the port to main This PR needs to be ported to `main` branch for the next major release label May 8, 2024
@alanorth
Copy link
Contributor

alanorth commented May 9, 2024

From reading the code in d46248e it seems this same functionality was introduced as part of the COAR Notify support recently merged in main for DSpace 8.0-SNAPSHOT.


Edit: I have just tested on sandbox.dspace.org (DSpace 8.0-RC1) and external links use target=_blank by default. Therefore this patch is not necessary on main.

Merging this for DSpace 7.6.1 as I have already tested it and this is 1 APPROVAL. Thanks @kunovercammen.

@alanorth alanorth removed the port to main This PR needs to be ported to `main` branch for the next major release label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge claimed: Atmire Atmire team is working on this issue & will contribute back improvement low priority
Projects
Development

Successfully merging this pull request may close these issues.

Allow opening ds-metadata-uri-values links in a new window
4 participants