Skip to content

Commit

Permalink
[Navigation.Item] Fix navigation sub item selected indicator when n…
Browse files Browse the repository at this point in the history
…o nav icon (#8343)

<!--
  ☝️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?

Fixes #8342

Before:

<img width="257" alt="Screenshot 2023-02-14 at 10 32 05 AM"
src="https://user-images.githubusercontent.com/29992628/218789163-ea365e03-732b-4592-9aab-5945377626ae.png">

<img width="262" alt="Screenshot 2023-02-14 at 10 32 16 AM"
src="https://user-images.githubusercontent.com/29992628/218789445-7ff2875d-48ba-4bd2-bb14-27f2be1749d2.png">

After:

<img width="326" alt="image"
src="https://user-images.githubusercontent.com/29992628/218789292-fa10a152-7060-4576-a4ad-b39c941c89f1.png">

<img width="333" alt="image"
src="https://user-images.githubusercontent.com/29992628/218789515-99038b3f-a2a4-4edf-91db-707bd429ad48.png">



### WHAT is this pull request doing?

- While building a nav in Partners I stumbled across this weird
behaviour, more details in the linked issue above
- TLDR: when a nav item does not have an icon and there are sub items,
if one of those sub items are selected the selector indicator (green
indicator on left of item) does not fully hug the leftmost margin
- This was due to legacy no icon styles that were introduced a while ago
- See this PR that added in these styles
#2874

### How to 🎩

- head to my spin
http://240.26.37.19:6006/?path=/story/playground--nav-1
- toggle between `nav-1` and `nav-2` which are the navs with an icon and
without
- they should now have the same spacing and the selector should fully
hug the left margin

🖥 [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)



### 🎩 checklist

- [x] 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
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
  • Loading branch information
alexanderMontague committed Feb 17, 2023
1 parent c66f498 commit 20d17c6
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/new-dragons-raise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Fix margin bug with sub nav item selected highlight styles
8 changes: 3 additions & 5 deletions polaris-react/src/components/Navigation/Navigation.scss
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,9 @@ $disabled-fade: 0.6;
}

.SecondaryNavigation-noIcon {
margin-left: var(--p-space-4);
.Item {
padding-left: var(--p-space-6);
}
}

// Section styles
Expand Down Expand Up @@ -679,7 +681,3 @@ $disabled-fade: 0.6;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
width: 10px;
}

.SecondaryNavigation-noIcon .Item {
padding-left: var(--p-space-3);
}

0 comments on commit 20d17c6

Please sign in to comment.