From 4782538c71811fc7e0ab860d7c80dc16c5b4f534 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andi=20Pa=CC=88tzold?= <andreas.paetzold@contentful.com>
Date: Thu, 29 Aug 2024 17:55:21 +0200
Subject: [PATCH 1/6] feat(navbar): various navbar spacing tweaks

---
 packages/components/navbar/src/Navbar.styles.ts |  2 ++
 .../navbar/src/NavbarItem/NavbarItem.styles.ts  | 17 ++++++++++++-----
 .../navbar/src/NavbarItem/NavbarItem.tsx        | 15 +++++++--------
 .../src/NavbarItem/NavbarItemSkeleton.tsx       |  6 +++---
 .../NavbarSwitcher/NavbarSwitcherSkeleton.tsx   |  8 ++++----
 5 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/packages/components/navbar/src/Navbar.styles.ts b/packages/components/navbar/src/Navbar.styles.ts
index 236180e7cb..229699f110 100644
--- a/packages/components/navbar/src/Navbar.styles.ts
+++ b/packages/components/navbar/src/Navbar.styles.ts
@@ -40,7 +40,9 @@ export const getNavbarStyles = ({
 
   mobileNavigationButton: css({
     display: 'flex',
+    minHeight: 'initial', // unset default 40px height
     height: '36px',
+    padding: '0 12px',
     borderRadius: '10px',
     [mqs.small]: {
       display: 'none',
diff --git a/packages/components/navbar/src/NavbarItem/NavbarItem.styles.ts b/packages/components/navbar/src/NavbarItem/NavbarItem.styles.ts
index 2985603fd5..484bf23609 100644
--- a/packages/components/navbar/src/NavbarItem/NavbarItem.styles.ts
+++ b/packages/components/navbar/src/NavbarItem/NavbarItem.styles.ts
@@ -3,10 +3,12 @@ import tokens from '@contentful/f36-tokens';
 import { hexToRGBA } from '@contentful/f36-utils';
 import { getGlowOnFocusStyles, increaseHitArea, mqs } from '../utils.styles';
 
+const borderWidth = '1px';
+
 export const getNavbarItemActiveStyles = () =>
   css({
     backgroundColor: tokens.blue100,
-    border: `1px solid ${tokens.blue400}`,
+    border: `${borderWidth} solid ${tokens.blue400}`,
     color: tokens.blue600,
     '&:hover': {
       backgroundColor: tokens.blue100,
@@ -16,19 +18,19 @@ export const getNavbarItemActiveStyles = () =>
 const commonItemStyles = {
   display: 'flex',
   justifyContent: 'center',
-  padding: `${tokens.spacing2Xs} ${tokens.spacingXs}`,
+  padding: `calc(${tokens.spacing2Xs} - ${borderWidth}) calc(${tokens.spacingXs} - ${borderWidth})`,
   alignItems: 'center',
   background: 'none',
   gap: tokens.spacing2Xs,
 };
 
-export const getNavbarItemStyles = ({ title }) => ({
+export const getNavbarItemStyles = ({ hasTitle }: { hasTitle: boolean }) => ({
   navbarItem: css(
     commonItemStyles,
     {
       appearance: 'none',
       background: 'none',
-      border: '1px solid transparent',
+      border: `${borderWidth} solid transparent`,
       margin: 0,
       outline: 'none',
       fontSize: tokens.fontSizeM,
@@ -45,6 +47,10 @@ export const getNavbarItemStyles = ({ title }) => ({
       transition: `color ${tokens.transitionDurationShort} ${tokens.transitionEasingCubicBezier}`,
       borderRadius: tokens.borderRadiusMedium,
 
+      padding: hasTitle
+        ? undefined
+        : `calc(${tokens.spacing2Xs} - ${borderWidth})`, // square button for icon-only items
+
       '&:hover': {
         backgroundColor: hexToRGBA(tokens.gray900, 0.05),
       },
@@ -78,10 +84,11 @@ export const getNavbarItemStyles = ({ title }) => ({
   icon: css({
     height: '20px',
     width: '20px',
-    display: !title ? 'block' : 'none',
+    display: hasTitle ? 'none' : 'block',
     [mqs.small]: {
       height: '16px',
       width: '16px',
+      padding: hasTitle ? '2px 0' : '2px', // square for icon-only items
     },
     [mqs.large]: {
       display: 'block',
diff --git a/packages/components/navbar/src/NavbarItem/NavbarItem.tsx b/packages/components/navbar/src/NavbarItem/NavbarItem.tsx
index 9ba2cf2d7d..18f719c983 100644
--- a/packages/components/navbar/src/NavbarItem/NavbarItem.tsx
+++ b/packages/components/navbar/src/NavbarItem/NavbarItem.tsx
@@ -56,19 +56,18 @@ function _NavbarItem(
     onClose,
     ...otherProps
   } = props;
-  const styles = getNavbarItemStyles({ title });
+  const styles = getNavbarItemStyles({ hasTitle: !!title });
   const isMenuTrigger = isNavbarItemHasMenu(props);
+  const showCaret = title && isMenuTrigger;
   const item = (
     <Comp
       {...otherProps}
       ref={ref}
       data-test-id={testId}
-      className={cx(
-        styles.navbarItem,
-        isMenuTrigger && styles.navbarItemMenuTrigger,
-        isActive && styles.isActive,
-        className,
-      )}
+      className={cx(styles.navbarItem, className, {
+        [styles.navbarItemMenuTrigger]: showCaret,
+        [styles.isActive]: isActive,
+      })}
       aria-label={title ? '' : label}
     >
       {icon && (
@@ -79,7 +78,7 @@ function _NavbarItem(
         />
       )}
       {title && <span>{title}</span>}
-      {title && isMenuTrigger && <CaretIcon size="tiny" isActive={isActive} />}
+      {showCaret && <CaretIcon size="tiny" isActive={isActive} />}
     </Comp>
   );
 
diff --git a/packages/components/navbar/src/NavbarItem/NavbarItemSkeleton.tsx b/packages/components/navbar/src/NavbarItem/NavbarItemSkeleton.tsx
index 96976a7e4a..98a385247d 100644
--- a/packages/components/navbar/src/NavbarItem/NavbarItemSkeleton.tsx
+++ b/packages/components/navbar/src/NavbarItem/NavbarItemSkeleton.tsx
@@ -14,14 +14,14 @@ export const NavbarItemSkeleton = ({
     <SkeletonContainer
       className={styles.itemSkeleton}
       svgWidth={estimatedWidth}
-      svgHeight={32}
+      svgHeight={28}
       backgroundColor={tokens.gray300}
       foregroundColor={tokens.gray200}
     >
       <SkeletonText
-        lineHeight={6}
+        lineHeight={18}
         numberOfLines={1}
-        offsetTop={10}
+        offsetTop={2}
         radiusX={tokens.borderRadiusSmall}
         radiusY={tokens.borderRadiusSmall}
       />
diff --git a/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx b/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx
index e24952f860..d8c59fed5a 100644
--- a/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx
+++ b/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx
@@ -12,15 +12,15 @@ export const NavbarSwitcherSkeleton = ({
 }) => (
   <SkeletonContainer
     svgWidth={estimatedWidth}
-    svgHeight={24}
+    svgHeight={18}
     backgroundColor={tokens.gray300}
     foregroundColor={tokens.gray200}
   >
     <SkeletonDisplayText
-      lineHeight={24}
+      lineHeight={18}
       numberOfLines={1}
-      radiusX={12}
-      radiusY={12}
+      radiusX={9}
+      radiusY={9}
     />
   </SkeletonContainer>
 );

From 6ce188df126b67026b031ee6a9c2d285af59998e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andi=20Pa=CC=88tzold?= <andreas.paetzold@contentful.com>
Date: Thu, 29 Aug 2024 18:03:24 +0200
Subject: [PATCH 2/6] fix: add box-sizing

---
 packages/components/navbar/src/NavbarItem/NavbarItem.styles.ts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/components/navbar/src/NavbarItem/NavbarItem.styles.ts b/packages/components/navbar/src/NavbarItem/NavbarItem.styles.ts
index 484bf23609..78b66f26c7 100644
--- a/packages/components/navbar/src/NavbarItem/NavbarItem.styles.ts
+++ b/packages/components/navbar/src/NavbarItem/NavbarItem.styles.ts
@@ -84,6 +84,7 @@ export const getNavbarItemStyles = ({ hasTitle }: { hasTitle: boolean }) => ({
   icon: css({
     height: '20px',
     width: '20px',
+    boxSizing: 'content-box',
     display: hasTitle ? 'none' : 'block',
     [mqs.small]: {
       height: '16px',

From a67495e82c9b98d8ed497f2f378199f3373ae9d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andi=20Pa=CC=88tzold?= <andreas.paetzold@contentful.com>
Date: Fri, 30 Aug 2024 15:13:14 +0200
Subject: [PATCH 3/6] fix: change switcher skeleton radius

---
 .../navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx      | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx b/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx
index d8c59fed5a..e5f673fb0f 100644
--- a/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx
+++ b/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx
@@ -19,8 +19,8 @@ export const NavbarSwitcherSkeleton = ({
     <SkeletonDisplayText
       lineHeight={18}
       numberOfLines={1}
-      radiusX={9}
-      radiusY={9}
+      radiusX={tokens.borderRadiusSmall}
+      radiusY={tokens.borderRadiusSmall}
     />
   </SkeletonContainer>
 );

From 9dafba493ad6302afb0cde9c6570a4b2775d8f05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andi=20Pa=CC=88tzold?= <andreas.paetzold@contentful.com>
Date: Fri, 30 Aug 2024 15:19:23 +0200
Subject: [PATCH 4/6] fix: use `SkeletonText` instead of `SkeletonDisplayText`

---
 .../navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx   | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx b/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx
index e5f673fb0f..9315b6d81c 100644
--- a/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx
+++ b/packages/components/navbar/src/NavbarSwitcher/NavbarSwitcherSkeleton.tsx
@@ -1,8 +1,5 @@
 import React from 'react';
-import {
-  SkeletonContainer,
-  SkeletonDisplayText,
-} from '@contentful/f36-skeleton';
+import { SkeletonContainer, SkeletonText } from '@contentful/f36-skeleton';
 import tokens from '@contentful/f36-tokens';
 
 export const NavbarSwitcherSkeleton = ({
@@ -16,7 +13,7 @@ export const NavbarSwitcherSkeleton = ({
     backgroundColor={tokens.gray300}
     foregroundColor={tokens.gray200}
   >
-    <SkeletonDisplayText
+    <SkeletonText
       lineHeight={18}
       numberOfLines={1}
       radiusX={tokens.borderRadiusSmall}

From 7abe233ef6eaa5256130f7340e618958f9802bcd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andi=20Pa=CC=88tzold?= <andreas.paetzold@contentful.com>
Date: Fri, 30 Aug 2024 15:19:37 +0200
Subject: [PATCH 5/6] fix: do not wrap `SwitcherSkeleton` in `Switcher`

---
 packages/components/navbar/stories/Navbar.stories.tsx | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/packages/components/navbar/stories/Navbar.stories.tsx b/packages/components/navbar/stories/Navbar.stories.tsx
index fd83d8ba24..b715f8fc2b 100644
--- a/packages/components/navbar/stories/Navbar.stories.tsx
+++ b/packages/components/navbar/stories/Navbar.stories.tsx
@@ -547,11 +547,7 @@ export const LoadingSkeleton: Story<{}> = () => {
       <Navbar
         mobileNavigation={<MobileMenu />}
         account={<Navbar.AccountSkeleton ariaLabel="Loading account" />}
-        switcher={
-          <Navbar.Switcher>
-            <Navbar.SwitcherSkeleton estimatedWidth={148} />
-          </Navbar.Switcher>
-        }
+        switcher={<Navbar.SwitcherSkeleton estimatedWidth={148} />}
         mainNavigation={
           <>
             <Navbar.ItemSkeleton estimatedWidth={100} />

From d2d4b125c529d6e76a1063e24a2ce3d903626b81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andi=20Pa=CC=88tzold?= <andreas.paetzold@contentful.com>
Date: Fri, 30 Aug 2024 16:52:34 +0200
Subject: [PATCH 6/6] chore: release new navbar version

---
 package-lock.json                       | 4 ++--
 packages/components/navbar/package.json | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index f96feb8a1f..5af7e215b7 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -41237,7 +41237,7 @@
     },
     "packages/components/layout": {
       "name": "@contentful/f36-layout",
-      "version": "5.0.0-alpha.9",
+      "version": "5.0.0-alpha.15",
       "license": "MIT",
       "dependencies": {
         "@contentful/f36-core": "^4.68.1",
@@ -41530,7 +41530,7 @@
     },
     "packages/components/navbar": {
       "name": "@contentful/f36-navbar",
-      "version": "5.0.0-alpha.29",
+      "version": "5.0.0-alpha.30",
       "license": "MIT",
       "dependencies": {
         "@contentful/f36-avatar": "4.68.1",
diff --git a/packages/components/navbar/package.json b/packages/components/navbar/package.json
index 1c30383a32..56a21309c6 100644
--- a/packages/components/navbar/package.json
+++ b/packages/components/navbar/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@contentful/f36-navbar",
-  "version": "5.0.0-alpha.29",
+  "version": "5.0.0-alpha.30",
   "description": "Forma 36: Navbar component",
   "scripts": {
     "build": "tsup"