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

As a researcher, I want URLs I enter in metadata fields to be clickable (i.e. alternativeURL) so that it's easier to follow links #3337

Closed
shlake opened this issue Sep 7, 2016 · 27 comments

Comments

@shlake
Copy link
Contributor

shlake commented Sep 7, 2016

The new (v4.4) alternativeURL metadata field should be clickable when citation metadata is displayed.

On metadata entry, the Watermark says "Enter full URL".

Solution: In citation.tsv file the field displayFormat is empty. In all the other metadata fields that are "URL" fields (and are clickable) have the following as the displayFormat:
<a href="#VALUE">#VALUE</a>

So I think that is what alternativeURL should have:
displayFormat = <a href="#VALUE">#VALUE</a>

On this record you can see the non-clickable link:
https://demo.dataverse.org/dataset.xhtml?persistentId=doi:10.5072/FK2/YJMBUN

@scolapasta
Copy link
Contributor

scolapasta commented Sep 9, 2016

@shlake Currently that displayFormat is only used for compound types. This is the first case of a URL in a primitive type. Using display format may be the correct path, but just setting it won't actually change anything with the current logic.

@shlake
Copy link
Contributor Author

shlake commented Sep 9, 2016

You are correct, I was going to update the ticket on Monday - me setting the displayFormat to the above code (as the other URLs are set to), does not make it hyperlinked when displaying with other metadata (on the metadata tab).

@tdilauro
Copy link
Contributor

tdilauro commented Feb 2, 2017

Just a note that we have run into a similar problem in our software metadata block. We have primitive types that we'd like to have render as links. We set the displayFormat to <a href="#VALUE">#VALUE</a>, as mentioned above. I checked the database to verify that they had made it in properly.

@scolapasta Is there any fundamental reason to restrict displayFormat to just the compound fields?

@pdurbin
Copy link
Member

pdurbin commented Feb 2, 2017

I'm a little confused about if there's a bug to fix or if documentation for the tsv format is lacking (#3168). Or both. 😄

@shlake
Copy link
Contributor Author

shlake commented Feb 2, 2017

Maybe start with updating/clarifying the documentation https://docs.google.com/document/d/1idh4wvCcoxSo7rPkolOkTD5Wvn6PQO__uXqkf7eQujA/edit#heading=h.xqa9co8gtrwh

Maybe the problem is with the "way it works" (documentation) or a bug with how things are being displayed (thinking that a field with a href value "should" be displayed as a hyperlink, and it isn't).

@scolapasta
Copy link
Contributor

scolapasta commented Feb 2, 2017

There is an issue to solve which is to have formatting for URLs. There are a few ways we can consider this.

@tdilauro the displayFormat was added for compound fields, because that's where you were "grouping" things and that's really what it's for. We should actually be handing type URL better.

Something like software is more tricky because what if that's not used for a URL? In that case, we may want consider allowing html in that field, like we do for some others. (I think the rule is if it is a textbox you can do html and it is rendered - whitelisted, of course).

@tdilauro
Copy link
Contributor

tdilauro commented Feb 2, 2017

@scolapasta Sorry, I meant fields in our software metadata block, not the software field. There are other fields in this block, as well, but these are the field types that represent links:

dvndb=# select name, title, fieldtype, displayformat from  datasetfieldtype where displayformat like '<a%' and name like 'sw%';
 swCodeRepositoryLink       | Code Repository Link            | URL       | <a href="#VALUE">#VALUE</a>
 swDependencyLink           | Dependency Location             | URL       | <a href="#VALUE">#VALUE</a>
 swInputOutputLink          | Input/Output Location           | URL       | <a href="#VALUE">#VALUE</a>
 swOtherRelatedSoftwareLink | Other Related software Location | URL       | <a href="#VALUE">#VALUE</a>
 swOtherRelatedResourceLink | Other Related resource Location | URL       | <a href="#VALUE">#VALUE</a>
(5 rows)

@edzale
Copy link

edzale commented Jun 20, 2017

Hi, I am experiencing the same issue and after reading your comments, I am a bit confused: is it a bug or a matter of misconfiguration?

@scolapasta
Copy link
Contributor

scolapasta commented Jun 20, 2017 via email

@pdurbin pdurbin added the User Role: Curator Curates and reviews datasets, manages permissions label Jul 4, 2017
@IQSS IQSS deleted a comment from scolapasta Feb 15, 2018
@jggautier jggautier changed the title alternativeURL Metadata Field should be Clickable As a researcher, I want URLs I enter in metadata fields to be clickable Feb 15, 2018
@jggautier jggautier changed the title As a researcher, I want URLs I enter in metadata fields to be clickable As a researcher, I want URLs I enter in metadata fields to be clickable so that it's easier to follow links Feb 15, 2018
@edzale
Copy link

edzale commented Feb 19, 2018

Hi @jggautier ,
I changed the displayFormat to <a href="#VALUE">#VALUE</a> and it remains unclickable (https://data.inra.fr/dataset.xhtml?persistentId=doi:10.15454/1.505380010373349E12). If there is a bug here as stated by @scolapasta when do you think it will be handled? thanks in advance for your help.
Esther

@jggautier
Copy link
Contributor

Hi @edzale,

You're right. This issue is unresolved. Changing the displayFormat of the primitive field "alternativeURL" to <a href="#VALUE">#VALUE</a> won't work because it works only for compound fields.

I defer to @djbrooke to help answer when it'll be handled.

(I edited your comment to make the displayFormat display as code <a href="#VALUE">#VALUE</a>. Otherwise it looks like #VALUE, which is another displayFormat and has caused confusion before :) .)

@edzale
Copy link

edzale commented Feb 20, 2018

Hi @jggautier
Thank you for your answer. Looking forward to a solution.
Esther

@pdurbin
Copy link
Member

pdurbin commented Feb 26, 2018

@edzale to be clear, you're talking specifically about the "Alternative URL" field, right? The screenshot below is from the dataset you mentioned and like you said, it isn't clickable:

screen shot 2018-02-26 at 1 38 55 pm

@edzale
Copy link

edzale commented Feb 26, 2018

@pdurbin, that's right, I am talking about the "Alternative URL" field.

@pdurbin pdurbin changed the title As a researcher, I want URLs I enter in metadata fields to be clickable so that it's easier to follow links As a researcher, I want URLs I enter in metadata fields to be clickable (i.e. alternativeURL) so that it's easier to follow links May 16, 2018
@umuchlish
Copy link
Contributor

Hi @pdurbin

Alternative URL has working perfectly at my end, by modify alternativeURL record in the datasetfieldtype table, add <a href="#VALUE">#VALUE</a> to displayFormat field.

image

I saw citation.tsv has not included "displayFormat" with <a href="#VALUE">#VALUE</a> since last night upgrade to 4.8.6 not effecting to existing altearnativeURL.

@pdurbin
Copy link
Member

pdurbin commented May 21, 2018

@umuchlish wow! That looks like a fairly easy fix! This issue isn't in our current sprint but do you think you'd be able to make a pull request so we can move this issue from "Inbox" to "Code Review" at https://waffle.io/IQSS/dataverse ?

@umuchlish
Copy link
Contributor

Sure Philip.

@pdurbin
Copy link
Member

pdurbin commented May 23, 2018

@umuchlish great! Here's the line to change:

alternativeURL Alternative URL A URL where the dataset can be viewed, such as a personal or project website. Enter full URL, starting with http:// url 3 FALSE FALSE FALSE FALSE FALSE FALSE citation

@pdurbin
Copy link
Member

pdurbin commented May 24, 2018

@umuchlish thanks for pull request #4711! I just moved it to code review at https://waffle.io/IQSS/dataverse . It looks fine to me. Thanks!

@pdurbin pdurbin removed Help Wanted: Code Mentor: pdurbin User Role: Curator Curates and reviews datasets, manages permissions labels May 24, 2018
@djbrooke djbrooke self-assigned this May 24, 2018
@mheppler
Copy link
Contributor

I was going to add this to a comment in the PR, but I wanted to float the suggestion here in the issue, in case there was reason not to follow this approach.

I have noticed that the <a href="#VALUE">#VALUE</a> for the alternativeURL does not include the attribute target="_blank" which opens the link in a new window. This is the approach we take with external links throughout the app. But apparently that is not the case in the citation.tsv file.

The links for citation fields alternativeURL, publicationURL, keywordVocabularyURI, topicClassVocabURI, producerURL, distributorURL, are all missing this attribute. I suggest that we add it for all of these and make sure to keep an eye of other links in the .tsv metadata blocks that are also missing it.

@scolapasta @sbarbosadataverse @jggautier Thoughts?

@pameyer
Copy link
Contributor

pameyer commented May 24, 2018

Another thought that's almost certainly out of scope for this issue, but related to the discussion of consistency with handling external links, would be adding rel="noopener" to external links (recommended by lighthouse for performance / security - https://developers.google.com/web/tools/lighthouse/audits/noopener).

@mheppler @djbrooke (and anybody else interested) - think it's worth a new issue?

@mheppler
Copy link
Contributor

I think that if we're adding one attribute to the links in this metadata block, then it perfectly in scope to also add a second, especially if it is a security AND performance improvement!

@pameyer
Copy link
Contributor

pameyer commented May 24, 2018

Performance is mostly "don't let javascript heavy page slow down current page" - so more avoiding making it worse than making it better. Still worth doing in my view.

@djbrooke
Copy link
Contributor

@umuchlish - thanks for the PR. Are you able and willing to make the change that @mheppler suggested above?

@pameyer - can you please create a separate issue for your suggestion?

@djbrooke djbrooke removed their assignment May 24, 2018
@umuchlish
Copy link
Contributor

Hi djbrooke, ah yes, that would be better.

@pdurbin
Copy link
Member

pdurbin commented May 29, 2018

In 6ffb28d I added target="_blank" to all the fields @mheppler mentioned. Now they open in a new window. Here's how the look in the UI:

screen shot 2018-05-29 at 9 02 37 am

Thank you again to @umuchlish for the pull request! The fix should also benefit @shlake and @edzale who have commented on this issue as well as others.

I moved this issue to QA at https://waffle.io/IQSS/dataverse

It's small so I'd love it if we could get this in 4.9.

@kcondon kcondon self-assigned this May 30, 2018
pdurbin added a commit to umuchlish/dataverse that referenced this issue Jun 4, 2018
pdurbin added a commit to umuchlish/dataverse that referenced this issue Jun 4, 2018
@pdurbin
Copy link
Member

pdurbin commented Jun 4, 2018

@sekmiller authored and I tested and committed a SQL update for existing installations: 630a8d1

New installations get the fix from the change to the citation.tsv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests