ExternalLink (ansible#4382)

* ExternalLink use an `ExternalLink` (`{ href, title | children }`) component for all external links (no other `_blank`, `ExternalLinkAltIcon`, `nofollow`, `noreferrer`, `noopener`; no other `<a>` except for `mailto:`) No-Issue * ee test - expect a, not button * about: add missing username when names are set * ExternalLink: title -> children s/title={\([^}]*\)}\( \/>\)\?/>{\1}<\/ExternalLink>/ to match Link and prevent conflict with `title` as an `<a>` attribute * title manual
himdel · Nov 12, 2023 · e265e67 · e265e67
1 parent bbb0fb2
commit e265e67
Show file tree

Hide file tree

Showing 17 changed files with 165 additions and 255 deletions.
diff --git a/src/components/about-modal/about-modal.tsx b/src/components/about-modal/about-modal.tsx
@@ -92,17 +92,19 @@ export const AboutModalWindow = ({
             ) : (
               <ExternalLink
                 href={`https://github.com/ansible/galaxy_ng/releases/tag/${galaxy_ng_version}`}
-                title={galaxy_ng_version}
-              />
+              >
+                {galaxy_ng_version}
+              </ExternalLink>
             )}
             {galaxy_ng_commit ? (
               <>
                 <br />
                 {galaxy_ng_sha ? (
                   <ExternalLink
                     href={`https://github.com/ansible/galaxy_ng/commit/${galaxy_ng_sha}`}
-                    title={galaxy_ng_commit}
-                  />
+                  >
+                    {galaxy_ng_commit}
+                  </ExternalLink>
                 ) : (
                   galaxy_ng_commit
                 )}
@@ -114,32 +116,36 @@ export const AboutModalWindow = ({
           <Value>
             <ExternalLink
               href={`https://github.com/pulp/pulp_ansible/releases/tag/${pulp_ansible_version}`}
-              title={pulp_ansible_version}
-            />
+            >
+              {pulp_ansible_version}
+            </ExternalLink>
           </Value>
 
           <Label>{t`Pulp Container Version`}</Label>
           <Value>
             <ExternalLink
               href={`https://github.com/pulp/pulp_container/releases/tag/${pulp_container_version}`}
-              title={pulp_container_version}
-            />
+            >
+              {pulp_container_version}
+            </ExternalLink>
           </Value>
 
           <Label>{t`Pulp Core Version`}</Label>
           <Value>
             <ExternalLink
               href={`https://github.com/pulp/pulpcore/releases/tag/${pulp_core_version}`}
-              title={pulp_core_version}
-            />
+            >
+              {pulp_core_version}
+            </ExternalLink>
           </Value>
 
           <Label>{t`Galaxy Importer`}</Label>
           <Value>
             <ExternalLink
               href={`https://github.com/ansible/galaxy-importer/releases/tag/v${galaxy_importer_version}`}
-              title={galaxy_importer_version}
-            />
+            >
+              {galaxy_importer_version}
+            </ExternalLink>
           </Value>
 
           {aap_version && (
@@ -153,8 +159,9 @@ export const AboutModalWindow = ({
           <Value>
             <ExternalLink
               href={`https://github.com/ansible/ansible-hub-ui/commit/${ui_sha}`}
-              title={ui_sha}
-            />
+            >
+              {ui_sha}
+            </ExternalLink>
           </Value>
 
           <Label>{t`Username`}</Label>
@@ -164,6 +171,9 @@ export const AboutModalWindow = ({
               title={user.username}
             >
               {userName}
+              {user?.username && user.username !== userName ? (
+                <> ({user.username})</>
+              ) : null}
             </Link>
           </Value>
 

diff --git a/src/components/execution-environment/publish-to-controller-modal.tsx b/src/components/execution-environment/publish-to-controller-modal.tsx
@@ -12,7 +12,6 @@ import {
   ListItem,
   Modal,
 } from '@patternfly/react-core';
-import ExternalLinkAltIcon from '@patternfly/react-icons/dist/esm/icons/external-link-alt-icon';
 import TagIcon from '@patternfly/react-icons/dist/esm/icons/tag-icon';
 import React, { useEffect, useState } from 'react';
 import { ControllerAPI, ExecutionEnvironmentAPI } from 'src/api';
@@ -23,6 +22,7 @@ import {
   CompoundFilter,
   EmptyStateFilter,
   EmptyStateNoData,
+  ExternalLink,
   LoadingPageSpinner,
   Pagination,
   ShaLabel,
@@ -181,7 +181,6 @@ export const PublishToControllerModal = (props: IProps) => {
 
   function renderControllers() {
     const { image, isOpen } = props;
-    const unsafeLinksSupported = !Object.keys(window).includes('chrome');
 
     if (!isOpen || !controllers) {
       return null;
@@ -211,24 +210,15 @@ export const PublishToControllerModal = (props: IProps) => {
 
           return (
             <ListItem style={{ paddingTop: '8px' }} key={host}>
-              <a href={href} target='_blank' rel='noreferrer'>
-                {host}
-              </a>{' '}
-              {unsafeLinksSupported && (
-                <small>
-                  <ExternalLinkAltIcon />
-                </small>
-              )}
-              {!unsafeLinksSupported && (
-                <ClipboardCopyButton
-                  variant={'plain'}
-                  id={href}
-                  textId={t`Copy to clipboard`}
-                  onClick={() => navigator.clipboard.writeText(href)}
-                >
-                  {t`Copy to clipboard`}
-                </ClipboardCopyButton>
-              )}
+              <ExternalLink href={href}>{host}</ExternalLink>
+              <ClipboardCopyButton
+                variant={'plain'}
+                id={href}
+                textId={t`Copy to clipboard`}
+                onClick={() => navigator.clipboard.writeText(href)}
+              >
+                {t`Copy to clipboard`}
+              </ClipboardCopyButton>
             </ListItem>
           );
         })}
@@ -247,19 +237,11 @@ export const PublishToControllerModal = (props: IProps) => {
   const notListedMessage = (
     <>
       {t`If the Controller is not listed in the table, check settings.py.`}{' '}
-      {docsLink && (
-        <>
-          <a href={docsLink} target='_blank' rel='noreferrer'>
-            {t`Learn more`}
-          </a>{' '}
-          <ExternalLinkAltIcon />
-        </>
-      )}
+      {docsLink && <ExternalLink href={docsLink}>{t`Learn more`}</ExternalLink>}
     </>
   );
 
   const Spacer = () => <div style={{ paddingTop: '24px' }} />;
-  const unsafeLinksSupported = !Object.keys(window).includes('chrome');
 
   return (
     <Modal
@@ -341,13 +323,6 @@ export const PublishToControllerModal = (props: IProps) => {
             Log in (if necessary) and follow the steps to complete the
             configuration.
           </Trans>
-          <br />
-          {!unsafeLinksSupported && (
-            <Trans>
-              <b>Note:</b> The following links may be blocked by your browser.
-              Copy and paste the external link manually.
-            </Trans>
-          )}
           <Spacer />
 
           <Flex>

diff --git a/src/components/headers/collection-header.tsx b/src/components/headers/collection-header.tsx
@@ -14,7 +14,6 @@ import {
   Spinner,
   Text,
 } from '@patternfly/react-core';
-import ExternalLinkAltIcon from '@patternfly/react-icons/dist/esm/icons/external-link-alt-icon';
 import React from 'react';
 import { Navigate } from 'react-router-dom';
 import {
@@ -39,6 +38,7 @@ import {
   CopyCollectionToRepositoryModal,
   DeleteCollectionModal,
   DownloadCount,
+  ExternalLink,
   ImportModal,
   LinkTabs,
   Logo,
@@ -579,10 +579,7 @@ export class CollectionHeader extends React.Component<IProps, IState> {
             <Flex>
               {DEPLOYMENT_MODE === Constants.INSIGHTS_DEPLOYMENT_MODE ? (
                 <FlexItem>
-                  <a href={issueUrl} target='_blank' rel='noreferrer'>
-                    {t`Create issue`}
-                  </a>{' '}
-                  <ExternalLinkAltIcon />
+                  <ExternalLink href={issueUrl}>{t`Create issue`}</ExternalLink>
                 </FlexItem>
               ) : null}
               {dropdownItems.length > 0 ? (
@@ -607,9 +604,6 @@ export class CollectionHeader extends React.Component<IProps, IState> {
           <div className='hub-tab-link-container'>
             <div className='tabs'>{this.renderTabs(activeTab)}</div>
             <div className='links'>
-              <div>
-                <ExternalLinkAltIcon />
-              </div>
               {urlKeys.map((link) => {
                 const url = content[link.key];
                 if (!url) {
@@ -618,9 +612,7 @@ export class CollectionHeader extends React.Component<IProps, IState> {
 
                 return (
                   <div className='link' key={link.key}>
-                    <a href={url} target='_blank' rel='noreferrer'>
-                      {link.name}
-                    </a>
+                    <ExternalLink href={url}>{link.name}</ExternalLink>
                   </div>
                 );
               })}

diff --git a/src/components/headers/partner-header.tsx b/src/components/headers/partner-header.tsx
@@ -1,11 +1,11 @@
 import { t } from '@lingui/macro';
-import ExternalLinkAltIcon from '@patternfly/react-icons/dist/esm/icons/external-link-alt-icon';
 import React from 'react';
 import { NamespaceType } from 'src/api';
 import {
   BaseHeader,
   BreadcrumbType,
   Breadcrumbs,
+  ExternalLink,
   Logo,
   Tabs,
   TabsType,
@@ -68,15 +68,10 @@ export class PartnerHeader extends React.Component<IProps> {
           </div>
           {namespace.links.length > 0 ? (
             <div className='links'>
-              <div>
-                <ExternalLinkAltIcon />
-              </div>
               {namespace.links.map((x, i) => {
                 return (
                   <div className='link' key={i}>
-                    <a href={x.url} target='blank'>
-                      {x.name}
-                    </a>
+                    <ExternalLink href={x.url}>{x.name}</ExternalLink>
                   </div>
                 );
               })}

diff --git a/src/components/render-plugin-doc/render-plugin-doc.tsx b/src/components/render-plugin-doc/render-plugin-doc.tsx
@@ -7,6 +7,7 @@ import {
   PluginOption,
   ReturnedValue,
 } from 'src/api';
+import { ExternalLink } from 'src/components';
 import './render-plugin-doc.scss';
 
 // Documentation for module doc string spec
@@ -282,11 +283,7 @@ export class RenderPluginDoc extends React.Component<IProps, IState> {
   }
 
   private formatPartURL(part: dom.URLPart): React.ReactNode {
-    return (
-      <a href={part.url} target='_blank' rel='noreferrer'>
-        {part.url}
-      </a>
-    );
+    return <ExternalLink href={part.url}>{part.url}</ExternalLink>;
   }
 
   private formatPartText(part: dom.TextPart): React.ReactNode {

diff --git a/src/components/repositories/remote-form.tsx b/src/components/repositories/remote-form.tsx
@@ -19,7 +19,12 @@ import ExclamationCircleIcon from '@patternfly/react-icons/dist/esm/icons/exclam
 import ExclamationTriangleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-triangle-icon';
 import React, { ReactNode } from 'react';
 import { RemoteType, WriteOnlyFieldType } from 'src/api';
-import { FileUpload, HelperText, WriteOnlyField } from 'src/components';
+import {
+  ExternalLink,
+  FileUpload,
+  HelperText,
+  WriteOnlyField,
+} from 'src/components';
 import { AppContext } from 'src/loaders/app-context';
 import {
   ErrorMessagesType,
@@ -196,13 +201,9 @@ export class RemoteForm extends React.Component<IProps, IState> {
       ];
 
     const docsAnsibleLink = (
-      <a
-        target='_blank'
-        href='https://docs.ansible.com/ansible/latest/user_guide/collections_using.html#install-multiple-collections-with-a-requirements-file'
-        rel='noreferrer'
-      >
+      <ExternalLink href='https://docs.ansible.com/ansible/latest/user_guide/collections_using.html#install-multiple-collections-with-a-requirements-file'>
         requirements.yml
-      </a>
+      </ExternalLink>
     );
 
     const yamlTemplate = [

diff --git a/src/components/shared/external-link.tsx b/src/components/shared/external-link.tsx
@@ -2,37 +2,47 @@ import ExternalLinkAltIcon from '@patternfly/react-icons/dist/esm/icons/external
 import React, { ReactNode } from 'react';
 
 interface IProps {
-  children?: ReactNode;
-  className?: string;
-  externalLinkIconStyle?: Record<string, string>;
+  children: ReactNode;
+  'data-cy'?: string;
   href: string;
-  title?: string;
+  variant?: 'default' | 'download' | 'menu' | 'nav';
 }
 
+// variants:
+// download - no external link icon (role download)
+// menu - top nav question mark menu (Customer Support, Training)
+// nav - left side nav (Documentation, Terms of Use)
+// default - everywhere else
+
 export const ExternalLink = ({
   children,
-  className,
-  externalLinkIconStyle,
+  'data-cy': dataCy,
   href,
-  title,
+  variant = 'default',
 }: IProps) => {
-  if (!href || !title) {
+  if (!href || !children) {
     return null;
   }
 
+  const iconStyle: Record<string, string> = {
+    nav: { position: 'absolute', right: '32px', top: '22px' },
+    download: { display: 'none' },
+  }[variant];
+  const className = {
+    nav: 'pf-c-nav__link',
+    menu: 'pf-c-dropdown__menu-item',
+  }[variant];
+
   return (
-    <>
-      <a
-        className={className}
-        href={href}
-        rel='noreferrer noopener'
-        target='_blank'
-      >
-        {title || children}
-      </a>{' '}
-      <small style={{ display: 'inline' }}>
-        <ExternalLinkAltIcon style={externalLinkIconStyle || {}} />
-      </small>
-    </>
+    <a
+      className={className}
+      data-cy={dataCy}
+      href={href}
+      rel='nofollow noopener noreferrer'
+      target='_blank'
+    >
+      {children}{' '}
+      <ExternalLinkAltIcon style={{ fontSize: 'smaller', ...iconStyle }} />
+    </a>
   );
 };