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

NavSingleItem: Allow setting a target attribute to an external item #9

Merged
merged 4 commits into from
Sep 4, 2019
Merged

Conversation

cstuder
Copy link
Contributor

@cstuder cstuder commented Aug 27, 2019

Sample from _nav.js:

bottom: [
    {
      name: 'Get Vibe',
      url: 'https://github.com/NiceDash/Vibe',
      icon: 'GitHub',
      external: true,
      target: '_blank'
    }

I would keep it as an option and not link to external items with a _blank target as default.

@julians300
Copy link
Member

Great idea :)

Consider adding your example to _nav.js as well.

@cstuder
Copy link
Contributor Author

cstuder commented Aug 28, 2019

I will try.

There's an additional security problem with target="_blank" requiring rel="noopener" as well (See https://mathiasbynens.github.io/rel-noopener/).

Additionally maybe a refactoring could also drop the requirement of adding external=true when putting an absolute URL as parameter.

When target is set to `_blank`, `rel="noopener"` is a required security practice. `rel="noreferrer` is for older browsers.

Details: https://mathiasbynens.github.io/rel-noopener/
@cstuder
Copy link
Contributor Author

cstuder commented Aug 29, 2019

Updated, what do you think?

Copy link
Member

@julians300 julians300 left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks!

@julians300 julians300 merged commit 75df2ab into NiceDash:master Sep 4, 2019
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