[Navigation] support multiple secondary actions for Navigation Item #7817

henryyi · 2022-11-30T20:45:47Z

WHY are these changes introduced?

Resolves Shopify/core-workflows/issues/559

The Navigation item for apps and channels were adapted to allow for a "pin" button. In order to support this natively we should allow for multiple secondaryActions. Since there is not a lot of space we should start with an initial limit of 2 actions. There is a separate task to allow the secondary actions to "float" above the text along with tighter margins which could allow for additional actions.
If there is a desire for more actions they can potentially overflow into a popover in the future but currently there doesn't seem to be a lot of usage.

WHAT is this pull request doing?

This PR deprecates secondaryAction in favour of secondaryActions so that more than one action is supported. The actions are place in one row after the badge. The actions are capped to 2 for now.
The icons have tighter spacing on desktop to provide more room for the label and badge but retains the 40x40px touch size on mobile.

The outcome of these changes should have no impact on the current state and appearance of the admin navigation while it will allow to address some accessibility concerns where app pinning was implemented.

Desktop

Mobile

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Open Storybook
Check Navigation -> With Multiple Secondary Actions...

🎩 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 · 2022-11-30T20:50:03Z

size-limit report 📦

Path	Size
polaris-react-cjs	211.22 KB (+0.06% 🔺)
polaris-react-esm	136.42 KB (+0.08% 🔺)
polaris-react-esnext	192.26 KB (+0.05% 🔺)
polaris-react-css	42.12 KB (-0.01% 🔽)

henryyi · 2022-12-01T13:11:29Z

/snapit

github-actions · 2022-12-01T13:15:23Z

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

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

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20221201131343

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

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

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

henryyi · 2022-12-01T15:34:12Z

I've tightened the spacing a bit on desktop.

henryyi · 2022-12-01T15:34:41Z

/snapit

github-actions · 2022-12-01T15:38:49Z

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

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

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20221201153701

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

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

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

Rmnlly

I think the slimmer spacing on the button is now out of line with the badges alignment we did recently.

henryyi · 2022-12-01T16:13:34Z

I think the slimmer spacing on the button is now out of line with the badges alignment we did recently.

I adjusted the spacing to work better with the badges now:

henryyi · 2022-12-01T16:14:13Z

/snapit

github-actions · 2022-12-01T16:18:21Z

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

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

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20221201161632

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

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

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

henryyi · 2022-12-01T16:57:12Z

I've made SecondaryAction url optional since there is a case where we only want the onClick handler (for app pinning). It will now render UnstyledButton if url is not supplied.

henryyi · 2022-12-01T18:46:21Z

/snapit

github-actions · 2022-12-01T18:50:29Z

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

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

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20221201184844

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

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

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

alex-page

I would prefer if we made the item contents React so that users can put whatever they need instead of adding additional props.

Rmnlly

🎩 Looks good!

nit: Only concern is the spacing when there's only 1 item is kinda tight

henryyi · 2022-12-06T18:59:00Z

/snapit

github-actions · 2022-12-06T19:04:19Z

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

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

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20221206190151

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

henryyi · 2022-12-07T13:58:33Z

I would prefer if we made the item contents React so that users can put whatever they need instead of adding additional props.

I'll give it some thought but we might lose some conformity. We could potentially create and expose subcomponents like <SecondaryAction> that would allow the consumer to construct the Nav item in parts.

henryyi · 2022-12-07T14:01:41Z

This also address a highlighting issue found in the latest release:

henryyi · 2022-12-07T14:37:48Z

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

  @include nav-item-icon-attributes;
 }

-.ExternalIcon {


external was removed in this PR so this is no longer needed.

henryyi · 2022-12-07T14:50:57Z

/snapit

github-actions · 2022-12-07T14:55:12Z

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

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

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20221207145313

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

stanley-xu

🎩 LGTM

stanley-xu · 2022-12-07T17:38:18Z

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

+  const actions = secondaryActions || (secondaryAction && [secondaryAction]);
+
+  if (actions && actions.length > MAX_SECONDARY_ACTIONS) {
+    actions.length = MAX_SECONDARY_ACTIONS;


oh nice! This seems more efficient than slicing out everything after the 2nd item

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

Rmnlly

Updates LGTM

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

chloerice

Just need to add the example and prop documentation for this to the website as well by adding a file here

aveline · 2022-12-14T16:59:17Z

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

-  );
+  if (secondaryAction && process.env.NODE_ENV === 'development') {
+    // eslint-disable-next-line no-console
+    console.warn(


If you are able to replace the deprecated prop usage in web that would be super, to avoid unnecessary noise for devs

Yup I'll be sure to do that post update in web.

@henryyi

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/polaris@10.19.0 ### Minor Changes - [#7817](#7817) [`6c310101d`](6c31010) Thanks [@henryyi](https://github.com/henryyi)! - Added `secondaryActions` prop in Navigation.Item to support up to two actions ### Patch Changes - [#8018](#8018) [`2620b0a20`](2620b0a) Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed modal scroll bug ## @shopify/stylelint-polaris@5.1.0 ### Minor Changes - [#7993](#7993) [`128f147d1`](128f147) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Created `polaris/declaration-property-value-disallowed-list` rule to ignore failures in `@font-face` at-rules ### Patch Changes - [#8006](#8006) [`6404b1638`](6404b16) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Temporarily disabled `border-radius-base` error ## @shopify/plugin-polaris@0.0.26 ### Patch Changes - Updated dependencies \[[`fad3809ef`](fad3809), [`56b0bc2dc`](56b0bc2)]: - @shopify/polaris-migrator@0.10.3 ## @shopify/polaris-migrator@0.10.3 ### Patch Changes - [#7997](#7997) [`fad3809ef`](fad3809) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated `styles-tokenize-font` to ignore `@font-face` at-rules - [#7783](#7783) [`56b0bc2dc`](56b0bc2) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Improve robustness of the `react-replace-text-components` migration - Updated dependencies \[[`6404b1638`](6404b16), [`128f147d1`](128f147)]: - @shopify/stylelint-polaris@5.1.0 ## polaris.shopify.com@0.28.3 ### Patch Changes - Updated dependencies \[[`2620b0a20`](2620b0a), [`6c310101d`](6c31010)]: - @shopify/polaris@10.19.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…hopify#7817) ### WHY are these changes introduced? Resolves Shopify/core-workflows/issues/559 The Navigation item for apps and channels were adapted to allow for a "pin" button. In order to support this natively we should allow for multiple `secondaryActions`. Since there is not a lot of space we should start with an initial limit of 2 actions. There is a [separate task](https://github.com/Shopify/core-workflows/issues/634) to allow the secondary actions to "float" above the text along with tighter margins which could allow for additional actions. If there is a desire for more actions they can potentially overflow into a popover in the future but currently there doesn't seem to be a lot of usage. ### WHAT is this pull request doing? This PR deprecates `secondaryAction` in favour of `secondaryActions` so that more than one action is supported. The actions are place in one row after the badge. The actions are capped to 2 for now. The icons have tighter spacing on desktop to provide more room for the label and badge but retains the 40x40px touch size on mobile. The outcome of these changes should have no impact on the current state and appearance of the admin navigation while it will allow to address some accessibility concerns where app pinning was implemented. Desktop <img width="300" src="https://user-images.githubusercontent.com/1307190/205093459-e54835b1-2211-4ec3-86ce-0b04b7aaf9c8.gif"> Mobile <img width="300" src="https://user-images.githubusercontent.com/1307190/205093515-a956c8d5-e096-4d8d-94a5-12a031cacd75.gif">   ### 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) 1. Open [Storybook](https://polaris.pnav.jita-yi.us.spin.dev/?path=/story/all-components-navigation--with-multiple-secondary-actions-for-an-item) 2. Check `Navigation` -> `With Multiple Secondary Actions...`  ### 🎩 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) - [x] 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

@henryyi

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/polaris@10.19.0 ### Minor Changes - [Shopify#7817](Shopify#7817) [`6c310101d`](Shopify@6c31010) Thanks [@henryyi](https://github.com/henryyi)! - Added `secondaryActions` prop in Navigation.Item to support up to two actions ### Patch Changes - [Shopify#8018](Shopify#8018) [`2620b0a20`](Shopify@2620b0a) Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed modal scroll bug ## @shopify/stylelint-polaris@5.1.0 ### Minor Changes - [Shopify#7993](Shopify#7993) [`128f147d1`](Shopify@128f147) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Created `polaris/declaration-property-value-disallowed-list` rule to ignore failures in `@font-face` at-rules ### Patch Changes - [Shopify#8006](Shopify#8006) [`6404b1638`](Shopify@6404b16) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Temporarily disabled `border-radius-base` error ## @shopify/plugin-polaris@0.0.26 ### Patch Changes - Updated dependencies \[[`fad3809ef`](Shopify@fad3809), [`56b0bc2dc`](Shopify@56b0bc2)]: - @shopify/polaris-migrator@0.10.3 ## @shopify/polaris-migrator@0.10.3 ### Patch Changes - [Shopify#7997](Shopify#7997) [`fad3809ef`](Shopify@fad3809) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated `styles-tokenize-font` to ignore `@font-face` at-rules - [Shopify#7783](Shopify#7783) [`56b0bc2dc`](Shopify@56b0bc2) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Improve robustness of the `react-replace-text-components` migration - Updated dependencies \[[`6404b1638`](Shopify@6404b16), [`128f147d1`](Shopify@128f147)]: - @shopify/stylelint-polaris@5.1.0 ## polaris.shopify.com@0.28.3 ### Patch Changes - Updated dependencies \[[`2620b0a20`](Shopify@2620b0a), [`6c310101d`](Shopify@6c31010)]: - @shopify/polaris@10.19.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

henryyi requested review from kyledurand, alex-page and Rmnlly November 30, 2022 22:01

henryyi changed the title ~~[Navigation] permit multiple secondary actions for Navigation Item~~ [Navigation] support multiple secondary actions for Navigation Item Dec 1, 2022

henryyi force-pushed the navigation/multi-secondary-actions branch 2 times, most recently from 82b5dc8 to 1e05208 Compare December 1, 2022 15:33

Rmnlly reviewed Dec 1, 2022

View reviewed changes

henryyi force-pushed the navigation/multi-secondary-actions branch from 1e05208 to dfcfc77 Compare December 1, 2022 16:12

henryyi force-pushed the navigation/multi-secondary-actions branch from dfcfc77 to 353b127 Compare December 1, 2022 16:51

alex-page approved these changes Dec 1, 2022

View reviewed changes

Rmnlly approved these changes Dec 1, 2022

View reviewed changes

henryyi force-pushed the navigation/multi-secondary-actions branch from 353b127 to c6286fc Compare December 6, 2022 18:58

henryyi force-pushed the navigation/multi-secondary-actions branch from c6286fc to ae4695e Compare December 7, 2022 13:38

henryyi force-pushed the navigation/multi-secondary-actions branch from ae4695e to f425c7e Compare December 7, 2022 14:01

henryyi force-pushed the navigation/multi-secondary-actions branch from f425c7e to 1140565 Compare December 7, 2022 14:21

henryyi commented Dec 7, 2022

View reviewed changes

henryyi requested review from Rmnlly and alex-page December 7, 2022 14:47

stanley-xu approved these changes Dec 7, 2022

View reviewed changes

stanley-xu reviewed Dec 7, 2022

View reviewed changes

polaris-react/src/components/Navigation/components/Item/tests/Item.test.tsx Show resolved Hide resolved

Rmnlly approved these changes Dec 9, 2022

View reviewed changes

aveline reviewed Dec 9, 2022

View reviewed changes

polaris-react/src/components/Navigation/Navigation.scss Outdated Show resolved Hide resolved

aveline reviewed Dec 9, 2022

View reviewed changes

polaris-react/src/components/Navigation/Navigation.scss Outdated Show resolved Hide resolved

henryyi force-pushed the navigation/multi-secondary-actions branch from 1140565 to 4f8ddbb Compare December 13, 2022 16:46

[Navigation] permit multiple secondary actions for Navigation Item

26815e4

henryyi force-pushed the navigation/multi-secondary-actions branch from 4f8ddbb to 26815e4 Compare December 13, 2022 17:02

henryyi requested a review from aveline December 13, 2022 17:25

chloerice approved these changes Dec 14, 2022

View reviewed changes

aveline approved these changes Dec 14, 2022

View reviewed changes

henryyi merged commit 6c31010 into main Jan 11, 2023

henryyi deleted the navigation/multi-secondary-actions branch January 11, 2023 14:23

shopify-github-actions-access bot mentioned this pull request Jan 11, 2023

Version Packages #7998

Merged

gwyneplaine mentioned this pull request Feb 14, 2023

8248/fix toc layout #8338

Merged

[Navigation] support multiple secondary actions for Navigation Item #7817

[Navigation] support multiple secondary actions for Navigation Item #7817

Uh oh!

Conversation

henryyi commented Nov 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

How to 🎩

🎩 checklist

Uh oh!

github-actions bot commented Nov 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

henryyi commented Dec 1, 2022

Uh oh!

github-actions bot commented Dec 1, 2022

Uh oh!

henryyi commented Dec 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

henryyi commented Dec 1, 2022

Uh oh!

github-actions bot commented Dec 1, 2022

Uh oh!

Rmnlly left a comment

Choose a reason for hiding this comment

Uh oh!

henryyi commented Dec 1, 2022

Uh oh!

henryyi commented Dec 1, 2022

Uh oh!

github-actions bot commented Dec 1, 2022

Uh oh!

henryyi commented Dec 1, 2022

Uh oh!

henryyi commented Dec 1, 2022

Uh oh!

github-actions bot commented Dec 1, 2022

Uh oh!

alex-page left a comment

Choose a reason for hiding this comment

Uh oh!

Rmnlly left a comment

Choose a reason for hiding this comment

Uh oh!

henryyi commented Dec 6, 2022

Uh oh!

github-actions bot commented Dec 6, 2022

Uh oh!

henryyi commented Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

henryyi commented Dec 7, 2022

Uh oh!

henryyi Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

henryyi commented Dec 7, 2022

Uh oh!

github-actions bot commented Dec 7, 2022

Uh oh!

stanley-xu left a comment

Choose a reason for hiding this comment

Uh oh!

stanley-xu Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Rmnlly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chloerice left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aveline Dec 14, 2022

henryyi commented Nov 30, 2022 •

edited

Loading

github-actions bot commented Nov 30, 2022 •

edited

Loading

henryyi commented Dec 1, 2022 •

edited

Loading

henryyi commented Dec 7, 2022 •

edited

Loading

chloerice left a comment •

edited

Loading