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

Set anchors target for external urls to _blank #2118

Conversation

FedericoNembrini
Copy link

As discussed in #1921 and #1922, I set the target to _blank for links to external sites.

I have only modified links that comply with the css rule applied via a[href^="https:"]:not([href*="getmonero.org"]) and which therefore display the external.svg icon

@netlify
Copy link

netlify bot commented Dec 30, 2022

Deploy Preview for barolo-time-757cf9 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit d01a074
🔍 Latest deploy log https://app.netlify.com/sites/barolo-time-757cf9/deploys/6487735ab1cfdc0008ba0d4f
😎 Deploy Preview https://deploy-preview-2118--barolo-time-757cf9.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@plowsof
Copy link
Collaborator

plowsof commented Dec 31, 2022

If @erciccione could clarify the scope of this PR (en folders are mentioned here) i can review/help if needed.

Copy link
Contributor

@erciccione erciccione left a comment

Choose a reason for hiding this comment

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

looks good :)

@erciccione
Copy link
Contributor

If @erciccione could clarify the scope of this PR (en folders are mentioned #1921 (comment)) i can review/help if needed.

The en folders don't need to be touched in this case as most if not all links are inside the documents themselves.

@erciccione
Copy link
Contributor

closes #1921

@erciccione
Copy link
Contributor

@FedericoNembrini there are some conflicts that need to be solved before we can merge this.

Copy link
Contributor

@erciccione erciccione left a comment

Choose a reason for hiding this comment

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

Please fix the merge commit

@FedericoNembrini
Copy link
Author

I have resolved the merge conflicts, have I forgotten something?

@erciccione
Copy link
Contributor

@FedericoNembrini The last commit you pushed is a merge commit. That usually happens after a borked rebase. You could try to remove it and rebase again, fixing the merge conflict.

@FedericoNembrini
Copy link
Author

I have resolved conflicts with github built in function.
I'll try to remove my last commit and do a rebase.

@erciccione
Copy link
Contributor

@FedericoNembrini please fix the conflicts and rebase :)

@erciccione
Copy link
Contributor

ping @FedericoNembrini

@FedericoNembrini
Copy link
Author

I will fix this in the coming days

@plowsof
Copy link
Collaborator

plowsof commented Jan 31, 2024

there is a plugin which claims to do this automagically for external links. i have not tested it yet but looks promising https://github.com/keithmifsud/jekyll-target-blank

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