Skip to content

Commit 824aef0

Browse files
authored
[Navigation] Allow the NavItem to highlight on hover/action uniformly (#7793)
<!-- ☝️How to write a good PR title: - Prefix it with [ComponentName] (if applicable), for example: [Button] - Start with a verb, for example: Add, Delete, Improve, Fix… - Give as much context as necessary and as little as possible - Prefix it with [WIP] while it’s a work in progress --> ### WHY are these changes introduced? As part of an effort to port bug fixes and style changes over from admin I am adding this highlight change into polaris. Fixes: Shopify/core-workflows#633 <!-- Context about the problem that’s being addressed. --> ### WHAT is this pull request doing? I have added a wrapper around the Nav Item contents to allow the nav item to extend its highlighting over to the secondary actions as well. This was part of UX changes made to the navigation in Admin during the pinning work. This applies for both hover and active state highlights, the secondary action will now appear as if its part of the navigation items hover instead of being separate, the focus rings/tab order is maintained. <!-- Summary of the changes committed. Before / after screenshots are appreciated for UI changes. Make sure to include alt text that describes the screenshot. If you include an animated gif showing your change, wrapping it in a details tag is recommended. Gifs usually autoplay, which can cause accessibility issues for people reviewing your PR: <details> <summary>Summary of your gif(s)</summary> <img src="..." alt="Description of what the gif shows"> </details> --> Before: <details> <summary>video of original interactions</summary> https://user-images.githubusercontent.com/33904740/203415160-147a03a0-dbf3-41ca-b9d6-0164aec33ea0.mov </details> Active and Active hover highlighting: <img width="290" alt="Original 'various states and sections' showing that the secondary action highlight when hovered and active is separate from the text in the navItem'" src="https://user-images.githubusercontent.com/33904740/203415460-169d648c-24f5-492d-ba7c-ad55719ee076.png"> Hover Highlighting: <img width="267" alt="Original NavItem hover highlight when not active showing the secondary item not highlighting on hover" src="https://user-images.githubusercontent.com/33904740/203415621-d5a329bb-cfad-4cc1-84af-ccbcd018aeb9.png"> After: <details> <summary>Video of new interactions and how the highlights appear uniform now</summary> https://user-images.githubusercontent.com/33904740/203418162-5c84130d-7b7a-4975-b826-f612b3a2750c.mov </details> Active and Active hover highlighting: <img width="280" alt="New 'various states and sections' showcasing that the highlight is now uniform across the text and secondary action" src="https://user-images.githubusercontent.com/33904740/203415977-ce63bd43-3649-420f-8565-a8a8ac483c7e.png"> Hover Highlighting (notice the highlight ends just before the secondary action): <img width="262" alt="New NavItem hover highlight when not active showing that the highlight is also uniform then" src="https://user-images.githubusercontent.com/33904740/203416068-cc1e4d60-ee45-431e-8f87-b6d061ad81d0.png"> Here we can see that the highlight now is uniform across the text of the nav item and the secondary action, similarly to how we have in the admin nav today. One thing you will notice is that the secondary action spacing is slightly different than before, it is now identical to the left hand spacing of the nav item. This is mainly so that the different highlights can be uniform in the nav now, so they're all aligned. Its a bit difficult to see but the secondary action highlight juts out past the non-secondary action items, so now they are the same width. <img width="293" alt="showcasing-secondaryactionhover-misalignment" src="https://user-images.githubusercontent.com/33904740/203416721-6cf0678b-c949-464d-9500-c54bb9c8d289.png"> ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog) <!-- Give as much information as needed to experiment with the component in the playground. --> <details> <summary>Copy-paste this code in <code>playground/Playground.tsx</code>:</summary> ```jsx import React from 'react'; import {Page} from '../src'; export function Playground() { return ( <Page title="Playground"> {/* Add the code you want to test in here */} </Page> ); } ``` </details> ### 🎩 checklist - [ ] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [x] Tested on [multiple browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers) - [x] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [ ] Updated the component's `README.md` with documentation changes - [ ] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide
1 parent 9d69c90 commit 824aef0

File tree

5 files changed

+59
-42
lines changed

5 files changed

+59
-42
lines changed

.changeset/yellow-pillows-visit.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/polaris': patch
3+
---
4+
5+
Extends Navigation Item highlight to full width to cover secondary actions

polaris-react/src/components/Navigation/Navigation.scss

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -92,18 +92,11 @@ $disabled-fade: 0.6;
9292
.Item {
9393
@include nav-item-attributes;
9494
position: relative;
95-
96-
margin-inline-start: var(--p-space-2);
97-
98-
&:last-child {
99-
margin-inline-end: var(--p-space-2);
100-
}
10195
}
10296

10397
.Item-selected {
10498
font-weight: var(--p-font-weight-semibold);
10599
color: var(--p-text-primary);
106-
background-color: var(--p-background-selected);
107100
outline: var(--p-border-width-1) solid transparent;
108101

109102
&::before {
@@ -203,9 +196,24 @@ $disabled-fade: 0.6;
203196
}
204197

205198
.ItemWrapper {
199+
width: 100%;
200+
padding: 0 var(--p-space-2);
201+
}
202+
203+
.ItemInnerWrapper {
206204
display: flex;
207205
flex-wrap: nowrap;
208206
width: 100%;
207+
208+
&:hover {
209+
background: var(--p-background-hovered);
210+
color: var(--p-text);
211+
text-decoration: none;
212+
}
213+
}
214+
215+
.ItemInnerWrapper-Selected {
216+
background-color: var(--p-background-selected);
209217
}
210218

211219
.Text {
@@ -238,8 +246,7 @@ $disabled-fade: 0.6;
238246
display: flex;
239247
align-items: center;
240248
height: nav(mobile-height);
241-
padding: var(--p-space-1) var(--p-space-4);
242-
margin-right: var(--p-space-1);
249+
padding: var(--p-space-1) var(--p-space-3) var(--p-space-1) var(--p-space-4);
243250
border-radius: var(--p-border-radius-1);
244251

245252
@media #{$p-breakpoints-md-up} {

polaris-react/src/components/Navigation/_variables.scss

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,6 @@ $nav-animation-variables: (
3434
text-decoration: none;
3535
text-align: left;
3636

37-
&:hover {
38-
background: var(--p-background-hovered);
39-
color: var(--p-text);
40-
text-decoration: none;
41-
}
42-
4337
@include focus-ring;
4438

4539
&.keyFocused {

polaris-react/src/components/Navigation/components/Item/Item.tsx

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -315,25 +315,32 @@ export function Item({
315315
return (
316316
<li className={className}>
317317
<div className={styles.ItemWrapper}>
318-
<UnstyledLink
319-
url={url}
320-
className={itemClassName}
321-
external={external}
322-
tabIndex={tabIndex}
323-
aria-disabled={disabled}
324-
aria-label={accessibilityLabel}
325-
onClick={getClickHandler(onClick)}
326-
onKeyUp={handleKeyUp}
327-
onBlur={handleBlur}
328-
{...normalizeAriaAttributes(
329-
secondaryNavigationId,
330-
subNavigationItems.length > 0,
331-
showExpanded,
318+
<div
319+
className={classNames(
320+
styles.ItemInnerWrapper,
321+
selected && canBeActive && styles['ItemInnerWrapper-Selected'],
332322
)}
333323
>
334-
{itemContentMarkup}
335-
</UnstyledLink>
336-
{secondaryActionMarkup}
324+
<UnstyledLink
325+
url={url}
326+
className={itemClassName}
327+
external={external}
328+
tabIndex={tabIndex}
329+
aria-disabled={disabled}
330+
aria-label={accessibilityLabel}
331+
onClick={getClickHandler(onClick)}
332+
onKeyUp={handleKeyUp}
333+
onBlur={handleBlur}
334+
{...normalizeAriaAttributes(
335+
secondaryNavigationId,
336+
subNavigationItems.length > 0,
337+
showExpanded,
338+
)}
339+
>
340+
{itemContentMarkup}
341+
</UnstyledLink>
342+
{secondaryActionMarkup}
343+
</div>
337344
</div>
338345
{secondaryNavigationMarkup}
339346
</li>

polaris-react/src/components/Navigation/components/Section/Section.tsx

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -139,16 +139,20 @@ export function Section({
139139

140140
const toggleRollup = rollup && items.length > rollup.after && (
141141
<div className={styles.ListItem} key="List Item">
142-
<button
143-
type="button"
144-
className={toggleClassName}
145-
onClick={toggleExpanded}
146-
aria-label={ariaLabel}
147-
>
148-
<span className={styles.Icon}>
149-
<Icon source={HorizontalDotsMinor} />
150-
</span>
151-
</button>
142+
<div className={styles.ItemWrapper}>
143+
<div className={styles.ItemInnerWrapper}>
144+
<button
145+
type="button"
146+
className={toggleClassName}
147+
onClick={toggleExpanded}
148+
aria-label={ariaLabel}
149+
>
150+
<span className={styles.Icon}>
151+
<Icon source={HorizontalDotsMinor} />
152+
</span>
153+
</button>
154+
</div>
155+
</div>
152156
</div>
153157
);
154158

0 commit comments

Comments
 (0)