From 7740aa3cf52e18673188bbd174bcb50326e0f7a8 Mon Sep 17 00:00:00 2001
From: Hannah Mudge <Heenawter@users.noreply.github.com>
Date: Wed, 15 Nov 2023 09:53:24 -0700
Subject: [PATCH] [EuiListGroupItem] Add external prop (#7352)

Co-authored-by: Cee Chen <constance.chen@elastic.co>
---
 changelogs/upcoming/7352.md                   |  2 +
 .../views/list_group/list_group_example.js    | 11 +++
 .../src/views/list_group/list_group_links.tsx |  5 +-
 .../collapsible_nav_link.test.tsx.snap        |  4 +-
 .../link/__snapshots__/link.test.tsx.snap     | 21 +----
 .../link/external_link_icon.test.tsx          | 85 +++++++++++++++++++
 src/components/link/external_link_icon.tsx    | 67 +++++++++++++++
 src/components/link/link.styles.ts            | 14 +--
 src/components/link/link.test.tsx             |  7 --
 src/components/link/link.tsx                  | 35 ++------
 .../list_group/list_group_item.styles.ts      |  3 +
 .../list_group/list_group_item.test.tsx       | 17 ++++
 src/components/list_group/list_group_item.tsx | 14 ++-
 13 files changed, 209 insertions(+), 76 deletions(-)
 create mode 100644 changelogs/upcoming/7352.md
 create mode 100644 src/components/link/external_link_icon.test.tsx
 create mode 100644 src/components/link/external_link_icon.tsx

diff --git a/changelogs/upcoming/7352.md b/changelogs/upcoming/7352.md
new file mode 100644
index 00000000000..0ae57cf1474
--- /dev/null
+++ b/changelogs/upcoming/7352.md
@@ -0,0 +1,2 @@
+- Updated `EuiListGroupItem` to render an external icon and screen reader affordance for links with `target` set to to `_blank`
+- Updated `EuiListGroupItem` with a new `external` prop, which allows enabling or disabling the new external link icon
diff --git a/src-docs/src/views/list_group/list_group_example.js b/src-docs/src/views/list_group/list_group_example.js
index b34bad0618f..76a23f0b8d0 100644
--- a/src-docs/src/views/list_group/list_group_example.js
+++ b/src-docs/src/views/list_group/list_group_example.js
@@ -79,6 +79,17 @@ export const ListGroupExample = {
             <EuiCode>isActive</EuiCode> and <EuiCode>isDisabled</EuiCode>{' '}
             properties.
           </p>
+          <p>
+            If your link is external or will open in a new tab, you can manually{' '}
+            set the <EuiCode>external</EuiCode> property. However, just like{' '}
+            with the{' '}
+            <Link to="/navigation/link">
+              <strong>EuiLink</strong>
+            </Link>{' '}
+            component, setting{' '}
+            <EuiCode language="tsx">{'target="_blank"'}</EuiCode> defaults to{' '}
+            <EuiCode language="tsx">{'external={true}'}</EuiCode>.
+          </p>
           <p>
             As is done in this example, the <strong>EuiListGroup</strong>{' '}
             component can also accept an array of items via the{' '}
diff --git a/src-docs/src/views/list_group/list_group_links.tsx b/src-docs/src/views/list_group/list_group_links.tsx
index 34fb9abeaee..283c4597e2d 100644
--- a/src-docs/src/views/list_group/list_group_links.tsx
+++ b/src-docs/src/views/list_group/list_group_links.tsx
@@ -26,9 +26,10 @@ const myContent = [
     iconType: 'copyClipboard',
   },
   {
-    label: 'Fifth link',
-    href: '#/display/list-group',
+    label: 'Fifth link will open in new tab',
+    href: 'http://www.elastic.co',
     iconType: 'crosshairs',
+    target: '_blank',
   },
 ];
 
diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_link.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_link.test.tsx.snap
index caea9f2b9c8..3b583eae7e1 100644
--- a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_link.test.tsx.snap
+++ b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_link.test.tsx.snap
@@ -20,13 +20,13 @@ exports[`EuiCollapsibleNavLink renders a link 1`] = `
 >
   Link
   <span
-    class="emotion-euiLink__externalIcon"
+    class="emotion-EuiExternalLinkIcon"
     data-euiicon-type="popout"
   >
     External link
   </span>
   <span
-    class="emotion-euiScreenReaderOnly-euiLink__screenReaderText"
+    class="emotion-euiScreenReaderOnly"
   >
     (opens in a new tab or window)
   </span>
diff --git a/src/components/link/__snapshots__/link.test.tsx.snap b/src/components/link/__snapshots__/link.test.tsx.snap
index 0cbb30de8b8..226cb0a1f84 100644
--- a/src/components/link/__snapshots__/link.test.tsx.snap
+++ b/src/components/link/__snapshots__/link.test.tsx.snap
@@ -7,21 +7,6 @@ exports[`EuiLink accent is rendered 1`] = `
 />
 `;
 
-exports[`EuiLink allows for target and external to be controlled independently 1`] = `
-<a
-  class="euiLink emotion-euiLink-primary"
-  href="#"
-  rel="noopener noreferrer"
-  target="_blank"
->
-  <span
-    class="emotion-euiScreenReaderOnly-euiLink__screenReaderText"
-  >
-    (opens in a new tab or window)
-  </span>
-</a>
-`;
-
 exports[`EuiLink button respects the type property 1`] = `
 <button
   class="euiLink emotion-euiLink-primary"
@@ -57,7 +42,7 @@ exports[`EuiLink it is an external link 1`] = `
   rel="noreferrer"
 >
   <span
-    class="emotion-euiLink__externalIcon"
+    class="emotion-EuiExternalLinkIcon"
     data-euiicon-type="popout"
   >
     External link
@@ -147,13 +132,13 @@ exports[`EuiLink supports target 1`] = `
   target="_blank"
 >
   <span
-    class="emotion-euiLink__externalIcon"
+    class="emotion-EuiExternalLinkIcon"
     data-euiicon-type="popout"
   >
     External link
   </span>
   <span
-    class="emotion-euiScreenReaderOnly-euiLink__screenReaderText"
+    class="emotion-euiScreenReaderOnly"
   >
     (opens in a new tab or window)
   </span>
diff --git a/src/components/link/external_link_icon.test.tsx b/src/components/link/external_link_icon.test.tsx
new file mode 100644
index 00000000000..5a16c7e9d8c
--- /dev/null
+++ b/src/components/link/external_link_icon.test.tsx
@@ -0,0 +1,85 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import React from 'react';
+import { render } from '../../test/rtl';
+import { shouldRenderCustomStyles } from '../../test/internal';
+
+import { EuiExternalLinkIcon } from './external_link_icon';
+
+// Note - the icon is not actually text, but it's mocked as such
+describe('EuiExternalLinkIcon', () => {
+  shouldRenderCustomStyles(<EuiExternalLinkIcon external={true} />);
+
+  it('always renders the icon if `external` is true', () => {
+    const { container } = render(<EuiExternalLinkIcon external={true} />);
+    expect(container).toMatchInlineSnapshot(`
+      <div>
+        <span
+          class="emotion-EuiExternalLinkIcon"
+          data-euiicon-type="popout"
+        >
+          External link
+        </span>
+      </div>
+    `);
+  });
+
+  describe('target="_blank"', () => {
+    it('renders the icon by default, along with screen reader text', () => {
+      const { container } = render(<EuiExternalLinkIcon target="_blank" />);
+      expect(container).toMatchInlineSnapshot(`
+        <div>
+          <span
+            class="emotion-EuiExternalLinkIcon"
+            data-euiicon-type="popout"
+          >
+            External link
+          </span>
+          <span
+            class="emotion-euiScreenReaderOnly"
+          >
+            (opens in a new tab or window)
+          </span>
+        </div>
+      `);
+    });
+
+    it('hides the icon if `external` is false, but still shows the screen reader text', () => {
+      const { container } = render(
+        <EuiExternalLinkIcon target="_blank" external={false} />
+      );
+      expect(container).toMatchInlineSnapshot(`
+        <div>
+          <span
+            class="emotion-euiScreenReaderOnly"
+          >
+            (opens in a new tab or window)
+          </span>
+        </div>
+      `);
+    });
+  });
+
+  it('renders nothing if neither external nor target="_blank" are set', () => {
+    const { container } = render(<EuiExternalLinkIcon />);
+    expect(container).toMatchInlineSnapshot(`<div />`);
+  });
+
+  it('allows configuring the icon props', () => {
+    const { getByTestSubject } = render(
+      <EuiExternalLinkIcon
+        external={true}
+        data-test-subj="test"
+        size="xl"
+        color="text"
+      />
+    );
+    expect(getByTestSubject('test')).toBeInTheDocument();
+  });
+});
diff --git a/src/components/link/external_link_icon.tsx b/src/components/link/external_link_icon.tsx
new file mode 100644
index 00000000000..f74093c46c2
--- /dev/null
+++ b/src/components/link/external_link_icon.tsx
@@ -0,0 +1,67 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import React, { FunctionComponent, AnchorHTMLAttributes } from 'react';
+
+import { useEuiTheme } from '../../services';
+import { logicalStyle } from '../../global_styling';
+import { EuiIcon, EuiIconProps } from '../icon';
+import { EuiI18n, useEuiI18n } from '../i18n';
+import { EuiScreenReaderOnly } from '../accessibility';
+
+/**
+ * DRY util for indicating external links both via icon and to
+ * screen readers. Used internally by at EuiLink and EuiListGroupItem
+ */
+
+export type EuiExternalLinkIconProps = {
+  target?: AnchorHTMLAttributes<HTMLAnchorElement>['target'];
+  /**
+   * Set to true to show an icon indicating that it is an external link;
+   * Defaults to true if `target="_blank"`
+   */
+  external?: boolean;
+};
+
+export const EuiExternalLinkIcon: FunctionComponent<
+  EuiExternalLinkIconProps & Partial<EuiIconProps>
+> = ({ target, external, ...rest }) => {
+  const { euiTheme } = useEuiTheme();
+
+  const showExternalLinkIcon =
+    (target === '_blank' && external !== false) || external === true;
+
+  const iconAriaLabel = useEuiI18n(
+    'euiExternalLinkIcon.ariaLabel',
+    'External link'
+  );
+
+  return (
+    <>
+      {showExternalLinkIcon && (
+        <EuiIcon
+          css={logicalStyle('margin-left', euiTheme.size.xs)}
+          aria-label={iconAriaLabel}
+          size="s"
+          type="popout"
+          {...rest}
+        />
+      )}
+      {target === '_blank' && (
+        <EuiScreenReaderOnly>
+          <span>
+            <EuiI18n
+              token="euiExternalLinkIcon.newTarget.screenReaderOnlyText"
+              default="(opens in a new tab or window)"
+            />
+          </span>
+        </EuiScreenReaderOnly>
+      )}
+    </>
+  );
+};
diff --git a/src/components/link/link.styles.ts b/src/components/link/link.styles.ts
index 3abe2ab8a24..7dd64046b26 100644
--- a/src/components/link/link.styles.ts
+++ b/src/components/link/link.styles.ts
@@ -8,11 +8,7 @@
 
 import { css } from '@emotion/react';
 import { UseEuiTheme } from '../../services';
-import {
-  euiFocusRing,
-  logicalCSS,
-  logicalTextAlignCSS,
-} from '../../global_styling';
+import { euiFocusRing, logicalTextAlignCSS } from '../../global_styling';
 
 const _colorCSS = (color: string) => {
   return `
@@ -87,13 +83,5 @@ export const euiLinkStyles = (euiThemeContext: UseEuiTheme) => {
     warning: css(_colorCSS(euiTheme.colors.warningText)),
     ghost: css(_colorCSS(euiTheme.colors.ghost)),
     text: css(_colorCSS(euiTheme.colors.text)),
-
-    // Children
-    euiLink__screenReaderText: css`
-      ${logicalCSS('left', '0px')}
-    `,
-    euiLink__externalIcon: css`
-      ${logicalCSS('margin-left', euiTheme.size.xs)}
-    `,
   };
 };
diff --git a/src/components/link/link.test.tsx b/src/components/link/link.test.tsx
index db039cef184..220d0613047 100644
--- a/src/components/link/link.test.tsx
+++ b/src/components/link/link.test.tsx
@@ -59,13 +59,6 @@ describe('EuiLink', () => {
     expect(container.firstChild).toMatchSnapshot();
   });
 
-  test('allows for target and external to be controlled independently', () => {
-    const { container } = render(
-      <EuiLink href="#" target="_blank" external={false} />
-    );
-    expect(container.firstChild).toMatchSnapshot();
-  });
-
   test('supports rel', () => {
     const { container } = render(<EuiLink href="hoi" rel="stylesheet" />);
     expect(container.firstChild).toMatchSnapshot();
diff --git a/src/components/link/link.tsx b/src/components/link/link.tsx
index 434352b3641..dc7c84da32c 100644
--- a/src/components/link/link.tsx
+++ b/src/components/link/link.tsx
@@ -13,14 +13,14 @@ import React, {
   MouseEventHandler,
 } from 'react';
 import classNames from 'classnames';
+
 import { getSecureRelForTarget, useEuiTheme } from '../../services';
-import { euiLinkStyles } from './link.styles';
-import { EuiIcon } from '../icon';
-import { EuiI18n, useEuiI18n } from '../i18n';
 import { CommonProps, ExclusiveUnion } from '../common';
-import { EuiScreenReaderOnly } from '../accessibility';
 import { validateHref } from '../../services/security/href_validator';
 
+import { EuiExternalLinkIcon } from './external_link_icon';
+import { euiLinkStyles } from './link.styles';
+
 export type EuiLinkType = 'button' | 'reset' | 'submit';
 
 export const COLORS = [
@@ -95,32 +95,10 @@ const EuiLink = forwardRef<HTMLAnchorElement | HTMLButtonElement, EuiLinkProps>(
     const euiTheme = useEuiTheme();
     const styles = euiLinkStyles(euiTheme);
     const cssStyles = [styles.euiLink];
-    const cssScreenReaderTextStyles = [styles.euiLink__screenReaderText];
-    const cssExternalLinkIconStyles = [styles.euiLink__externalIcon];
 
     const isHrefValid = !href || validateHref(href);
     const disabled = _disabled || !isHrefValid;
 
-    const newTargetScreenreaderText = (
-      <EuiScreenReaderOnly css={cssScreenReaderTextStyles}>
-        <span>
-          <EuiI18n
-            token="euiLink.newTarget.screenReaderOnlyText"
-            default="(opens in a new tab or window)"
-          />
-        </span>
-      </EuiScreenReaderOnly>
-    );
-
-    const externalLinkIcon = (
-      <EuiIcon
-        aria-label={useEuiI18n('euiLink.external.ariaLabel', 'External link')}
-        size="s"
-        css={cssExternalLinkIconStyles}
-        type="popout"
-      />
-    );
-
     if (href === undefined || !isHrefValid) {
       const buttonProps = {
         className: classNames('euiLink', className),
@@ -152,8 +130,6 @@ const EuiLink = forwardRef<HTMLAnchorElement | HTMLButtonElement, EuiLinkProps>(
       onClick,
       ...rest,
     };
-    const showExternalLinkIcon =
-      (target === '_blank' && external !== false) || external === true;
 
     return (
       <a
@@ -161,8 +137,7 @@ const EuiLink = forwardRef<HTMLAnchorElement | HTMLButtonElement, EuiLinkProps>(
         {...(anchorProps as EuiLinkAnchorProps)}
       >
         {children}
-        {showExternalLinkIcon && externalLinkIcon}
-        {target === '_blank' && newTargetScreenreaderText}
+        <EuiExternalLinkIcon external={external} target={target} />
       </a>
     );
   }
diff --git a/src/components/list_group/list_group_item.styles.ts b/src/components/list_group/list_group_item.styles.ts
index 47204d84e58..d68dba8675f 100644
--- a/src/components/list_group/list_group_item.styles.ts
+++ b/src/components/list_group/list_group_item.styles.ts
@@ -163,6 +163,9 @@ export const euiListGroupItemInnerStyles = (euiThemeContext: UseEuiTheme) => {
         text-decoration: underline;
       }
     `,
+    externalIcon: css`
+      ${logicalCSS('margin-left', euiTheme.size.xs)}
+    `,
   };
 };
 
diff --git a/src/components/list_group/list_group_item.test.tsx b/src/components/list_group/list_group_item.test.tsx
index 4230910b515..c41cb0f5dfd 100644
--- a/src/components/list_group/list_group_item.test.tsx
+++ b/src/components/list_group/list_group_item.test.tsx
@@ -7,6 +7,7 @@
  */
 
 import React from 'react';
+
 import { fireEvent } from '@testing-library/react';
 import { shouldRenderCustomStyles } from '../../test/internal';
 import { requiredProps } from '../../test/required_props';
@@ -205,6 +206,22 @@ describe('EuiListGroupItem', () => {
       });
     });
 
+    describe('external', () => {
+      test('is rendered', () => {
+        const { getByText } = render(
+          <EuiListGroupItem label="Label" href="#" external />
+        );
+        expect(getByText('External link')).toBeInTheDocument();
+      });
+
+      test('target `_blank` renders external icon', () => {
+        const { getByText } = render(
+          <EuiListGroupItem label="Label" href="#" target="_blank" />
+        );
+        expect(getByText('External link')).toBeInTheDocument();
+      });
+    });
+
     describe('onClick', () => {
       test('is rendered', () => {
         const { container } = render(
diff --git a/src/components/list_group/list_group_item.tsx b/src/components/list_group/list_group_item.tsx
index 4ee2d2cc93d..f28301bdce9 100644
--- a/src/components/list_group/list_group_item.tsx
+++ b/src/components/list_group/list_group_item.tsx
@@ -33,6 +33,7 @@ import {
   cloneElementWithCss,
 } from '../../services';
 import { validateHref } from '../../services/security/href_validator';
+import { EuiExternalLinkIcon } from '../link/external_link_icon';
 
 import {
   euiListGroupItemStyles,
@@ -89,10 +90,13 @@ export type EuiListGroupItemProps = CommonProps &
      * While permitted, `href` and `onClick` should not be used together in most cases and may create problems.
      */
     href?: string;
-
-    target?: string;
-
     rel?: string;
+    target?: string;
+    /**
+     * Set to true to show an icon indicating that it is an external link;
+     * Defaults to true if `target="_blank"`
+     */
+    external?: boolean;
 
     /**
      * Adds `EuiIcon` of `EuiIcon.type`
@@ -157,8 +161,9 @@ export const EuiListGroupItem: FunctionComponent<EuiListGroupItemProps> = ({
   isActive = false,
   isDisabled: _isDisabled = false,
   href,
-  target,
   rel,
+  target,
+  external,
   className,
   css: customCss,
   style,
@@ -289,6 +294,7 @@ export const EuiListGroupItem: FunctionComponent<EuiListGroupItemProps> = ({
       >
         {iconNode}
         {labelContent}
+        <EuiExternalLinkIcon external={external} target={target} />
       </a>
     );
   } else if ((href && isDisabled) || onClick) {