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

Updating links in Quickstarts to open in new tab #10505

Merged
merged 5 commits into from
Feb 21, 2025

Conversation

mchang16-auth
Copy link
Contributor

@mchang16-auth mchang16-auth commented Feb 15, 2025

Changes

  • Changed all links in markdown files contained within /articles/quickstart to <a href="link" target="_blank" rel="noreferrer">. This will make it open in a new tab.
  • Added the same anchor tag logic to 4 files located in /articles/_includes. These files are commonly used in Quickstarts (api_auth_intro.md, _callback_url.md, etc).

Changes were made using this script, and manually tested in local environment to check that the links were working properly. Images in the markdown format of ![text](image) were not altered.

const fs = require('node:fs');
const path = require('path');
const { opendir } = require('node:fs/promises');

const changeLinks = async () => {
  const dir = await opendir('./articles/quickstart', { recursive: true });
  for await (const dirent of dir) {
    if (dirent.isFile() && path.extname(dirent.path) === ".md") {
      console.log(dirent.path);
      fs.readFile(dirent.path, 'utf8', (err, data) => {
        if (err) {
          console.error("READ:", err);
          return;
        }

        let newContent = data.replaceAll(/(\[.*?\])(\(.*?\))/g, (match, p1, p2) => {
          const text = p1.replace(/[\[\]']+/g, "");
          const url = p2.replace(/[{()}]/g, "");
          return `<a href="${url}" target="_blank">${text}</a>`
        })

        fs.writeFile(dirent.path, newContent, err => {
          if (err)
            console.error("WRITE:", err);
        });
      });
    }
  }
}

changeLinks();

@mchang16-auth mchang16-auth changed the title updating links in quickstarts to open in new tab Updating links in Quickstarts to open in new tab Feb 15, 2025
@mchang16-auth mchang16-auth marked this pull request as ready for review February 16, 2025 06:30
@BcnCarlos BcnCarlos self-requested a review February 17, 2025 12:59
BcnCarlos
BcnCarlos previously approved these changes Feb 17, 2025
Copy link
Contributor

@BcnCarlos BcnCarlos 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 to me.

Copy link
Member

@frederikprijck frederikprijck left a comment

Choose a reason for hiding this comment

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

Did we consider adding rel="noreferrer"?

See: https://developer.chrome.com/docs/lighthouse/best-practices/external-anchors-use-rel-noopener

When you link to a page on another site using the target="_blank" attribute, you can expose your site to performance and security issues:

I see we are adding target="_blank" that target another site.

The page I linked also explains the solution:

Adding rel="noopener" or rel="noreferrer" to your target="_blank" links avoids these issues.

It also mentions this is mostly for legacy browsers, as modern browser solve this internally already. So I would still recommend considering it.

BcnCarlos
BcnCarlos previously approved these changes Feb 19, 2025
Copy link
Contributor

@BcnCarlos BcnCarlos left a comment

Choose a reason for hiding this comment

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

Reviewed and approved

Copy link
Contributor

@BcnCarlos BcnCarlos left a comment

Choose a reason for hiding this comment

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

Updates approved

@mchang16-auth mchang16-auth force-pushed the change-links branch 2 times, most recently from ce800bf to aed48ef Compare February 20, 2025 22:16
@BcnCarlos BcnCarlos merged commit be62947 into auth0:master Feb 21, 2025
3 of 4 checks passed
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