Merge pull request #7071 from himdel/menu-helper

Menu link helper

(cherry picked from commit 5a82d5e)

app/helpers/application_helper/navbar.rb

@@ -11,7 +11,7 @@ def item_to_hash(item)
         :id      => item.id.to_s,
         :title   => item.name,
         :icon    => item.icon,
-        :href    => item.link_params[:href],
+        :href    => item.href,
         :type    => item.type,
         :visible => item.visible?,
         :active  => item_active?(item),

app/javascript/components/main-menu/main-menu.jsx

@@ -8,7 +8,6 @@ import TopLevel from './top-level';
 import SecondLevel from './second-level';
 import ThirdLevel from './third-level';
 import { menuProps, RecursiveMenuProps } from './recursive-props';
-import { adaptContentWidth } from './helpers';
 
 const Fallback = props => <ThirdLevel level={2} {...props} />;
 
@@ -26,7 +25,16 @@ const MainMenu = ({ menu }) => {
   const isVerticalMenuCollapsed = useSelector(({ menuReducer: { isVerticalMenuCollapsed } }) => isVerticalMenuCollapsed);
 
   useEffect(() => {
-    adaptContentWidth(isVerticalMenuCollapsed);
+    const content = document.querySelector('.container-pf-nav-pf-vertical-with-sub-menus');
+    if (! content) {
+      return;
+    }
+
+    if (isVerticalMenuCollapsed) {
+      content.classList.add('collapsed-nav');
+    } else {
+      content.classList.remove('collapsed-nav');
+    }
   }, [isVerticalMenuCollapsed]);
 
   const handleSetActiveIds = (value) => {

app/javascript/components/main-menu/second-level.jsx

@@ -3,10 +3,7 @@ import PropTypes from 'prop-types';
 import ClassNames from 'classnames';
 import { MenuItem, HoverContext } from './main-menu';
 import { menuProps } from './recursive-props';
-import {
-  getHrefByType, getIdByCategory, handleUnsavedChanges,
-} from './helpers';
-import getTargetByType from '../../helpers/get-target-by-type';
+import { itemId, linkProps } from '../../menu/item-type';
 
 const SecondLevel = ({
   id,
@@ -31,20 +28,11 @@ const SecondLevel = ({
           active,
         },
       )}
-      id={getIdByCategory(hasSubitems, id)}
+      id={itemId(id, hasSubitems)}
       onMouseEnter={() => (handleSetActiveIds(hasSubitems ? { secondLevelId: id } : undefined))}
       onMouseLeave={() => handleSetActiveIds({ secondLevelId: undefined })}
     >
-      <a
-        href={getHrefByType(type, href, id)}
-        onClick={(event) => {
-          if (handleUnsavedChanges(type) === false) {
-            event.preventDefault();
-          }
-          return false;
-        }}
-        target={getTargetByType(type)}
-      >
+      <a {...linkProps({ type, href, id })}>
         <span className="list-group-item-value">{title}</span>
       </a>
       <div className="nav-pf-tertiary-nav">

app/javascript/components/main-menu/third-level.jsx

@@ -1,8 +1,7 @@
 import React from 'react';
 import ClassNames from 'classnames';
 import { menuProps } from './recursive-props';
-import { getHrefByType, handleUnsavedChanges } from './helpers';
-import getTargetByType from '../../helpers/get-target-by-type';
+import { itemId, linkProps } from '../../menu/item-type';
 
 const ThirdLevel = ({
   id,
@@ -13,24 +12,15 @@ const ThirdLevel = ({
   type,
 }) => (!visible ? null : (
   <li
-    id={`menu_item_${id}`}
+    id={itemId(id)}
     className={ClassNames(
       'menu-list-group-item',
       {
         active,
       },
     )}
   >
-    <a
-      href={getHrefByType(type, href, id)}
-      onClick={(event) => {
-        if (handleUnsavedChanges(type) === false) {
-          event.preventDefault();
-        }
-        return false;
-      }}
-      target={getTargetByType(type)}
-    >
+    <a {...linkProps({ type, href, id })}>
       <span className="list-group-item-value">{title}</span>
     </a>
   </li>

app/javascript/components/main-menu/top-level.jsx

@@ -3,10 +3,7 @@ import PropTypes from 'prop-types';
 import ClassNames from 'classnames';
 import { MenuItem, HoverContext } from './main-menu';
 import { menuProps, RecursiveMenuProps } from './recursive-props';
-import {
-  getHrefByType, getSectionId, handleUnsavedChanges, getItemId,
-} from './helpers';
-import getTargetByType from '../../helpers/get-target-by-type';
+import { itemId, linkProps } from '../../menu/item-type';
 
 const TopLevel = ({
   level,
@@ -32,20 +29,13 @@ const TopLevel = ({
             'is-hover': hoveredTopLevelId === id,
           },
         )}
-        id={getSectionId(id)}
+        id={itemId(id, isSection)}
         onMouseEnter={() => handleSetActiveIds({ topLevelId: id })}
         onBlur={() => undefined}
       >
         <a
           className="top-level-item"
-          href={getHrefByType(type, href, id)}
-          onClick={(event) => {
-            if (handleUnsavedChanges(type) === false) {
-              event.preventDefault();
-            }
-            return false;
-          }}
-          target={getTargetByType(type)}
+          {...linkProps({ type, href, id })}
         >
           <span className={icon} />
           <span className="list-group-item-value">{title}</span>
@@ -68,7 +58,7 @@ const TopLevel = ({
 
   return (
     <li
-      id={getItemId(id)}
+      id={itemId(id, isSection)}
       className={ClassNames(
         'menu-list-group-item',
         {
@@ -81,14 +71,7 @@ const TopLevel = ({
     >
       <a
         className="top-level-item"
-        href={getHrefByType(type, href, id)}
-        onClick={(event) => {
-          if (handleUnsavedChanges(type) === false) {
-            event.preventDefault();
-          }
-          return false;
-        }}
-        target={getTargetByType(type)}
+        {...linkProps({ type, href, id })}
       >
         <span className={icon} />
         <span className="list-group-item-value">{title}</span>

app/javascript/components/top-navbar/help.jsx

@@ -1,9 +1,8 @@
 import React from 'react';
 import { Dropdown, Icon, MenuItem } from 'patternfly-react';
 import PropTypes from 'prop-types';
-import { showAboutModal } from './helpers';
 import { helpMenuProps, recursiveHelpMenuProps } from './recursive-props';
-import getTargetByType from '../../helpers/get-target-by-type';
+import { linkProps } from '../../menu/item-type';
 
 const Help = ({
   helpMenu,
@@ -47,9 +46,7 @@ const Help = ({
               <MenuItem
                 id={`help-menu-${item.title.toLowerCase()}`}
                 key={item.id}
-                href={item.type === 'modal' ? '' : item.href}
-                onClick={e => (item.type === 'modal' ? showAboutModal(e) : !miqCheckForChanges() && e.preventDefault())}
-                target={getTargetByType(item.type)}
+                {...linkProps(item)}
               >
                 {item.title}
               </MenuItem>

app/javascript/components/top-navbar/navbar-header.jsx

@@ -1,7 +1,6 @@
 import React, { useEffect } from 'react';
 import PropTypes from 'prop-types';
 import { useDispatch, useSelector } from 'react-redux';
-import { saveVerticalMenuState } from '../main-menu/helpers';
 import { toggleVerticalMenuCollapsed } from '../../miq-redux/menu-reducer';
 
 const NavbarHeader = ({
@@ -11,7 +10,7 @@ const NavbarHeader = ({
   const dispatch = useDispatch();
   const isVerticalMenuCollapsed = useSelector(({ menuReducer: { isVerticalMenuCollapsed } }) => isVerticalMenuCollapsed);
   useEffect(() => {
-    saveVerticalMenuState(isVerticalMenuCollapsed);
+    window.localStorage.setItem('patternfly-navigation-primary', isVerticalMenuCollapsed ? 'collapsed' : 'expanded');
   }, [isVerticalMenuCollapsed]);
 
   return (

app/javascript/menu/item-type.js

@@ -0,0 +1,35 @@
+// there are 4 menu item types (used both in navbar and menu)
+// * default (href) - opens href
+// * big_iframe (id) - no menu, only navbar and ..a big iframe (external with our header)
+// * modal () - open the About Modal (extend for any modals)
+// * new_window (href) - opens href in new window (for external links)
+
+export const linkProps = ({type, href, id}) => ({
+  href: {
+    big_iframe: `/dashboard/iframe?id=${id}`,
+    default: href,
+    modal: undefined,
+    new_window: href,
+  }[type || 'default'],
+
+  target: (type === 'new_window' ? '_blank' : '_self'),
+
+  onClick: (event) => {
+    if (type === 'modal') {
+      sendDataWithRx({ type: 'showAboutModal' });
+      event.preventDefault();
+      return;
+    }
+
+    if (['default', 'big_iframe'].includes(type) && miqCheckForChanges() === false) {
+      event.preventDefault();
+      return;
+    }
+
+    if (href === '/dashboard/logout') {
+      ManageIQ.logoutInProgress = true;
+    }
+  },
+});
+
+export const itemId = (id, section) => (section ? `menu_section_${id}` : `menu_item_${id}`);

app/javascript/spec/top-navbar/__snapshots__/top-navbar.spec.js.snap

@@ -325,7 +325,6 @@ exports[`Top navbar tests should render correctly 1`] = `
                         disabled={false}
                         divider={false}
                         header={false}
-                        href=""
                         id="help-menu-about"
                         key=".$about"
                         onClick={[Function]}
@@ -339,7 +338,6 @@ exports[`Top navbar tests should render correctly 1`] = `
                         >
                           <SafeAnchor
                             componentClass="a"
-                            href=""
                             id="help-menu-about"
                             onClick={[Function]}
                             onKeyDown={[Function]}

app/presenters/menu/item.rb

@@ -38,17 +38,6 @@ def visible?
       rbac_feature.nil? || ApplicationHelper.role_allows?(rbac_feature)
     end
 
-    def link_params
-      params = case type.try(:to_sym)
-               when :big_iframe then {:href => "/dashboard/iframe?id=#{id}"}
-               when :new_window then {:href => href, :target => '_new'}
-               when :modal      then {:onclick => "sendDataWithRx({type: 'showAboutModal'});", :href => 'javascript:void(0);'}
-               else                  {:href => href}
-               end
-      params[:onclick] = 'return miqCheckForChanges();' unless type.try(:to_sym) == :modal
-      params
-    end
-
     def leaf?
       true
     end

app/presenters/menu/section.rb

@@ -44,12 +44,13 @@ def subsection?
       @subsection ||= Array(items).detect { |el| el.kind_of?(Section) }
     end
 
-    def link_params
-      params = case type
-               when :big_iframe then {:href => "/dashboard/iframe?sid=#{id}"}
-               else                  {:href => "/dashboard/maintab/?tab=#{id}"}
-               end
-      params.merge(:onclick => 'return miqCheckForChanges();')
+    def href
+      case type
+      when :big_iframe
+        "/dashboard/iframe?sid=#{id}"
+      else
+        "/dashboard/maintab/?tab=#{id}"
+      end
     end
 
     def leaf?
@@ -75,7 +76,7 @@ def default_redirect_url
       items.each do |item|
         next unless item.visible?
         if item.kind_of?(Item)
-          return item.link_params[:href]
+          return item.href
         else
           section_result = item.default_redirect_url
           return section_result if section_result