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

Open external nav links in new tab by default #71

Merged
merged 2 commits into from
Nov 3, 2023

Conversation

mjaehn
Copy link
Contributor

@mjaehn mjaehn commented Oct 27, 2023

This is just setting a default value. At the moment, we don't have external links in the navigation bar, so it won't affect the page.

We may think about adding external links to the navigation if needed.

@mjaehn mjaehn requested a review from AnnikaLau November 1, 2023 09:13
Copy link
Contributor

@AnnikaLau AnnikaLau left a comment

Choose a reason for hiding this comment

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

I cannot test it myself but I trust you that it works.

@AnnikaLau AnnikaLau merged commit f9940f0 into main Nov 3, 2023
2 checks passed
@AnnikaLau AnnikaLau deleted the nav_external_links_new_tab branch November 3, 2023 16:04
@AnnikaLau
Copy link
Contributor

It actually doesn't work...

@mjaehn
Copy link
Contributor Author

mjaehn commented Nov 6, 2023

What exactly does not work? Website LGTM

@AnnikaLau
Copy link
Contributor

I thought this merge request should "open external navigation links in new tab". At least for me that's not happening.

@mjaehn
Copy link
Contributor Author

mjaehn commented Nov 6, 2023

This is because we don't have external links in the navigation bar (yet).

@AnnikaLau
Copy link
Contributor

Ah, I thought that would apply to any external link. Shouldn't we have that rule for all links?

@mjaehn
Copy link
Contributor Author

mjaehn commented Nov 6, 2023

Yes I would prefer that, too. However, this is not yet implemented in just-the-docs.

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.

2 participants