From 323eafd0b418d74c12634fe60f5d644b0a63597f Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Fri, 7 Jan 2022 09:36:01 +0000 Subject: [PATCH 1/4] chore: tighten nav density on large screens --- UNRELEASED.md | 2 + src/components/Navigation/Navigation.scss | 37 ++++++++++++++++--- src/components/Navigation/_variables.scss | 29 ++++++++++++--- .../Navigation/components/Item/Item.tsx | 9 ++++- 4 files changed, 64 insertions(+), 13 deletions(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index f6a0a8f19b5..1436da53ca6 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -8,6 +8,8 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t ### Enhancements +- Tightened up the Navigation component UI density. ([#4874](https://github.com/Shopify/polaris-react/pull/4874)) + ### Bug fixes - Fixed `segmented` `ButtonGroup` misaligning icon only buttons when grouped with text only buttons ([#4079](https://github.com/Shopify/polaris-react/issues/4079)) diff --git a/src/components/Navigation/Navigation.scss b/src/components/Navigation/Navigation.scss index 968cb9da763..c9e9460dd9c 100644 --- a/src/components/Navigation/Navigation.scss +++ b/src/components/Navigation/Navigation.scss @@ -55,7 +55,7 @@ $nav-max-width: rem(360px); } @include breakpoint-after(nav-min-window-corrected()) { - padding-top: rem(12px); + padding-top: rem(16px); } } @@ -93,10 +93,9 @@ $disabled-fade: 0.6; &::before { content: ''; position: absolute; - top: 0; - bottom: 0; + top: rem(1px); + bottom: rem(1px); left: -1 * spacing(tight); - height: 100%; width: $active-indicator-width; background-color: var(--p-action-primary); border-top-right-radius: var(--p-border-radius-base); @@ -123,6 +122,10 @@ $disabled-fade: 0.6; color: var(--p-text-primary); background-color: var(--p-background-pressed); } + + @include breakpoint-after(nav-min-window-corrected()) { + font-weight: 500; + } } .Item-disabled { @@ -282,7 +285,8 @@ $secondary-item-font-size: rem(15px); @include breakpoint-after(nav-min-window-corrected()) { font-size: $item-font-size-small; - line-height: rem(28px); + line-height: rem(16px); + padding-left: nav(icon-size) + spacing(base-tight); } } @@ -290,6 +294,10 @@ $secondary-item-font-size: rem(15px); margin-top: rem(6px); margin-bottom: rem(6px); line-height: rem(20px); + + @include breakpoint-after(nav-min-window-corrected()) { + line-height: rem(16px); + } } .Item-selected { @@ -329,8 +337,12 @@ $secondary-item-font-size: rem(15px); @include safe-area-for(padding-left, 0, left); + .Section { - padding-top: spacing(extra-tight); padding-bottom: spacing(); + padding-top: spacing('tight'); + } + + + .Section-withSeparator { + padding-top: spacing('extra-tight'); } } @@ -344,11 +356,16 @@ $secondary-item-font-size: rem(15px); .SectionHeading { @include text-style-subheading; + text-transform: none; // Adding this intentionally to override the default subheading styling. display: flex; align-items: center; min-height: nav(desktop-nav-height); padding-left: spacing(); + @include breakpoint-after(nav-min-window-corrected()) { + padding-left: spacing(loose); + } + .Text { font-size: font-size(subheading); color: var(--p-text-subdued); @@ -370,6 +387,14 @@ $secondary-item-font-size: rem(15px); @include breakpoint-after(nav-min-window-corrected()) { height: nav(desktop-nav-height); + + // stylelint-disable selector-max-combinators + svg, + img { + height: rem(16px); + width: rem(16px); + margin: rem(2px); + } } @include focus-ring; diff --git a/src/components/Navigation/_variables.scss b/src/components/Navigation/_variables.scss index b238cb04eb6..6fea8027715 100644 --- a/src/components/Navigation/_variables.scss +++ b/src/components/Navigation/_variables.scss @@ -1,13 +1,13 @@ $item-font-size: rem(16px); $item-font-size-small: rem(14px); -$item-line-height-small: rem(32px); +$item-line-height-small: rem(28px); $item-line-height-large: rem(36px); $text-line-height: rem(20px); $nav-variables: ( mobile-spacing: rem(10px), - desktop-spacing: rem(6px), + desktop-spacing: rem(4px), mobile-height: rem(40px), - desktop-height: rem(32px), + desktop-height: rem(28px), icon-size: rem(20px), item-line-height: rem(40px), ); @@ -65,13 +65,30 @@ $nav-animation-variables: ( @include breakpoint-after(nav-min-window-corrected()) { font-size: $item-font-size; - font-weight: 600; + font-weight: 500; line-height: $item-line-height-small; + padding-left: rem(14px); } &::-moz-focus-inner { border: 0; } + + svg, + img { + display: block; + height: rem(20px); + width: rem(20px); + } + + .Icon-resized { + svg, + img { + margin: rem(2px); + height: rem(16px); + width: rem(16px); + } + } } @mixin nav-item-icon-attributes { @@ -93,11 +110,11 @@ $nav-animation-variables: ( width: nav(icon-size); height: nav(icon-size); margin-top: nav(mobile-spacing); - margin-right: spacing(); + margin-right: spacing(base-tight); margin-bottom: nav(mobile-spacing); @include breakpoint-after(nav-min-window-corrected()) { margin-top: nav(desktop-spacing); - margin-right: spacing(); + margin-right: nav(desktop-spacing); margin-bottom: nav(desktop-spacing); } diff --git a/src/components/Navigation/components/Item/Item.tsx b/src/components/Navigation/components/Item/Item.tsx index 8d20b4a0138..8a62a09ce65 100644 --- a/src/components/Navigation/components/Item/Item.tsx +++ b/src/components/Navigation/components/Item/Item.tsx @@ -58,6 +58,7 @@ export interface ItemProps extends ItemURLDetails { subNavigationItems?: SubNavigationItem[]; secondaryAction?: SecondaryAction; onClick?(): void; + shouldResizeIcon?: boolean; } enum MatchState { @@ -85,6 +86,7 @@ export function Item({ matchPaths, excludePaths, external, + shouldResizeIcon, }: ItemProps) { const i18n = useI18n(); const {isNavigationCollapsed} = useMediaQuery(); @@ -125,7 +127,12 @@ export function Item({ ) : null; const iconMarkup = icon ? ( -
+
) : null; From 29507a34a926c4c4bcd4f9b32f6910f6a534284b Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Thu, 20 Jan 2022 10:55:53 +0000 Subject: [PATCH 2/4] chore: update readme --- src/components/Navigation/README.md | 35 +++++++++++++++-------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/components/Navigation/README.md b/src/components/Navigation/README.md index 5a5bad386ef..cc8072217ab 100644 --- a/src/components/Navigation/README.md +++ b/src/components/Navigation/README.md @@ -113,23 +113,24 @@ The content of the navigation component consists of navigation items. Each item #### Item properties -| Prop | Type | Description | -| ------------------ | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | -| url | string | A location for the navigation item to navigate to when clicked | -| matches | boolean | A boolean property indicating whether the navigation item should respond to a closely matching location property | -| exactMatch | boolean | A boolean property indicating whether the navigation item should respond to an exactly matching location property | -| matchPaths | string[] | A string property providing a collection of additional paths for the navigation item to respond to | -| excludePaths | string[] | A string property providing an explicit collection of paths the navigation item should not respond to | -| icon | IconProps['source'] | An icon to be displayed next to the navigation item | -| badge | string \| null | A string property allowing content to be displayed in a badge next to the navigation item | -| label | string | A string property allowing content to be displayed as link text in the navigation item | -| disabled | boolean | A boolean property indicating whether the navigation item is disabled | -| new | boolean | Indicate whether the navigation item is new by adding an indicator dot to the parent and badge to the item (overwritten by the badge prop) | -| accessibilityLabel | string | A visually hidden label for screen readers to understand the content of a navigation item | -| selected | boolean | A boolean property indicating whether the navigation item is the currently-selected item | -| subNavigationItems | SubNavigationItem[] | A collection of navigation items rendered as nested secondary navigation items | -| secondaryAction | SecondaryAction | Renders an icon-only action as a supplementary action next to a navigation item | -| onClick() | function | A callback function to handle clicking on a navigation item | +| Prop | Type | Description | +| ------------------ | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | +| url | string | A location for the navigation item to navigate to when clicked | +| matches | boolean | A boolean property indicating whether the navigation item should respond to a closely matching location property | +| exactMatch | boolean | A boolean property indicating whether the navigation item should respond to an exactly matching location property | +| matchPaths | string[] | A string property providing a collection of additional paths for the navigation item to respond to | +| excludePaths | string[] | A string property providing an explicit collection of paths the navigation item should not respond to | +| icon | IconProps['source'] | An icon to be displayed next to the navigation. Please prefer minor icons here. If a major icon has to be used, set the `shouldResizeIcon` prop to true | +| badge | string \| null | A string property allowing content to be displayed in a badge next to the navigation item | +| label | string | A string property allowing content to be displayed as link text in the navigation item | +| disabled | boolean | A boolean property indicating whether the navigation item is disabled | +| new | boolean | Indicate whether the navigation item is new by adding an indicator dot to the parent and badge to the item (overwritten by the badge prop) | +| accessibilityLabel | string | A visually hidden label for screen readers to understand the content of a navigation item | +| selected | boolean | A boolean property indicating whether the navigation item is the currently-selected item | +| shouldResizeIcon | boolean | Will allow for major icons to be displayed at the same size as minor icons | +| subNavigationItems | SubNavigationItem[] | A collection of navigation items rendered as nested secondary navigation items | +| secondaryAction | SecondaryAction | Renders an icon-only action as a supplementary action next to a navigation item | +| onClick() | function | A callback function to handle clicking on a navigation item | From 36d26137181eaa6a6d99f202ec2c889480ad8573 Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Thu, 20 Jan 2022 12:08:33 +0000 Subject: [PATCH 3/4] chore: update storybook with new minor icons --- .storybook/polaris-readme-loader.js | 9 ++- package.json | 2 +- src/components/Navigation/README.md | 101 +++++++++++++++++++--------- yarn.lock | 8 +-- 4 files changed, 84 insertions(+), 36 deletions(-) diff --git a/.storybook/polaris-readme-loader.js b/.storybook/polaris-readme-loader.js index 965aff23c5d..be765f9cb25 100644 --- a/.storybook/polaris-readme-loader.js +++ b/.storybook/polaris-readme-loader.js @@ -182,6 +182,7 @@ import { CirclePlusOutlineMinor, ConversationMinor, CustomersMajor, + CustomersMinor, DeleteMinor, CircleDisableMinor, DisputeMinor, @@ -191,17 +192,22 @@ import { ExternalMinor, QuestionMarkMajor, HomeMajor, + HomeMinor, HorizontalDotsMinor, ImportMinor, LogOutMinor, MarketingMajor, + MarketingMinor, MobileHamburgerMajor, NoteMinor, NotificationMajor, OnlineStoreMajor, + OnlineStoreMinor, OrdersMajor, + OrdersMinor, PrintMinor, ProductsMajor, + ProductsMinor, ProfileMinor, RefreshMinor, RiskMinor, @@ -356,7 +362,8 @@ function filterMarkdownForPlatform(markdown, platform) { `([\\s\\S]+?)`, 'gu', ); - const deleteRemainingPlatformsRegExp = /[\s\S]+?/gu; + const deleteRemainingPlatformsRegExp = + /[\s\S]+?/gu; return ( markdown diff --git a/package.json b/package.json index f2e8ed500fd..297d0548d50 100644 --- a/package.json +++ b/package.json @@ -72,7 +72,7 @@ "postcolordocs": "yarn run format" }, "dependencies": { - "@shopify/polaris-icons": "^4.11.0", + "@shopify/polaris-icons": "^4.14.0", "@shopify/polaris-tokens": "^3.0.0", "@types/react": "^17.0.19", "@types/react-dom": "^17.0.9", diff --git a/src/components/Navigation/README.md b/src/components/Navigation/README.md index cc8072217ab..24929600a78 100644 --- a/src/components/Navigation/README.md +++ b/src/components/Navigation/README.md @@ -176,18 +176,18 @@ Use to present a navigation menu in the [frame](https://polaris.shopify.com/comp { url: '/', label: 'Home', - icon: HomeMajor, + icon: HomeMinor, }, { url: '/path/to/place', label: 'Orders', - icon: OrdersMajor, + icon: OrdersMinor, badge: '15', }, { url: '/path/to/place', label: 'Products', - icon: ProductsMajor, + icon: ProductsMinor, }, ]} /> @@ -205,18 +205,18 @@ Use to present a secondary action, related to a section and to title the section { url: '/path/to/place', label: 'Home', - icon: HomeMajor, + icon: HomeMinor, }, { url: '/path/to/place', label: 'Orders', - icon: OrdersMajor, + icon: OrdersMinor, badge: '15', }, { url: '/admin/products', label: 'Products', - icon: ProductsMajor, + icon: ProductsMinor, selected: true, subNavigationItems: [ { @@ -248,17 +248,17 @@ Use to present a secondary action, related to a section and to title the section { url: '/path/to/place', label: 'Home', - icon: HomeMajor, + icon: HomeMinor, }, { url: '/path/to/place', label: 'Orders', - icon: OrdersMajor, + icon: OrdersMinor, }, { url: '/path/to/place', label: 'Products', - icon: ProductsMajor, + icon: ProductsMinor, }, ]} /> @@ -268,7 +268,7 @@ Use to present a secondary action, related to a section and to title the section { url: '/path/to/place', label: 'Online Store', - icon: OnlineStoreMajor, + icon: OnlineStoreMinor, }, ]} action={{ @@ -291,12 +291,12 @@ Use to add a different action for an item than the main action, like to view or { url: '/path/to/place', label: 'Home', - icon: HomeMajor, + icon: HomeMinor, }, { url: '/path/to/place', label: 'Orders', - icon: OrdersMajor, + icon: OrdersMinor, secondaryAction: { url: '/admin/orders/add', accessibilityLabel: 'Add an order', @@ -306,7 +306,7 @@ Use to add a different action for an item than the main action, like to view or { url: '/path/to/place', label: 'Products', - icon: ProductsMajor, + icon: ProductsMinor, }, ]} /> @@ -324,17 +324,17 @@ Use to show a limited number of items in a section with an option to expand the { url: '/path/to/place', label: 'Home', - icon: HomeMajor, + icon: HomeMinor, }, { url: '/path/to/place', label: 'Orders', - icon: OrdersMajor, + icon: OrdersMinor, }, { url: '/path/to/place', label: 'Products', - icon: ProductsMajor, + icon: ProductsMinor, }, ]} rollup={{ @@ -358,17 +358,17 @@ Use to add a horizontal line below the section. { url: '/path/to/place', label: 'Home', - icon: HomeMajor, + icon: HomeMinor, }, { url: '/path/to/place', label: 'Orders', - icon: OrdersMajor, + icon: OrdersMinor, }, { url: '/path/to/place', label: 'Products', - icon: ProductsMajor, + icon: ProductsMinor, }, ]} /> @@ -377,7 +377,7 @@ Use to add a horizontal line below the section. { url: '/path/to/place', label: 'Online Store', - icon: OnlineStoreMajor, + icon: OnlineStoreMinor, }, ]} separator @@ -396,12 +396,12 @@ This example showcases the many elements that can compose a navigation, especial { url: '/path/to/place', label: 'Inactive item', - icon: HomeMajor, + icon: HomeMinor, }, { url: '/path/to/place', label: 'Item with indicator', - icon: HomeMajor, + icon: HomeMinor, subNavigationItems: [ { url: '/path/to/place/index', @@ -414,25 +414,25 @@ This example showcases the many elements that can compose a navigation, especial { url: '/path/to/place', label: 'External link item', - icon: HomeMajor, + icon: HomeMinor, external: true, }, { url: '/path/to/place', label: 'New item', new: true, - icon: HomeMajor, + icon: HomeMinor, }, { url: '/path/to/place', label: 'Badged item', badge: 'Old', - icon: HomeMajor, + icon: HomeMinor, }, { url: '/path/to/place', label: 'Active with secondary action', - icon: OrdersMajor, + icon: OrdersMinor, selected: true, secondaryAction: { url: '/admin/orders/add', @@ -443,7 +443,7 @@ This example showcases the many elements that can compose a navigation, especial { url: '/admin/products', label: 'Active item with sub navigation', - icon: ProductsMajor, + icon: ProductsMinor, selected: true, subNavigationItems: [ { @@ -467,13 +467,13 @@ This example showcases the many elements that can compose a navigation, especial { url: '/path/to/place', label: 'Disabled item', - icon: CustomersMajor, + icon: CustomersMinor, disabled: true, }, { url: '/path/to/place', label: 'Overflow item', - icon: MarketingMajor, + icon: MarketingMinor, }, ]} rollup={{ @@ -489,21 +489,24 @@ This example showcases the many elements that can compose a navigation, especial { url: '/path/to/place', label: 'Icon as svg', - icon: OnlineStoreMajor, + icon: OnlineStoreMinor, }, { url: '/path/to/place', label: 'Icon as img', + shouldResizeIcon: true, icon: '', }, { url: '/', label: 'Icon as img – Active', + shouldResizeIcon: true, icon: '', }, { url: '/path/to/place', label: 'Other secondary action', + shouldResizeIcon: true, icon: '', secondaryAction: { url: '/path/to/place/view', @@ -531,23 +534,61 @@ This example shows how to add an aria-labelledby to add a hidden label to the `n

Hidden label

 + + +``` + +### Navigation using Major icons + +This example shows how to use the shouldResizeIcon prop when using Major icons + +```jsx + diff --git a/yarn.lock b/yarn.lock index cd27f34b7a3..6a49b515a93 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2008,10 +2008,10 @@ fs-extra "^9.0.0" glob "^7.1.6" -"@shopify/polaris-icons@^4.11.0": - version "4.11.0" - resolved "https://registry.yarnpkg.com/@shopify/polaris-icons/-/polaris-icons-4.11.0.tgz#8b1ebf7f07077300f8281e12824588835f108b23" - integrity sha512-vq37+I6oDlFzQXqkuDI0DO4AXz36AiyGOMkSstnBWTI6UR/eHAuI2aALqbPMxYbD6877Pi1kk+bAfWIFf3D4LA== +"@shopify/polaris-icons@^4.14.0": + version "4.14.0" + resolved "https://registry.yarnpkg.com/@shopify/polaris-icons/-/polaris-icons-4.14.0.tgz#26d134af7fda64e81ecb97212ff5f424a8a6774a" + integrity sha512-UyNcPmR/emsL01ezXR87U++9fLANaBiZZtG55XBe92cDeiG+VmVnx/a75U9wEL9jlDGjWNtO1Jd7juq0r/EHLQ== "@shopify/polaris-tokens@^3.0.0": version "3.0.0" From faec46f0cc2920b9dbd92e9c426c5b57d7a8b268 Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Tue, 25 Jan 2022 17:45:41 +0000 Subject: [PATCH 4/4] chore: use spacing over rem --- package.json | 2 +- src/components/Navigation/Navigation.scss | 28 ++++++++++------------- src/components/Navigation/_variables.scss | 12 +++++----- yarn.lock | 2 +- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index 297d0548d50..db53266cfe4 100644 --- a/package.json +++ b/package.json @@ -72,7 +72,7 @@ "postcolordocs": "yarn run format" }, "dependencies": { - "@shopify/polaris-icons": "^4.14.0", + "@shopify/polaris-icons": "4.14.0", "@shopify/polaris-tokens": "^3.0.0", "@types/react": "^17.0.19", "@types/react-dom": "^17.0.9", diff --git a/src/components/Navigation/Navigation.scss b/src/components/Navigation/Navigation.scss index c9e9460dd9c..4b1c806f0f3 100644 --- a/src/components/Navigation/Navigation.scss +++ b/src/components/Navigation/Navigation.scss @@ -55,7 +55,7 @@ $nav-max-width: rem(360px); } @include breakpoint-after(nav-min-window-corrected()) { - padding-top: rem(16px); + padding-top: spacing(); } } @@ -93,8 +93,8 @@ $disabled-fade: 0.6; &::before { content: ''; position: absolute; - top: rem(1px); - bottom: rem(1px); + top: 1px; + bottom: 1px; left: -1 * spacing(tight); width: $active-indicator-width; background-color: var(--p-action-primary); @@ -285,19 +285,15 @@ $secondary-item-font-size: rem(15px); @include breakpoint-after(nav-min-window-corrected()) { font-size: $item-font-size-small; - line-height: rem(16px); + line-height: 1; padding-left: nav(icon-size) + spacing(base-tight); } } .Text { - margin-top: rem(6px); - margin-bottom: rem(6px); - line-height: rem(20px); - - @include breakpoint-after(nav-min-window-corrected()) { - line-height: rem(16px); - } + margin-top: spacing(tight) / 2; + margin-bottom: spacing(tight) / 2; + line-height: spacing(loose); } .Item-selected { @@ -338,11 +334,11 @@ $secondary-item-font-size: rem(15px); + .Section { padding-bottom: spacing(); - padding-top: spacing('tight'); + padding-top: spacing(tight); } + .Section-withSeparator { - padding-top: spacing('extra-tight'); + padding-top: spacing(extra-tight); } } @@ -391,9 +387,9 @@ $secondary-item-font-size: rem(15px); // stylelint-disable selector-max-combinators svg, img { - height: rem(16px); - width: rem(16px); - margin: rem(2px); + height: spacing(); + width: spacing(); + margin: spacing(extra-tight) / 2; } } diff --git a/src/components/Navigation/_variables.scss b/src/components/Navigation/_variables.scss index 6fea8027715..235d933f8c0 100644 --- a/src/components/Navigation/_variables.scss +++ b/src/components/Navigation/_variables.scss @@ -67,7 +67,7 @@ $nav-animation-variables: ( font-size: $item-font-size; font-weight: 500; line-height: $item-line-height-small; - padding-left: rem(14px); + padding-left: spacing() - (spacing(extra-tight) / 2); } &::-moz-focus-inner { @@ -77,16 +77,16 @@ $nav-animation-variables: ( svg, img { display: block; - height: rem(20px); - width: rem(20px); + height: spacing(loose); + width: spacing(loose); } .Icon-resized { svg, img { - margin: rem(2px); - height: rem(16px); - width: rem(16px); + margin: spacing(extra-tight) / 2; + height: spacing(); + width: spacing(); } } } diff --git a/yarn.lock b/yarn.lock index 6a49b515a93..a32c45fedf2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2008,7 +2008,7 @@ fs-extra "^9.0.0" glob "^7.1.6" -"@shopify/polaris-icons@^4.14.0": +"@shopify/polaris-icons@4.14.0": version "4.14.0" resolved "https://registry.yarnpkg.com/@shopify/polaris-icons/-/polaris-icons-4.14.0.tgz#26d134af7fda64e81ecb97212ff5f424a8a6774a" integrity sha512-UyNcPmR/emsL01ezXR87U++9fLANaBiZZtG55XBe92cDeiG+VmVnx/a75U9wEL9jlDGjWNtO1Jd7juq0r/EHLQ==