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

Linkable ROR icon for organisations in the record page #2130

Merged
merged 8 commits into from
Sep 11, 2023

Conversation

ramGranell
Copy link
Contributor

@ramGranell ramGranell commented Sep 8, 2023

This is related to ticket #2109. A good example it is this record https://deploy-preview-2130--fairsharing.netlify.app/FAIRsharing.wkggtx Check the organisations in that section.

@knirirr , you can note, that Codacy complains saying that "Current Status: Not Analyzed " meaning that it is not able to analyse the code.

@allysonlister
Copy link
Contributor

Thank you - the links look good, and I like the size and placement of the ROR logos.

When you click on the ROR logo it loads by default in the current page, while when you click on the FS organisation link it loads by default in a new tab. I am agnostic about the default behaviour, but I would like both of these clicks to behave the same. If you would like an opinion (though I am happy to hear other ideas), I'd suggest opening both FS orgs and ROR links in a new tab.

Thanks!

@ramGranell
Copy link
Contributor Author

ramGranell commented Sep 11, 2023

Thank you - the links look good, and I like the size and placement of the ROR logos.

When you click on the ROR logo it loads by default in the current page, while when you click on the FS organisation link it loads by default in a new tab. I am agnostic about the default behaviour, but I would like both of these clicks to behave the same. If you would like an opinion (though I am happy to hear other ideas), I'd suggest opening both FS orgs and ROR links in a new tab.

Thanks!

This is changed and a new tab is open when pressing the ROR icon.

Copy link
Contributor

@allysonlister allysonlister left a comment

Choose a reason for hiding this comment

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

All looks great, thanks!

Copy link
Contributor

@knirirr knirirr left a comment

Choose a reason for hiding this comment

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

In this case I think that the codacy warning can be ignored (I had a long discussion with their support about this sort of thing some time ago).

@ramGranell ramGranell merged commit d87357d into dev Sep 11, 2023
8 of 9 checks passed
@ramGranell ramGranell deleted the RG_ROR_logo_records#2109 branch September 11, 2023 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants