Remove Box usage and sx prop from UnderlineNav

.changeset/busy-planes-leave.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': major
+---
+
+Removes sx prop from UnderlineNav components

packages/react/src/UnderlineNav/UnderlineNav.docs.json

@@ -55,11 +55,6 @@
       "type": "'inset' | 'flush'",
       "defaultValue": "'inset'",
       "description": "`inset` children are offset horizontally from container edges. `flush` children are flush horizontally with container edges"
-    },
-    {
-      "name": "sx",
-      "type": "SystemStyleObject",
-      "deprecated": true
     }
   ],
   "subcomponents": [
@@ -100,11 +95,6 @@
           "name": "as",
           "type": "React.ElementType",
           "defaultValue": "\"a\""
-        },
-        {
-          "name": "sx",
-          "type": "SystemStyleObject",
-          "deprecated": true
         }
       ],
       "passthrough": {

packages/react/src/UnderlineNav/UnderlineNav.module.css

@@ -0,0 +1,77 @@
+.Divider {
+  display: inline-block;
+  border-left: var(--borderWidth-thin) solid var(--borderColor-muted);
+  width: var(--borderWidth-thin);
+  margin-right: var(--base-size-4);
+  height: 24px; /* The height of the divider - reference from Figma */
+}
+
+.MoreButton {
+  /* Set margin 0 here because safari puts extra margin around the button,
+     rest is to reset style to make it look like a list element */
+  margin: 0;
+  border: 0;
+  font-weight: var(--base-text-weight-normal);
+  box-shadow: none;
+  padding-block: var(--base-size-4);
+  padding-inline: var(--base-size-8);
+}
+
+.MoreButton,
+.MoreButton:hover,
+.MoreButton[aria-expanded='true'] {
+  background: transparent;
+}
+
+.MoreButton[data-component='trailingVisual'] {
+  margin-left: 0;
+}
+
+.MoreMenuListItem {
+  display: flex;
+  align-items: center;
+  /* Hard-coded height is needed to make sure we don't have a layout shift when the more button is the only item in the nav. */
+  height: 45px;
+}
+
+.MenuContainer {
+  position: absolute;
+  z-index: 1;
+  top: 90%;
+  box-shadow:
+  /* TODO: replace custom shadow with Primer Primitives variable */
+  /* stylelint-disable-next-line primer/box-shadow */
+    rgba(0, 0, 0, 0.12) 0 1px 3px,
+    rgba(0, 0, 0, 0.24) 0 1px 2px;
+  border-radius: var(--borderRadius-large);
+  background-color: var(--bgColor-default);
+  list-style: none;
+  min-width: 192px; /* baseMenuMinWidth */
+  max-width: 640px;
+  right: 0;
+  display: block;
+}
+
+.MenuContainer[data-is-widget-open='false'] {
+  display: none;
+}
+
+.MenuItem {
+  text-decoration: none;
+}
+
+.MenuItem > span:first-child {
+  display: none;
+}
+
+.MenuItemContent {
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+}
+
+.NavListItem {
+  display: flex;
+  flex-direction: column;
+  align-items: center;
+}

packages/react/src/UnderlineNav/UnderlineNav.test.tsx

@@ -14,7 +14,7 @@ import {
 } from '@primer/octicons-react'
 
 import {UnderlineNav} from '.'
-import {baseMenuMinWidth, menuStyles} from './styles'
+import {BASE_MENU_MIN_WIDTH, getLeftAnchoredPosition} from './UnderlineNav'
 
 const ResponsiveUnderlineNav = ({
   selectedItemText = 'Code',
@@ -166,20 +166,20 @@ describe('UnderlineNav', () => {
     spy.mockRestore()
   })
 
-  it(`menuStyles should set the menu position, if the container size is below ${baseMenuMinWidth} px`, () => {
+  it(`should set the menu position using getLeftAnchoredPosition, if the container size is below ${BASE_MENU_MIN_WIDTH} px`, () => {
     // GIVEN
     // Mock the refs.
     const containerRef = document.createElement('div')
     const listRef = document.createElement('div')
     // Set the clientWidth on the mock element
-    Object.defineProperty(listRef, 'clientWidth', {value: baseMenuMinWidth - 1})
+    Object.defineProperty(listRef, 'clientWidth', {value: BASE_MENU_MIN_WIDTH - 1})
 
     // WHEN
-    const results = menuStyles(containerRef, listRef)
+    const results = getLeftAnchoredPosition(containerRef, listRef)
 
     // THEN
-    // We are expecting a left value back, that way we know the `getAnchoredPosition` ran.
-    expect(results).toEqual(expect.objectContaining({left: 0}))
+    // We are expecting a left value back, that way we know the `getLeftAnchoredPosition` ran.
+    expect(results).toEqual(0)
   })
 
   it('should support icons passed in as an element', () => {

packages/react/src/UnderlineNav/UnderlineNav.tsx

@@ -1,33 +1,28 @@
 import type {MutableRefObject, RefObject} from 'react'
 import React, {useRef, forwardRef, useCallback, useState, useEffect} from 'react'
-import Box from '../Box'
-import type {SxProp} from '../sx'
-import sx from '../sx'
+import {getAnchoredPosition} from '@primer/behaviors'
 import {UnderlineNavContext} from './UnderlineNavContext'
 import type {ResizeObserverEntry} from '../hooks/useResizeObserver'
 import {useResizeObserver} from '../hooks/useResizeObserver'
 import {useTheme} from '../ThemeProvider'
 import type {ChildWidthArray, ResponsiveProps, ChildSize} from './types'
 import VisuallyHidden from '../_VisuallyHidden'
-import {moreBtnStyles, getDividerStyle, menuStyles, menuItemStyles, baseMenuStyles, baseMenuMinWidth} from './styles'
 import {UnderlineItemList, UnderlineWrapper, LoadingCounter, GAP} from '../internal/components/UnderlineTabbedInterface'
-import styled from 'styled-components'
 import {Button} from '../Button'
 import {TriangleDownIcon} from '@primer/octicons-react'
 import {useOnEscapePress} from '../hooks/useOnEscapePress'
 import {useOnOutsideClick} from '../hooks/useOnOutsideClick'
 import {useId} from '../hooks/useId'
 import {ActionList} from '../ActionList'
-import {defaultSxProp} from '../utils/defaultSxProp'
 import CounterLabel from '../CounterLabel'
 import {invariant} from '../utils/invariant'
+import classes from './UnderlineNav.module.css'
 
 export type UnderlineNavProps = {
   children: React.ReactNode
   'aria-label'?: React.AriaAttributes['aria-label']
   as?: React.ElementType
   className?: string
-  sx?: SxProp['sx']
   /**
    * loading state for all counters. It displays loading animation for individual counters (UnderlineNav.Item) until all are resolved. It is needed to prevent multiple layout shift.
    */
@@ -42,19 +37,7 @@ export type UnderlineNavProps = {
 // When page is loaded, we don't have ref for the more button as it is not on the DOM yet.
 // However, we need to calculate number of possible items when the more button present as well. So using the width of the more button as a constant.
 export const MORE_BTN_WIDTH = 86
-// The height is needed to make sure we don't have a layout shift when the more button is the only item in the nav.
-const MORE_BTN_HEIGHT = 45
-
-// Needed this because passing a ref using HTMLULListElement to `Box` causes a type error
-export const NavigationList = styled.ul`
-  ${sx};
-`
-
-export const MoreMenuListItem = styled.li`
-  display: flex;
-  align-items: center;
-  height: ${MORE_BTN_HEIGHT}px;
-`
+export const BASE_MENU_MIN_WIDTH = 192
 
 const overflowEffect = (
   navWidth: number,
@@ -140,12 +123,25 @@ const calculatePossibleItems = (childWidthArray: ChildWidthArray, navWidth: numb
   return breakpoint
 }
 
+/**
+ *
+ * @param containerRef The Menu List Container Reference.
+ * @param listRef The Underline Nav Container Reference.
+ * @description This calculates the position of the menu
+ *
+ * Exported because it's used in unit tests.
+ */
+export const getLeftAnchoredPosition = (containerRef: Element | null, listRef: Element | null): number => {
+  return containerRef && listRef
+    ? getAnchoredPosition(containerRef, listRef, {align: 'start', side: 'outside-bottom'}).left
+    : 0
+}
+
 export const UnderlineNav = forwardRef(
   (
     {
       as = 'nav',
       'aria-label': ariaLabel,
-      sx: sxProp = defaultSxProp,
       loadingCounters = false,
       variant = 'inset',
       className,
@@ -316,28 +312,21 @@ export const UnderlineNav = forwardRef(
         }}
       >
         {ariaLabel && <VisuallyHidden as="h2">{`${ariaLabel} navigation`}</VisuallyHidden>}
-        <UnderlineWrapper
-          as={as}
-          aria-label={ariaLabel}
-          className={className}
-          ref={navRef}
-          sx={sxProp}
-          data-variant={variant}
-        >
+        <UnderlineWrapper as={as} aria-label={ariaLabel} className={className} ref={navRef} data-variant={variant}>
           <UnderlineItemList ref={listRef} role="list">
             {listItems}
             {menuItems.length > 0 && (
-              <MoreMenuListItem ref={moreMenuRef}>
-                {!onlyMenuVisible && <Box sx={getDividerStyle(theme)}></Box>}
+              <li className={classes.MoreMenuListItem} ref={moreMenuRef}>
+                {!onlyMenuVisible && <div className={classes.Divider}></div>}
                 <Button
                   ref={moreMenuBtnRef}
-                  sx={moreBtnStyles}
+                  className={classes.MoreButton}
                   aria-controls={disclosureWidgetId}
                   aria-expanded={isWidgetOpen}
                   onClick={onAnchorClick}
                   trailingAction={TriangleDownIcon}
                 >
-                  <Box as="span">
+                  <span className={classes.ButtonText}>
                     {onlyMenuVisible ? (
                       <>
                         <VisuallyHidden as="span">{`${ariaLabel}`}&nbsp;</VisuallyHidden>Menu
@@ -347,18 +336,20 @@ export const UnderlineNav = forwardRef(
                         More<VisuallyHidden as="span">&nbsp;{`${ariaLabel} items`}</VisuallyHidden>
                       </>
                     )}
-                  </Box>
+                  </span>
                 </Button>
                 <ActionList
                   selectionVariant="single"
                   ref={containerRef}
                   id={disclosureWidgetId}
-                  sx={
-                    listRef.current?.clientWidth && listRef.current.clientWidth >= baseMenuMinWidth
-                      ? baseMenuStyles
-                      : menuStyles(containerRef.current, listRef.current)
-                  }
-                  style={{display: isWidgetOpen ? 'block' : 'none'}}
+                  className={classes.MenuContainer}
+                  data-is-widget-open={isWidgetOpen}
+                  style={{
+                    left:
+                      listRef.current?.clientWidth && listRef.current.clientWidth >= BASE_MENU_MIN_WIDTH
+                        ? undefined
+                        : getLeftAnchoredPosition(containerRef.current, listRef.current),
+                  }}
                 >
                   {menuItems.map((menuItem, index) => {
                     const {
@@ -385,7 +376,7 @@ export const UnderlineNav = forwardRef(
                     return (
                       <ActionList.LinkItem
                         key={menuItemChildren}
-                        sx={menuItemStyles}
+                        className={classes.MenuItem}
                         onClick={(
                           event: React.MouseEvent<HTMLAnchorElement> | React.KeyboardEvent<HTMLAnchorElement>,
                         ) => {
@@ -398,23 +389,23 @@ export const UnderlineNav = forwardRef(
                         }}
                         {...menuItemProps}
                       >
-                        <Box as="span" sx={{display: 'flex', alignItems: 'center', justifyContent: 'space-between'}}>
+                        <span className={classes.MenuItemContent}>
                           {menuItemChildren}
                           {loadingCounters ? (
                             <LoadingCounter />
                           ) : (
                             counter !== undefined && (
-                              <Box as="span" data-component="counter">
+                              <span data-component="counter">
                                 <CounterLabel>{counter}</CounterLabel>
-                              </Box>
+                              </span>
                             )
                           )}
-                        </Box>
+                        </span>
                       </ActionList.LinkItem>
                     )
                   })}
                 </ActionList>
-              </MoreMenuListItem>
+              </li>
             )}
           </UnderlineItemList>
         </UnderlineWrapper>