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

Option for Setting target="_blank" on Links #12

Closed
enijar opened this issue Jan 19, 2016 · 24 comments
Closed

Option for Setting target="_blank" on Links #12

enijar opened this issue Jan 19, 2016 · 24 comments

Comments

@enijar
Copy link

enijar commented Jan 19, 2016

Is it feasible to allow this as an option for this package?

Something like the following:

<ReactMarkdown source={input} linkTargets="_blank"/>
@rexxars
Copy link
Collaborator

rexxars commented Jan 19, 2016

A bit unsure. It's easy enough to add, but I don't know if the API makes sense. I think there are two options:

  • Allowing users to pass their own handlers for types, eg Link: MyCustomLinkComponent. This would be very flexible and is something I've been wanting to add anyway.
  • Adding a linkTarget prop that can either be a string or a function. The functions could determine the target based on the node, for instance whether it is a relative URL or not.

I'm leaning towards the first, as that will allow all sorts of flexibility to be added without bloating the renderer with options.

@enijar
Copy link
Author

enijar commented Jan 19, 2016

I agree. The first option you listed sounds the best way to go and would open up the API to be much more flexible.

Do you have any inkling as to when such a feature could be ready?

@rexxars
Copy link
Collaborator

rexxars commented Jan 19, 2016

I don't think it should be too much work, so it's more a question of when I'll have the time to prioritize it. I'm hoping it'll be this week, at least.

@enijar
Copy link
Author

enijar commented Jan 19, 2016

That would be perfect. I really like this package as I don't have to use the dangerouslySetInnerHTML property!

Keep up the good work on this package 👍

@petetnt
Copy link

petetnt commented Feb 4, 2016

Wouldn't the most optimal case be that any valid attribute would be valid and applied? When rendering the container, you could just spread the props with ...this.props (and handling special cases such as source like you do now).

Then you could just do <ReactMarkdown containerTagName="a" target="_blank" href="https://foo.com" source={"Link **here** to visit foo.com"}>

@rexxars
Copy link
Collaborator

rexxars commented Feb 4, 2016

I've implemented custom renderers in commonmark-react-renderer (which powers this component). I'll see if I can get a new release of react-markdown out quite soon. When that is out it should be as as easy as:

<ReactMarkdown
  source="Click [here](https://espen.codes/) to visit external site"
  renderers={{Link: props => <a href={props.href} target="_blank">{props.children</a>}}
/>

@rexxars
Copy link
Collaborator

rexxars commented Feb 21, 2016

Version 4 is now out, and the above example should work for this use case.

@rexxars rexxars closed this as completed Feb 21, 2016
@cyberwombat
Copy link

cyberwombat commented Oct 8, 2016

I wonder how to do a paragraph like this? I end up Each child in an array or iterator should have a unique "key" prop. Check the render method of 'Paragraph'

This seems to occur when using the softBreak: 'br' option.

Update - can be fixed by overriding the softBreak renderer with const SoftBreak = () => (<br/>)

@elimisteve
Copy link

In case what I found helps others:

I needed something similar within an Electron app and went with this solution before I found this issue:

    var shell = require('electron').shell;
    //open links externally by default
    $(document).on('click', 'a[href^="http"]', function(event) {
        event.preventDefault();
        shell.openExternal(this.href);
    });

...though this makes all links on the page get opened by the user's default browser, not just links rendered by react-markdown, which might not quite be what you're looking for.

@zoruc
Copy link

zoruc commented May 5, 2017

shouldn't it be

<ReactMarkdown source="Click [here](https://espen.codes/) to visit external site" renderers={{Link: props => <a href={props.href} target="_blank">{props.children}</a>}}/>

@ogtfaber
Copy link

Dont forget to add rel="noopener noreferrer"
(See https://medium.com/@jitbit/target-blank-the-most-underestimated-vulnerability-ever-96e328301f4c)

@goldylucks
Copy link

goldylucks commented Sep 4, 2017

I'm currently using it like so:

    <ReactMarkdown
      renderers={{ Link: (props: Object) => {
        if (props.href.match('http')) {
          return <a href={props.href} target="_blank" rel="nofollow noreferrer noopener">{props.children}</a>
        }
        return <a href={props.href}>{props.children}</a>
      } }}

maybe this should be built in?
or passable as a prop?

so we could do something like ...

    <ReactMarkdown externalLinksInNewTab />

@gajus
Copy link

gajus commented Sep 15, 2017

Whats the reason you have if (props.href.match('http')) { check?

What else can it be?

@gajus
Copy link

gajus commented Sep 15, 2017

Never mind. Just realised that there are a plethora of protocols. That said, I would not recommend blindly constructing a link. A safer option would be:

const renderLink = (props: Object) => {
  if (!props.href.startsWith('http')) {
    return props.children;
  }

  return <a href={props.href} rel='nofollow noreferrer noopener' target='_blank'>{props.children}</a>;
};

@goldylucks
Copy link

@gajus
that will not work, because [my internal link](/some-internal-path/) will just render my internal link as text

@gajus
Copy link

gajus commented Sep 15, 2017

Sorry, this should have been props.href, i.e.

const renderLink = (props: Object) => {
  if (!props.href.startsWith('http')) {
    return props.href;
  }

  return <a href={props.href} rel='nofollow noreferrer noopener' target='_blank'>{props.children}</a>;
};

And yes, that would just render plain text link.

@goldylucks
Copy link

@gajus
yes but we want to render an internal link

@gajus
Copy link

gajus commented Sep 15, 2017

oh, I see. Forgot about that possibility.

@szeredaiakos
Copy link

szeredaiakos commented Mar 8, 2018

having a [legitimate link](http://www.example.com//){name:value} look easy enough where the {...} is a json with properties added to the link tag. and ignore it if there is a space between ] and { . or maybe go with the {:target="_blank"}

i'd prefer the json solution.

@CMatias
Copy link

CMatias commented Feb 15, 2019

For anyone wondering, in version 4.0.6 the Link should be lowercase:

<ReactMarkdown 
  source={source}
  renderers={{link: props => <a href={props.href} target="_blank">{props.children}</a>}}
/>

@lebyanelm
Copy link

lebyanelm commented Dec 9, 2021

This has now been changed, renderers attribute is no longer being used. linkTarget is the attribute, with values like _blank. More about the APIs can be found in the API Docs https://github.com/remarkjs/react-markdown#api.

@gapon2401
Copy link

gapon2401 commented Dec 22, 2021

Here is how you can add rel="noopener noreferrer" for target="_blank":

<ReactMarkdown
    linkTarget={'_blank'}
    components={{
        a: ({ node, children, ...props}) => {
            const linkProps = props;
            if (props.target === '_blank') {
                linkProps['rel'] = 'noopener noreferrer';
            }
            return <a {...linkProps}>{children}</a>
        }
    }}
>
{`[Link with rel="noopener noreferrer"](https://somedomain.com])`}
</ReactMarkdown>

@YusukeSano
Copy link

This method is not a good one because the anchor link will also open in a new tab.
I think the best practice is to identify internal and external links.

<ReactMarkdown
  components={{
    a: ({ node, children, ...props }) => {
      if (props.href?.includes('http')) {
        props.target = '_blank'
        props.rel = 'noopener noreferrer'
      }
      return <a {...props}>{children}</a>
    },
  }}
>
  {`[External Link](https://example.com)\n\n[Internal Link](#anchor)`}
</ReactMarkdown>

Rendered as:

<p>
  <a href="https://example.com" target="_blank" rel="noopener noreferrer">External Link</a>
</p>
<p>
  <a href="#anchor">Internal Link</a>
</p>

@szhu
Copy link

szhu commented Jul 4, 2023

Appreciate all the code snippets everyone's shared above. I wanted to share an improved version that I'm now using, that are more robust than checking whether the URL contains http (which fails for links like /path/containing/http and about:blank):

If you want to check whether the link points to the same domain, regardless of whether it's an absolute URL or not:

<ReactMarkdown
  components={{
    a({ node, children, ...props }) {
      let url = new URL(props.href ?? "", location.href);
      if (url.origin !== location.origin) {
        props.target = "_blank";
        props.rel = "noopener noreferrer";
      }

      return <a {...props}>{children}</a>;
    },
  }}
>
  {`[External Link](https://example.com)\n\n[Internal Link](#anchor)`}
</ReactMarkdown>

If you just want to check whether the link is an absolute URL:

<ReactMarkdown
  components={{
    a({ node, children, ...props }) {
      try {
        new URL(props.href ?? "");
        // If we don't get an error, then it's an absolute URL.

        props.target = "_blank";
        props.rel = "noopener noreferrer";
      } catch (e) {}

      return <a {...props}>{children}</a>;
    },
  }}
>
  {`[External Link](https://example.com)\n\n[Internal Link](#anchor)`}
</ReactMarkdown>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests