[Navigation.Item] Fix navigation sub item selected indicator when no nav icon

[alexanderMontague]

WHY are these changes introduced?

Fixes #8342

Before:

After:

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 [Navigation] Fix secondary navigation spacing when no icon is present #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
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility
• Updated the component's README.md with documentation changes
• Tophatted documentation changes in the style guide

[github-actions]

size-limit report 📦

Path	Size
polaris-react-cjs	220.13 KB (0%)
polaris-react-esm	139.98 KB (0%)
polaris-react-esnext	195.85 KB (-0.01% 🔽)
polaris-react-css	42.36 KB (-0.01% 🔽)

[alexanderMontague]

/snapit

[kyledurand]

cc @henryyi just want to make sure this doesn't affect any of your nav work

[github-actions]

🫰✨ Thanks @alexanderMontague! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/polaris-cli@0.0.0-snapshot-release-20230214161645

yarn add @shopify/polaris-icons@0.0.0-snapshot-release-20230214161645

yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20230214161645

yarn add @shopify/polaris@0.0.0-snapshot-release-20230214161645

yarn add @shopify/polaris-tokens@0.0.0-snapshot-release-20230214161645

yarn add @shopify/stylelint-polaris@0.0.0-snapshot-release-20230214161645

[sophschneider]

Thanks for fixing this!

[henryyi]

Not sure if intended but the change causes the sub-nav indentation to be wider than it is currently:
in prod:

in spin:

[alexanderMontague]

@henryyi how did you come across this? Unless I'm missing something it seems to be the same:

In Spin/storybook:

(40/4 padding)

---

In prod/web:
(40/4 padding)

[henryyi]

@henryyi how did you come across this? Unless I'm missing something it seems to be the same:

This happens in a store with the global-nav. You can check this test store: https://henry-dev2.myshopify.com/admin
Also in this sandbox with latest polaris: https://codesandbox.io/s/divine-hill-2w33ru

[alexanderMontague]

@henryyi you are correct, this seems to only be happening for the plus side nav there. This brings into question (since I've never actually seen this nav view before), the highlight bug is actually in effect in this nav as you can see here:

The sub item spacing has not been consistent with the rest of Polaris Icon sub nav items. So this actually makes them consistent with that, but should this be the case? I wouldn't mind some design input on this.

Basically, should the sub nav items all have the same left indent regardless of if the parent has an icon or not, or should there be less padding like it currently is on that plus nav right now

[henryyi]

@dustinmalik do you mind having a look to determine what the preferred indentation is when we are not using icons (e.g when global-nav is visible).

[alexanderMontague]

For more context, I was able to boot this view up in spin with the change. This is what it looks like on this view:

It does look a little too justified without the icon, but this is the same indent as web

[alexanderMontague]

Any updates @henryyi @dustinmalik ?

[henryyi]

Any updates @henryyi @dustinmalik ?

I'll wait for Dustin's input but my gut feeling is that we'll want to keep the lesser spacing since that is what merchants currently see despite the selection bug.

[dustinmalik]

Tried finding old Figma files for this but couldn't find anything. I would keep the sub nav items flush with the rest of the nav items like when there is an icon.

@alexblaise was the designer on this. Thoughts?

[dustinmalik]

Discussed with @chrisblinstrub. Let's go with 24px padding. Thanks!

[alexanderMontague]

/snapit

[github-actions]

🫰✨ Thanks @alexanderMontague! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/polaris-cli@0.0.0-snapshot-release-20230216174113

yarn add @shopify/polaris-icons@0.0.0-snapshot-release-20230216174113

yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20230216174113

yarn add @shopify/polaris@0.0.0-snapshot-release-20230216174113

yarn add @shopify/polaris-tokens@0.0.0-snapshot-release-20230216174113

yarn add @shopify/stylelint-polaris@0.0.0-snapshot-release-20230216174113

[alexanderMontague]

Thanks for the follow up everyone, appreciate it!! @henryyi ready for review again:

Storybook:

http://240.26.37.19:6006/?path=/story/playground--nav-1

Admin:

https://admin.web.web-nths.alex-montague.us.spin.dev/store/shop2/draft_orders?selectedView=all

Plus admin nav:

https://admin.web.plus-home-k10h.alex-montague.us.spin.dev/store/shop5/draft_orders

[henryyi]

Looks good!