Skip to content

Force external links to open in a new tab - #920

Merged
oliver-sanders merged 6 commits into
cylc:8.6.xfrom
samuel-denton:8.6.x_Ext_links_new_tab
May 8, 2026
Merged

oliver-sanders merged 6 commits into
cylc:8.6.xfrom
samuel-denton:8.6.x_Ext_links_new_tab

Conversation

@samuel-denton

@samuel-denton samuel-denton commented Apr 13, 2026 •

Copy link
Copy Markdown
Contributor

Closes #908 if agreed.

Added some JavaScript to the theme layout which Adds target=_blank to external links so they open in a new tab.
I have seen some discourse online as to whether developers should try to effect the users choice when clicking links (they can already chose to open links in a new tab themselves), however in our case, external links are almost always definitions or further reading type of stuff so it feels a bit jarring for them to take you away from the docs.

Also added an icon and hover text to external links making them obvious.

Built example:
https://wwwspice/~samuel.denton/cylc-doc/8.6.x_Ext_links_new_tab/html/tutorial/scheduling/graphing.html
image

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.

Comment thread src/_templates/layout.html Outdated
Comment on lines +7 to +12
document.addEventListener("DOMContentLoaded", function () {
document.querySelectorAll('a.external').forEach(link => {
link.setAttribute('target', '_blank');
link.setAttribute('rel', 'noopener noreferrer');
});
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There are two places we can insert this script, either here in the theme layout doc or in a custom JS doc. The advantage of putting it here is that this file already exists as part of the RTD theme. The downside being it would potentially need re-implementing if the theme was changed.

Moving this to a custom JS file is as simple as adding a new file to src/_static/js with that function, then add:

html_js_files = [
    'name_of_new_file.js',
]

To src/conf.py

@samuel-denton samuel-denton self-assigned this Apr 13, 2026
@samuel-denton

Copy link
Copy Markdown
Contributor Author

This is working here
image
Link 1 is internal, link 2 is external.

@samuel-denton

Copy link
Copy Markdown
Contributor Author

The other method I found to achieve the same was this extension, which worked but relies on it being maintained. The only additional benefit from the extension is that it can display an icon next to external links. We could replicate that if we wanted.

@samuel-denton

Copy link
Copy Markdown
Contributor Author

If we want per link control, we could use something like this:

A Cylc workflow is defined by a flow.cylc configuration file, which uses a nested |location_link|.

.. |location_link| raw:: html

   <a href="https://en.wikipedia.org/wiki/INI_file" target="_blank">INI format</a>

But that's messy and would be a pain to implement for every link we want to effect.

@oliver-sanders oliver-sanders added this to the 8.6.x milestone Apr 14, 2026
@oliver-sanders

Copy link
Copy Markdown
Member

LGTM.

Adding the icon might be simple with a drop of CSS.

E.g, This will cause a red "x" symbol to be added after each external link:

a.external:after {
  content: ' x';
  color:red;
}

I think the RTD theme imports "Font Awesome" which probably has an appropriate icon, so might be simple.

@samuel-denton

Copy link
Copy Markdown
Contributor Author

Adding the icon might be simple with a drop of CSS.

@oliver-sanders I can do that. Do you think the JS should be in the theme layout.html file or should I move it to _static/js like the CSS?

@oliver-sanders

Copy link
Copy Markdown
Member

Do you think the JS should be in the theme layout.html file or should I move it to _static/js like the CSS?

I'm not actually sure how the _static scripts get sourced, you may find you need to add a corresponding <script source="..." /> line to the HTML file anyway?

@samuel-denton

Copy link
Copy Markdown
Contributor Author

Do you think the JS should be in the theme layout.html file or should I move it to _static/js like the CSS?

I'm not actually sure how the _static scripts get sourced, you may find you need to add a corresponding <script source="..." /> line to the HTML file anyway?

@oliver-sanders I believe I can add:

html_js_files = [
    'name_of_new_file.js',
]

to the conf.py to pick up scripts. That seems like the proper way to do it and it opens the door for any further JS we might want to add, it's just whether it's worth creating a new file in this case.

@oliver-sanders

oliver-sanders commented Apr 14, 2026 •

Copy link
Copy Markdown
Member

Sounds good.

New files cost little.

Added some CSS to show an icon next to external links.
Comment thread src/_static/css/custom.css Outdated

/* Add an icon to external links */
a.external::after {
content: "🔗 ↗ ⧉";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've looked into using font awesome icons but it looks like on the latest versions of the RTD theme for sphinx, it only bundles specific bits of RTD, so I have been unable to get any icons from them working. I would have liked: this
So assuming FA icons are not possible, which of those three do you like @oliver-sanders? I will adjust the colour, size and position once we pick one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image

Options ^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fair 'nough.

Would probs go with ↗

We can mess with the font-size and vertical-align to make it display nicely.

Comment on lines +18 to +28
/*
Adds target=_blank to external links so they open in a new tab:
- noopener: prevents the new page accessing window.opener (tabnabbing protection)
- noreferrer: removes referrer info and adds extra safety
*/
document.addEventListener("DOMContentLoaded", function () {
document.querySelectorAll('a.external').forEach(link => {
link.setAttribute('target', '_blank');
link.setAttribute('rel', 'noopener noreferrer');
});
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure on preferred code/comment style for JS yet, hopefully this is ok?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fine by me.

We generally use eslint "standard" for JS.

Comment thread src/conf.py
Comment on lines +248 to +264
# -- Static file locations -----------------------------------------------
html_static_path = ['_static']

# These paths are either relative to html_static_path
# or fully qualified paths (eg. https://...)
# NOTE: these are added to the end of the default/theme static files, so
# will override them if there are any conflicts.

# Custom CSS
html_css_files = [
'css/custom.css',
]

# Custom JS
html_js_files = [
'js/extern_links_new_tab.js',
]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved the static file settings to their own block.

@oliver-sanders

Copy link
Copy Markdown
Member

Got an example of unwanted decoration on the homepage:

Screenshot from 2026-04-15 11-10-46

And a suggested change to the CSS:

font-size: 0.7em;
vertical-align: super;
opacity: 0.5;
Screenshot from 2026-04-15 11-12-01

@oliver-sanders

Copy link
Copy Markdown
Member

Curiously, I found some internal links flagged as external?

Screenshot from 2026-04-15 15-54-25

workflow-design-guide/style-guide.html#indentation

@samuel-denton

samuel-denton commented Apr 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Curiously, I found some internal links flagged as external?

ok interesting, I'll have a look into why they are tagged differently. I think there is some config that controls what counts as external.

I'm thinking about adding some hover text that tells the user a link is going to take them to an external page too.

@samuel-denton

Copy link
Copy Markdown
Contributor Author

Curiously, I found some internal links flagged as external?

@oliver-sanders
How did you build / view the test site? That section is added by:

.. automodule:: cylc.flow.scripts.lint

Which is syntax I am not yet familiar with but appears to either generate or import a module from elsewhere. The links within this module are external when built / viewed on my test server as they link back to the live version of the site.

URL of the test site:
https://wwwspice/~samuel.denton/cylc-doc/8.6.x_Ext_links_new_tab/html/user-guide/writing-workflows/configuration.html

URL that is linked to:
https://cylc.github.io/cylc-doc/stable/html/workflow-design-guide/style-guide.html#tab-characters

So I would guess this will fix itself on a live build. How we test that I am unsure.
If we wanted to explicitly fix this, we could exclude links where the href contains something like:
'cylc.github.io/cylc-doc/stable/html'

@oliver-sanders

Copy link
Copy Markdown
Member

How did you build / view the test site

make html && firefox ./build

@oliver-sanders

Copy link
Copy Markdown
Member

automodule and friends are Sphinx's autodocumenter tools, they turn Python docstrings into Sphinx documentation.

In this case, it's reading from cylc.flow.scripts.lint.__doc__ which, on inspection, reveals the issue.

Don't worry about this one, I'll poke at it from the cylc-flow end...

@oliver-sanders

Copy link
Copy Markdown
Member

Don't worry about this one, I'll poke at it from the cylc-flow end...

No clean way to handle this one, will just have to ignore it.

…d with an icon appended.

Added hover text (title) to external links saying they are external and open in a new tab.
Added an aria label for screen readers to pick up.
Comment thread src/_static/js/extern_links_new_tab.js Outdated
document.querySelectorAll('a.external').forEach(link => {
link.setAttribute('target', '_blank');
link.setAttribute('rel', 'noopener noreferrer');
link.setAttribute('title', 'This link will open in a new tab and take you to an external site.');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

adds on hover text.

Comment thread src/_static/js/extern_links_new_tab.js Resolved
link.setAttribute('target', '_blank');
link.setAttribute('rel', 'noopener noreferrer');
link.setAttribute('title', 'This link will open in a new tab and take you to an external site.');
link.setAttribute('aria-label', 'External link: opens in a new tab');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

additional information for a screen reader.

@RyanBcoding RyanBcoding left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested changes. Seems to be working as expected. Comments explain decision for why the changes were made, so happy to approve.

@RyanBcoding
RyanBcoding requested a review from oliver-sanders May 5, 2026 15:36
@oliver-sanders
oliver-sanders merged commit b744c14 into cylc:8.6.x May 8, 2026
2 checks passed
@samuel-denton
samuel-denton deleted the 8.6.x_Ext_links_new_tab branch May 8, 2026 14:23
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