Fixes special clicks and 3rd party icon sizes in nav (#69767) (#69950)

elastic · Jun 25, 2020 · 08ced76 · 08ced76
1 parent 4433dea
commit 08ced76
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 22 deletions.
diff --git a/src/core/public/chrome/ui/header/collapsible_nav.tsx b/src/core/public/chrome/ui/header/collapsible_nav.tsx
@@ -38,7 +38,7 @@ import { AppCategory } from '../../../../types';
 import { InternalApplicationStart } from '../../../application/types';
 import { HttpStart } from '../../../http';
 import { OnIsLockedUpdate } from './';
-import { createEuiListItem, createRecentNavLink } from './nav_link';
+import { createEuiListItem, createRecentNavLink, isModifiedOrPrevented } from './nav_link';
 
 function getAllCategories(allCategorizedLinks: Record<string, ChromeNavLink[]>) {
   const allCategories = {} as Record<string, AppCategory | undefined>;
@@ -150,17 +150,13 @@ export function CollapsibleNav({
                 label: 'Home',
                 iconType: 'home',
                 href: homeHref,
-                onClick: (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
-                  closeNav();
-                  if (
-                    event.isDefaultPrevented() ||
-                    event.altKey ||
-                    event.metaKey ||
-                    event.ctrlKey
-                  ) {
+                onClick: (event) => {
+                  if (isModifiedOrPrevented(event)) {
                     return;
                   }
+
                   event.preventDefault();
+                  closeNav();
                   navigateToApp('home');
                 },
               },
@@ -196,7 +192,13 @@ export function CollapsibleNav({
               return {
                 ...hydratedLink,
                 'data-test-subj': 'collapsibleNavAppLink--recent',
-                onClick: closeNav,
+                onClick: (event) => {
+                  if (isModifiedOrPrevented(event)) {
+                    return;
+                  }
+
+                  closeNav();
+                },
               };
             })}
             maxWidth="none"

diff --git a/src/core/public/chrome/ui/header/nav_link.tsx b/src/core/public/chrome/ui/header/nav_link.tsx
@@ -17,20 +17,15 @@
  * under the License.
  */
 
-import { EuiImage } from '@elastic/eui';
+import { EuiIcon } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import React from 'react';
 import { ChromeNavLink, ChromeRecentlyAccessedHistoryItem, CoreStart } from '../../..';
 import { HttpStart } from '../../../http';
 import { relativeToAbsolute } from '../../nav_links/to_nav_link';
 
-function isModifiedEvent(event: React.MouseEvent<HTMLButtonElement, MouseEvent>) {
-  return !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);
-}
-
-function LinkIcon({ url }: { url: string }) {
-  return <EuiImage size="s" alt="" aria-hidden={true} url={url} />;
-}
+export const isModifiedOrPrevented = (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) =>
+  event.metaKey || event.altKey || event.ctrlKey || event.shiftKey || event.defaultPrevented;
 
 interface Props {
   link: ChromeNavLink;
@@ -62,13 +57,15 @@ export function createEuiListItem({
     href,
     /* Use href and onClick to support "open in new tab" and SPA navigation in the same link */
     onClick(event: React.MouseEvent<HTMLButtonElement, MouseEvent>) {
-      onClick();
+      if (!isModifiedOrPrevented(event)) {
+        onClick();
+      }
+
       if (
         !legacyMode && // ignore when in legacy mode
         !legacy && // ignore links to legacy apps
-        !event.defaultPrevented && // onClick prevented default
         event.button === 0 && // ignore everything but left clicks
-        !isModifiedEvent(event) // ignore clicks with modifier keys
+        !isModifiedOrPrevented(event)
       ) {
         event.preventDefault();
         navigateToApp(id);
@@ -80,7 +77,8 @@ export function createEuiListItem({
     'data-test-subj': dataTestSubj,
     ...(basePath && {
       iconType: euiIconType,
-      icon: !euiIconType && icon ? <LinkIcon url={basePath.prepend(`/${icon}`)} /> : undefined,
+      icon:
+        !euiIconType && icon ? <EuiIcon type={basePath.prepend(`/${icon}`)} size="m" /> : undefined,
     }),
   };
 }