From 268decfc7f79daa453306b05c87d110ded322215 Mon Sep 17 00:00:00 2001
From: Hannele Valtanen <hannele.valtanen@iki.fi>
Date: Mon, 17 Oct 2022 17:50:25 +0300
Subject: [PATCH] fix(HeaderMenu): fix HeaderMenu with isCurrentPage to have a
 pointer cursor (#10109) (#12265)

* fix(HeaderMenu): menu-item--current instead of submenu--current (#10109)

* fix(HeaderMenu): add isCurrentPage for HeaderMenu story

* fix(HeaderMenu): update tests for HeaderMenu to use menu-item--current

* fix(HeaderMenu): remove duplicated styles for current page HeaderMenu
---
 .../src/components/UIShell/HeaderMenu.js      | 24 +++++++++++++++----
 .../src/components/UIShell/UIShell.stories.js |  2 +-
 .../UIShell/__tests__/HeaderMenu-test.js      | 12 +++++++---
 .../components/ui-shell/header/_header.scss   | 19 ---------------
 4 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/packages/react/src/components/UIShell/HeaderMenu.js b/packages/react/src/components/UIShell/HeaderMenu.js
index 6a9edb49dba9..1e06d8aa1c2d 100644
--- a/packages/react/src/components/UIShell/HeaderMenu.js
+++ b/packages/react/src/components/UIShell/HeaderMenu.js
@@ -26,11 +26,21 @@ class HeaderMenu extends React.Component {
      */
     ...AriaLabelPropType,
 
+    /**
+     * Optionally provide a custom class to apply to the underlying `<li>` node
+     */
+    className: PropTypes.string,
+
     /**
      * Provide a custom ref handler for the menu button
      */
     focusRef: PropTypes.func,
 
+    /**
+     * Applies selected styles to the item if a user sets this to true and aria-current !== 'page'.
+     */
+    isCurrentPage: PropTypes.bool,
+
     /**
      * Provide a label for the link text
      */
@@ -175,11 +185,17 @@ class HeaderMenu extends React.Component {
       'aria-label': ariaLabel,
       'aria-labelledby': ariaLabelledBy,
     };
-    const className = cx({
+    const itemClassName = cx({
       [`${prefix}--header__submenu`]: true,
-      [`${prefix}--header__submenu--current`]: isCurrentPage,
       [customClassName]: !!customClassName,
     });
+    const linkClassName = cx({
+      [`${prefix}--header__menu-item`]: true,
+      [`${prefix}--header__menu-title`]: true,
+      // We set the current class only if `isCurrentPage` is passed in and we do
+      // not have an `aria-current="page"` set for the breadcrumb item
+      [`${prefix}--header__menu-item--current`]: isCurrentPage,
+    });
 
     // Notes on eslint comments and based on the examples in:
     // https://www.w3.org/TR/wai-aria-practices/examples/menubar/menubar-1/menubar-1.html#
@@ -191,14 +207,14 @@ class HeaderMenu extends React.Component {
     return (
       <li // eslint-disable-line jsx-a11y/mouse-events-have-key-events,jsx-a11y/no-noninteractive-element-interactions
         {...rest}
-        className={className}
+        className={itemClassName}
         onKeyDown={this.handleMenuClose}
         onClick={this.handleOnClick}
         onBlur={this.handleOnBlur}>
         <a // eslint-disable-line jsx-a11y/role-supports-aria-props,jsx-a11y/anchor-is-valid
           aria-haspopup="menu" // eslint-disable-line jsx-a11y/aria-proptypes
           aria-expanded={this.state.expanded}
-          className={`${prefix}--header__menu-item ${prefix}--header__menu-title`}
+          className={linkClassName}
           href="#"
           onKeyDown={this.handleOnKeyDown}
           ref={this.handleMenuButtonRef}
diff --git a/packages/react/src/components/UIShell/UIShell.stories.js b/packages/react/src/components/UIShell/UIShell.stories.js
index 1dda7bf0a942..0899185c5bf2 100644
--- a/packages/react/src/components/UIShell/UIShell.stories.js
+++ b/packages/react/src/components/UIShell/UIShell.stories.js
@@ -302,7 +302,7 @@ export const HeaderBaseWNavigationAndActions = () => (
           <HeaderMenuItem href="#">Link 1</HeaderMenuItem>
           <HeaderMenuItem href="#">Link 2</HeaderMenuItem>
           <HeaderMenuItem href="#">Link 3</HeaderMenuItem>
-          <HeaderMenu aria-label="Link 4" menuLinkName="Link 4">
+          <HeaderMenu isCurrentPage aria-label="Link 4" menuLinkName="Link 4">
             <HeaderMenuItem href="#">Sub-link 1</HeaderMenuItem>
             <HeaderMenuItem href="#">Sub-link 2</HeaderMenuItem>
             <HeaderMenuItem href="#">Sub-link 3</HeaderMenuItem>
diff --git a/packages/react/src/components/UIShell/__tests__/HeaderMenu-test.js b/packages/react/src/components/UIShell/__tests__/HeaderMenu-test.js
index f8fbef825f8b..42146084c7d4 100644
--- a/packages/react/src/components/UIShell/__tests__/HeaderMenu-test.js
+++ b/packages/react/src/components/UIShell/__tests__/HeaderMenu-test.js
@@ -11,14 +11,20 @@ import { HeaderMenu, HeaderMenuItem } from '../';
 
 describe('HeaderMenu', () => {
   it('should set the current class if `isCurrentPage` is true', () => {
-    const { container } = render(
-      <HeaderMenu aria-label="test" menuLinkName="test" isCurrentPage>
+    render(
+      <HeaderMenu
+        data-testid="test"
+        aria-label="test"
+        menuLinkName="test"
+        isCurrentPage>
         <HeaderMenuItem href="/a">a</HeaderMenuItem>
         <HeaderMenuItem href="/b">b</HeaderMenuItem>
         <HeaderMenuItem href="/c">c</HeaderMenuItem>
       </HeaderMenu>
     );
-    expect(container.firstChild).toHaveClass('cds--header__submenu--current');
+    expect(screen.getByTestId('test').firstChild).toHaveClass(
+      'cds--header__menu-item--current'
+    );
   });
 
   it('should support a custom `className` prop on the outermost element', () => {
diff --git a/packages/styles/scss/components/ui-shell/header/_header.scss b/packages/styles/scss/components/ui-shell/header/_header.scss
index 52163cf07e12..7ba940177a07 100644
--- a/packages/styles/scss/components/ui-shell/header/_header.scss
+++ b/packages/styles/scss/components/ui-shell/header/_header.scss
@@ -277,25 +277,6 @@
     position: relative;
   }
 
-  .#{$prefix}--header__submenu--current::after {
-    position: absolute;
-    top: 0;
-    right: 0;
-    bottom: 0;
-    left: 0;
-    width: 100%;
-    border-bottom: 3px solid $border-interactive;
-    content: '';
-  }
-
-  .#{$prefix}--header__submenu--current:focus {
-    border: 2px solid $focus;
-  }
-
-  .#{$prefix}--header__submenu--current:focus::after {
-    border: 0;
-  }
-
   .#{$prefix}--header__menu-title[aria-haspopup='true'] {
     position: relative;
   }