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

Allow NavLinks to open in the same window (applies to default theme) #1353

Closed
bryanvaz opened this issue Feb 25, 2019 · 4 comments · Fixed by #1734 · May be fixed by adamlaska/osmos-cosmos-sdk#5
Closed

Allow NavLinks to open in the same window (applies to default theme) #1353

bryanvaz opened this issue Feb 25, 2019 · 4 comments · Fixed by #1734 · May be fixed by adamlaska/osmos-cosmos-sdk#5
Labels
contribution welcome Contributions welcome has PR Has a related PR

Comments

@bryanvaz
Copy link

Feature request

Similar to #186, the nav links in the the default theme should also allow devs to define if a link should open in a new tab or the same tab.

What problem does this feature solve?

If there are .html files in the public directory, or external sites we want to link to (one of our links says "Back to "), but not open in a new window, we should at least have the option to override the target=_blank.

What does the proposed API look like?

Same as before

How should this be implemented in your opinion?

So essentially, the theme config would look like this if we wanted an external link to open in the same tab:

themeConfig: {
    nav: [
      { text: 'Home', link: '/' },
      {
        text: 'Partners',
        items: [
          { text: 'Site 1 that opens in a new tab', link: 'https://site1.com' },
          { text: 'Site 2 opens in same tab', link: 'https://site2.com', target: null }
        ]
      }
    ]
  }

with the same logic as #186 applied to the NavLink component

Are you willing to work on this yourself?**

Sure

@ulivz
Copy link
Member

ulivz commented Feb 26, 2019

Try:

module.exports = {
   markdown: {
      externalLinks: {
          target: '_self',
          rel: ''
      }
   }
}

@ulivz ulivz closed this as completed Feb 26, 2019
@bryanvaz
Copy link
Author

bryanvaz commented Feb 26, 2019

@ulivz thanks, but I'm talking about the NavLink component in the default theme's header:

<a
v-else
:href="link"
class="nav-link external"
:target="isMailto(link) || isTel(link) ? null : '_blank'"
:rel="isMailto(link) || isTel(link) ? null : 'noopener noreferrer'"
>

The suggestion you provided only affects page content written in markdown.

The change should probably be along the lines of:

 <a 
   v-else 
   :href="link" 
   class="nav-link external" 
   :target="resolveTarget(link)" 
   :rel="isMailto(link) || isTel(link) ? null : 'noopener noreferrer'" 
 >

...

resolveTarget(link) {
  if (link.target) return link.target;
  if (isMailto(link) || isTel(link)) return null;
  return '_blank'; 
}

Can you re-open the issue and I can create a PR against it?

@burasuk
Copy link

burasuk commented Apr 10, 2019

I would also use it

@ulivz ulivz reopened this Apr 13, 2019
@ulivz ulivz added the contribution welcome Contributions welcome label Apr 13, 2019
@yangwao
Copy link

yangwao commented Jun 11, 2019

I ran in this issue as well. I can drop $5 for this in Eth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome Contributions welcome has PR Has a related PR
Projects
None yet
5 participants