fix: Correctly apply externalIconAriaLabel for top navigation utiliti…

…es (#536)
cloudscape-design · Dec 5, 2022 · a514891 · a514891
1 parent af152c5
commit a514891
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 19 deletions.
diff --git a/src/top-navigation/__integ__/top-navigation.test.ts b/src/top-navigation/__integ__/top-navigation.test.ts
@@ -26,7 +26,7 @@ describe('Top navigation', () => {
       'renders utilities with text',
       setupTest(pageWidth, async page => {
         await expect(page.getText(wrapper.findUtility(1).toSelector())).resolves.toBe('New thing');
-        await expect(page.getText(wrapper.findUtility(2).toSelector())).resolves.toBe('Docs');
+        await expect(page.getText(wrapper.findUtility(2).toSelector())).resolves.toBe('Docs ');
         await expect(page.getText(wrapper.findUtility(4).toSelector())).resolves.toBe('John Doe');
       })
     );
@@ -51,7 +51,7 @@ describe('Top navigation', () => {
         await expect(page.getText(wrapper.findUtility(1).toSelector())).resolves.toBe('New thing');
 
         // Docs shouldn't be collapsed because it doesn't have an icon.
-        await expect(page.getText(wrapper.findUtility(2).toSelector())).resolves.toBe('Docs');
+        await expect(page.getText(wrapper.findUtility(2).toSelector())).resolves.toBe('Docs ');
       })
     );
   });

diff --git a/src/top-navigation/__tests__/top-navigation.test.tsx b/src/top-navigation/__tests__/top-navigation.test.tsx
@@ -147,9 +147,10 @@ describe('TopNavigation Component', () => {
     });
 
     test('renders an external icon with the provided label', () => {
-      expect(
-        topNavigation.findUtility(1)!.findButtonLinkType()!.find('[aria-label="opens in a new tab"]')
-      ).toBeTruthy();
+      expect(topNavigation.findUtility(1)!.findButtonLinkType()!.getElement()).toHaveAttribute(
+        'aria-label',
+        'External link opens in a new tab'
+      );
     });
 
     test('calls onClick when clicked', () => {

diff --git a/src/top-navigation/parts/utility.tsx b/src/top-navigation/parts/utility.tsx
@@ -12,6 +12,7 @@ import { TopNavigationProps } from '../interfaces';
 
 import styles from '../styles.css.js';
 import { checkSafeUrl } from '../../internal/utils/check-safe-url';
+import { joinStrings } from '../../internal/utils/strings';
 
 export interface UtilityProps {
   hideText: boolean;
@@ -22,9 +23,12 @@ export interface UtilityProps {
 export default function Utility({ hideText, definition, offsetRight }: UtilityProps) {
   const hasIcon = !!definition.iconName || !!definition.iconUrl || !!definition.iconAlt || !!definition.iconSvg;
   const shouldHideText = hideText && !definition.disableTextCollapse && hasIcon;
-  const ariaLabel = definition.ariaLabel ?? definition.text;
+  let ariaLabel = definition.ariaLabel ?? definition.text;
 
   if (definition.type === 'button') {
+    ariaLabel = definition.ariaLabel
+      ? definition.ariaLabel
+      : joinStrings(definition.text, definition.externalIconAriaLabel);
     checkSafeUrl('TopNavigation', definition.href);
     if (definition.variant === 'primary-button') {
       return (
@@ -67,7 +71,7 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
           <InternalLink
             variant="top-navigation"
             href={definition.href}
-            target={definition.external ? '_blank' : undefined}
+            external={definition.external}
             onFollow={definition.onClick}
             ariaLabel={ariaLabel}
           >
@@ -83,18 +87,6 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
             {!shouldHideText && definition.text && (
               <span className={hasIcon ? styles['utility-link-icon'] : undefined}>{definition.text}</span>
             )}
-            {/* HACK: The link component uses size="inherit", and Firefox scales up SVGs massively on the first render */}
-            {definition.external && (
-              <>
-                {' '}
-                <span
-                  role={definition.externalIconAriaLabel ? 'img' : undefined}
-                  aria-label={definition.externalIconAriaLabel}
-                >
-                  <InternalIcon name="external" size="normal" />
-                </span>
-              </>
-            )}
           </InternalLink>
         </span>
       );