fix(sidenav-link): external link opens in new tab (#645)

External links now will open in a new tab matching the documentation, as well as having rel="noopener" added to the link. If a target is manually specified it will be used instead of the "_blank" set by it being external.
ithaka · Nov 6, 2023 · 1875aaa · 1875aaa
1 parent 55c263f
commit 1875aaa
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 4 deletions.
diff --git a/.changeset/large-tomatoes-pull.md b/.changeset/large-tomatoes-pull.md
@@ -0,0 +1,5 @@
+---
+'@ithaka/pharos': patch
+---
+
+Update external sidenav links to open in a new tab
diff --git a/packages/pharos/src/components/sidenav/PharosSidenav.react.stories.jsx b/packages/pharos/src/components/sidenav/PharosSidenav.react.stories.jsx
@@ -66,10 +66,10 @@ export const Base = {
             <PharosSidenavLink href="#">Menu item 2</PharosSidenavLink>
           </PharosSidenavMenu>
           <PharosSidenavLink href="#">Menu item</PharosSidenavLink>
-          <PharosSidenavLink href="#" target="_blank" external>
+          <PharosSidenavLink href="#" external>
             External link
           </PharosSidenavLink>
-          <PharosSidenavLink href="#" target="_blank" external>
+          <PharosSidenavLink href="#" external>
             External link
           </PharosSidenavLink>
         </PharosSidenavSection>

diff --git a/packages/pharos/src/components/sidenav/pharos-sidenav-link.test.ts b/packages/pharos/src/components/sidenav/pharos-sidenav-link.test.ts
@@ -25,4 +25,17 @@ describe('pharos-sidenav-link', () => {
     );
     expect(icon).not.to.be.null;
   });
+  it('opens links in a new tab when the link is external', async () => {
+    component.external = true;
+    await component.updateComplete;
+    const link = component.renderRoot.querySelector('a[target="_blank"][rel="noopener"]');
+    expect(link).not.to.be.null;
+  });
+  it('prioritizes a manually set target when link is set to external', async () => {
+    component.external = true;
+    component.target = '_top';
+    await component.updateComplete;
+    const link = component.renderRoot.querySelector('a[target="_top"]');
+    expect(link).not.to.be.null;
+  });
 });
diff --git a/packages/pharos/src/components/sidenav/pharos-sidenav-link.ts b/packages/pharos/src/components/sidenav/pharos-sidenav-link.ts
@@ -56,6 +56,10 @@ export class PharosSidenavLink extends ScopedRegistryMixin(PharosLink) {
   }
 
   protected override render(): TemplateResult {
+    if (!this.target && this.external) {
+      this.target = '_blank';
+      this.rel = 'noopener';
+    }
     return html`${super.render()}`;
   }
 }
diff --git a/packages/pharos/src/components/sidenav/pharos-sidenav.wc.stories.jsx b/packages/pharos/src/components/sidenav/pharos-sidenav.wc.stories.jsx
@@ -48,10 +48,10 @@ export const Base = {
             <storybook-pharos-sidenav-link href="#">Menu item 2</storybook-pharos-sidenav-link>
           </storybook-pharos-sidenav-menu>
           <storybook-pharos-sidenav-link href="#">Menu item</storybook-pharos-sidenav-link>
-          <storybook-pharos-sidenav-link href="#" target="_blank" external
+          <storybook-pharos-sidenav-link href="#" external target="_top"
             >External link</storybook-pharos-sidenav-link
           >
-          <storybook-pharos-sidenav-link href="#" target="_blank" external
+          <storybook-pharos-sidenav-link href="#" external
             >External link</storybook-pharos-sidenav-link
           >
         </storybook-pharos-sidenav-section>