From 63bc73ab175db119a98db915d7489e2e3b2913f6 Mon Sep 17 00:00:00 2001
From: Raman Lally <raman.lally@shopify.com>
Date: Tue, 22 Nov 2022 20:19:26 +0000
Subject: [PATCH] created a wrapper to allow the nav item with its secondary
 actions to highlight uniformly

---
 .changeset/yellow-pillows-visit.md            |  5 +++
 .../src/components/Navigation/Navigation.scss | 25 +++++++----
 .../src/components/Navigation/_variables.scss |  6 ---
 .../Navigation/components/Item/Item.tsx       | 41 +++++++++++--------
 .../Navigation/components/Section/Section.tsx | 24 ++++++-----
 5 files changed, 59 insertions(+), 42 deletions(-)
 create mode 100644 .changeset/yellow-pillows-visit.md

diff --git a/.changeset/yellow-pillows-visit.md b/.changeset/yellow-pillows-visit.md
new file mode 100644
index 00000000000..138591d3013
--- /dev/null
+++ b/.changeset/yellow-pillows-visit.md
@@ -0,0 +1,5 @@
+---
+'@shopify/polaris': patch
+---
+
+Extends Navigation Item highlight to full width to cover secondary actions
diff --git a/polaris-react/src/components/Navigation/Navigation.scss b/polaris-react/src/components/Navigation/Navigation.scss
index fdbe9e10ea7..fbcb22b4e84 100644
--- a/polaris-react/src/components/Navigation/Navigation.scss
+++ b/polaris-react/src/components/Navigation/Navigation.scss
@@ -92,18 +92,11 @@ $disabled-fade: 0.6;
 .Item {
   @include nav-item-attributes;
   position: relative;
-
-  margin-inline-start: var(--p-space-2);
-
-  &:last-child {
-    margin-inline-end: var(--p-space-2);
-  }
 }
 
 .Item-selected {
   font-weight: var(--p-font-weight-semibold);
   color: var(--p-text-primary);
-  background-color: var(--p-background-selected);
   outline: var(--p-border-width-1) solid transparent;
 
   &::before {
@@ -203,9 +196,24 @@ $disabled-fade: 0.6;
 }
 
 .ItemWrapper {
+  width: 100%;
+  padding: 0 var(--p-space-2);
+}
+
+.ItemInnerWrapper {
   display: flex;
   flex-wrap: nowrap;
   width: 100%;
+
+  &:hover {
+    background: var(--p-background-hovered);
+    color: var(--p-text);
+    text-decoration: none;
+  }
+}
+
+.ItemInnerWrapper-Selected {
+  background-color: var(--p-background-selected);
 }
 
 .Text {
@@ -238,8 +246,7 @@ $disabled-fade: 0.6;
   display: flex;
   align-items: center;
   height: nav(mobile-height);
-  padding: var(--p-space-1) var(--p-space-4);
-  margin-right: var(--p-space-1);
+  padding: var(--p-space-1) var(--p-space-3) var(--p-space-1) var(--p-space-4);
   border-radius: var(--p-border-radius-1);
 
   @media #{$p-breakpoints-md-up} {
diff --git a/polaris-react/src/components/Navigation/_variables.scss b/polaris-react/src/components/Navigation/_variables.scss
index be67a815cb1..837458ee22a 100644
--- a/polaris-react/src/components/Navigation/_variables.scss
+++ b/polaris-react/src/components/Navigation/_variables.scss
@@ -34,12 +34,6 @@ $nav-animation-variables: (
   text-decoration: none;
   text-align: left;
 
-  &:hover {
-    background: var(--p-background-hovered);
-    color: var(--p-text);
-    text-decoration: none;
-  }
-
   @include focus-ring;
 
   &.keyFocused {
diff --git a/polaris-react/src/components/Navigation/components/Item/Item.tsx b/polaris-react/src/components/Navigation/components/Item/Item.tsx
index 7d302d62426..4087a2c7df2 100644
--- a/polaris-react/src/components/Navigation/components/Item/Item.tsx
+++ b/polaris-react/src/components/Navigation/components/Item/Item.tsx
@@ -315,25 +315,32 @@ export function Item({
   return (
     <li className={className}>
       <div className={styles.ItemWrapper}>
-        <UnstyledLink
-          url={url}
-          className={itemClassName}
-          external={external}
-          tabIndex={tabIndex}
-          aria-disabled={disabled}
-          aria-label={accessibilityLabel}
-          onClick={getClickHandler(onClick)}
-          onKeyUp={handleKeyUp}
-          onBlur={handleBlur}
-          {...normalizeAriaAttributes(
-            secondaryNavigationId,
-            subNavigationItems.length > 0,
-            showExpanded,
+        <div
+          className={classNames(
+            styles.ItemInnerWrapper,
+            selected && canBeActive && styles['ItemInnerWrapper-Selected'],
           )}
         >
-          {itemContentMarkup}
-        </UnstyledLink>
-        {secondaryActionMarkup}
+          <UnstyledLink
+            url={url}
+            className={itemClassName}
+            external={external}
+            tabIndex={tabIndex}
+            aria-disabled={disabled}
+            aria-label={accessibilityLabel}
+            onClick={getClickHandler(onClick)}
+            onKeyUp={handleKeyUp}
+            onBlur={handleBlur}
+            {...normalizeAriaAttributes(
+              secondaryNavigationId,
+              subNavigationItems.length > 0,
+              showExpanded,
+            )}
+          >
+            {itemContentMarkup}
+          </UnstyledLink>
+          {secondaryActionMarkup}
+        </div>
       </div>
       {secondaryNavigationMarkup}
     </li>
diff --git a/polaris-react/src/components/Navigation/components/Section/Section.tsx b/polaris-react/src/components/Navigation/components/Section/Section.tsx
index 92fe9ef3394..0b8cef2fff8 100644
--- a/polaris-react/src/components/Navigation/components/Section/Section.tsx
+++ b/polaris-react/src/components/Navigation/components/Section/Section.tsx
@@ -139,16 +139,20 @@ export function Section({
 
   const toggleRollup = rollup && items.length > rollup.after && (
     <div className={styles.ListItem} key="List Item">
-      <button
-        type="button"
-        className={toggleClassName}
-        onClick={toggleExpanded}
-        aria-label={ariaLabel}
-      >
-        <span className={styles.Icon}>
-          <Icon source={HorizontalDotsMinor} />
-        </span>
-      </button>
+      <div className={styles.ItemWrapper}>
+        <div className={styles.ItemInnerWrapper}>
+          <button
+            type="button"
+            className={toggleClassName}
+            onClick={toggleExpanded}
+            aria-label={ariaLabel}
+          >
+            <span className={styles.Icon}>
+              <Icon source={HorizontalDotsMinor} />
+            </span>
+          </button>
+        </div>
+      </div>
     </div>
   );