UnderlineNav with updated design for states and counter (#2277)

* UnderlineNav states with counters * update li and a hover and focus states * add a wrapper to style hover * Fix rounded bordem bottom when rounding the outline * fix padding and color name * move radius to wrapper initial style * use px rather than primitive * remove margin and change the width of select state line * add the underline back * focus and selected border re-implement * add comments * focus trap and fixes * theme aware underlineNav and fix safari extra margin issue * add changeset * update docs and tests * fix export issues * add a comment and fix the test import * update changeset to minor
primer · Sep 5, 2022 · cc88235 · cc88235
1 parent 4536b87
commit cc88235
Show file tree

Hide file tree

Showing 7 changed files with 162 additions and 76 deletions.
diff --git a/.changeset/thirty-mayflies-travel.md b/.changeset/thirty-mayflies-travel.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+UnderlineNav.Link renamed to UnderlineNav.Item along with updated styles
diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx
@@ -10,29 +10,29 @@ description: Use an underlined nav to allow tab like navigation with overflow be
 
 ```jsx live drafts
 <UnderlineNav label="simple nav">
-  <UnderlineNav.Link selected>Item 1</UnderlineNav.Link>
-  <UnderlineNav.Link>Item 2</UnderlineNav.Link>
-  <UnderlineNav.Link>Item 3</UnderlineNav.Link>
+  <UnderlineNav.Item selected>Item 1</UnderlineNav.Item>
+  <UnderlineNav.Item>Item 2</UnderlineNav.Item>
+  <UnderlineNav.Item>Item 3</UnderlineNav.Item>
 </UnderlineNav>
 ```
 
 ### With icons
 
 ```jsx live drafts
 <UnderlineNav label="simple nav with icons">
-  <UnderlineNav.Link selected leadingIcon={EyeIcon}>
+  <UnderlineNav.Item selected leadingIcon={EyeIcon}>
     Item 1
-  </UnderlineNav.Link>
-  <UnderlineNav.Link>Item 2</UnderlineNav.Link>
+  </UnderlineNav.Item>
+  <UnderlineNav.Item>Item 2</UnderlineNav.Item>
 </UnderlineNav>
 ```
 
 ### Small variant
 
 ```jsx live drafts
 <UnderlineNav label="small variant" variant="small">
-  <UnderlineNav.Link selected>Item 1</UnderlineNav.Link>
-  <UnderlineNav.Link>Item 2</UnderlineNav.Link>
+  <UnderlineNav.Item selected>Item 1</UnderlineNav.Item>
+  <UnderlineNav.Item>Item 2</UnderlineNav.Item>
 </UnderlineNav>
 ```
 
@@ -65,7 +65,7 @@ description: Use an underlined nav to allow tab like navigation with overflow be
   <PropsTableSxRow />
 </PropsTable>
 
-### UnderlineNav.Link
+### UnderlineNav.Item
 
 <PropsTable>
   <PropsTableRow name="leadingIcon" type="Component" description="The leading icon comes before item label" />

diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx
@@ -2,15 +2,15 @@ import React from 'react'
 import '@testing-library/jest-dom/extend-expect'
 import {render} from '@testing-library/react'
 
-import UnderlineNav from '.'
+import {UnderlineNav} from '.'
 
 describe('UnderlineNav', () => {
   test('selected nav', () => {
     const {getByText} = render(
       <UnderlineNav label="Test nav">
-        <UnderlineNav.Link selected>Item 1</UnderlineNav.Link>
-        <UnderlineNav.Link>Item 2</UnderlineNav.Link>
-        <UnderlineNav.Link>Item 3</UnderlineNav.Link>
+        <UnderlineNav.Item selected>Item 1</UnderlineNav.Item>
+        <UnderlineNav.Item>Item 2</UnderlineNav.Item>
+        <UnderlineNav.Item>Item 3</UnderlineNav.Item>
       </UnderlineNav>
     )
     const selectedNavLink = getByText('Item 1').closest('a')
@@ -20,9 +20,9 @@ describe('UnderlineNav', () => {
   test('basic nav functionality', () => {
     const {container} = render(
       <UnderlineNav label="Test nav">
-        <UnderlineNav.Link selected>Item 1</UnderlineNav.Link>
-        <UnderlineNav.Link>Item 2</UnderlineNav.Link>
-        <UnderlineNav.Link>Item 3</UnderlineNav.Link>
+        <UnderlineNav.Item selected>Item 1</UnderlineNav.Item>
+        <UnderlineNav.Item>Item 2</UnderlineNav.Item>
+        <UnderlineNav.Item>Item 3</UnderlineNav.Item>
       </UnderlineNav>
     )
     expect(container.getElementsByTagName('nav').length).toEqual(1)
@@ -33,9 +33,9 @@ describe('UnderlineNav', () => {
   test('respect align prop', () => {
     const {container} = render(
       <UnderlineNav label="Test nav" align="right">
-        <UnderlineNav.Link selected>Item 1</UnderlineNav.Link>
-        <UnderlineNav.Link>Item 2</UnderlineNav.Link>
-        <UnderlineNav.Link>Item 3</UnderlineNav.Link>
+        <UnderlineNav.Item selected>Item 1</UnderlineNav.Item>
+        <UnderlineNav.Item>Item 2</UnderlineNav.Item>
+        <UnderlineNav.Item>Item 3</UnderlineNav.Item>
       </UnderlineNav>
     )
     const nav = container.getElementsByTagName('nav')[0]

diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx
@@ -5,6 +5,8 @@ import {UnderlineNavContext} from './UnderlineNavContext'
 import {ActionMenu} from '../ActionMenu'
 import {ActionList} from '../ActionList'
 import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver'
+import {useFocusZone} from '../hooks/useFocusZone'
+import {FocusKeys} from '@primer/behaviors'
 
 type Overflow = 'auto' | 'menu' | 'scroll'
 type ChildWidthArray = Array<{width: number}>
@@ -84,6 +86,16 @@ export const UnderlineNav = forwardRef(
   ) => {
     const backupRef = useRef<HTMLElement>(null)
     const newRef = (forwardedRef ?? backupRef) as MutableRefObject<HTMLElement>
+
+    // This might change if we decide tab through the navigation items rather than navigationg with the arrow keys.
+    // TBD. In the meantime keeping it as a menu with the focus trap.
+    // ref: https://www.w3.org/WAI/ARIA/apg/example-index/menubar/menubar-navigation.html (Keyboard Support)
+    useFocusZone({
+      containerRef: backupRef,
+      bindKeys: FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd,
+      focusOutBehavior: 'wrap'
+    })
+
     const styles = {
       display: 'flex',
       justifyContent: align === 'right' ? 'flex-end' : 'space-between',
@@ -98,7 +110,8 @@ export const UnderlineNav = forwardRef(
       listStyle: 'none',
       padding: '0',
       margin: '0',
-      marginBottom: '-1px'
+      marginBottom: '-1px',
+      alignItems: 'center'
     }
 
     const [selectedLink, setSelectedLink] = useState<RefObject<HTMLElement> | undefined>(undefined)
@@ -143,14 +156,15 @@ export const UnderlineNav = forwardRef(
       <UnderlineNavContext.Provider
         value={{setChildrenWidth, selectedLink, setSelectedLink, afterSelect: afterSelectHandler, variant}}
       >
-        <Box as={as} sx={merge(styles, sxProp)} aria-label={label} ref={newRef}>
+        <Box tabIndex={0} as={as} sx={merge(styles, sxProp)} aria-label={label} ref={newRef}>
           <Box as="ul" sx={merge<BetterSystemStyleObject>(overflowStyles, ulStyles)}>
             {responsiveProps.items}
           </Box>
 
           {actions.length > 0 && (
             <ActionMenu>
-              <ActionMenu.Button>More</ActionMenu.Button>
+              {/* set margin 0 here because safari puts extra margin around the button */}
+              <ActionMenu.Button sx={{m: 0}}>More</ActionMenu.Button>
               <ActionMenu.Overlay>
                 <ActionList>
                   {actions.map((action, index) => {

diff --git a/src/UnderlineNav2/UnderlineNavLink.tsx → src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavLink.tsx → src/UnderlineNav2/UnderlineNavItem.tsx
@@ -4,6 +4,8 @@ import {merge, SxProp, BetterSystemStyleObject} from '../sx'
 import {IconProps} from '@primer/octicons-react'
 import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import {UnderlineNavContext} from './UnderlineNavContext'
+import CounterLabel from '../CounterLabel'
+import {Theme, useTheme} from '../ThemeProvider'
 
 // adopted from React.AnchorHTMLAttributes
 type LinkProps = {
@@ -18,7 +20,7 @@ type LinkProps = {
   referrerPolicy?: React.AnchorHTMLAttributes<HTMLAnchorElement>['referrerPolicy']
 }
 
-export type UnderlineNavLinkProps = {
+export type UnderlineNavItemProps = {
   /**
    * Primary content for an NavLink
    */
@@ -36,16 +38,21 @@ export type UnderlineNavLinkProps = {
    */
   leadingIcon?: React.FunctionComponent<IconProps>
   as?: React.ElementType
+  /**
+   * Counter
+   */
+  counter?: number
 } & SxProp &
   LinkProps
 
-export const UnderlineNavLink = forwardRef(
+export const UnderlineNavItem = forwardRef(
   (
     {
       sx: sxProp = {},
       as: Component = 'a',
       href = '#',
       children,
+      counter,
       onSelect,
       selected: preSelected = false,
       leadingIcon: LeadingIcon,
@@ -56,11 +63,13 @@ export const UnderlineNavLink = forwardRef(
     const backupRef = useRef<HTMLElement>(null)
     const ref = forwardedRef ?? backupRef
     const {setChildrenWidth, selectedLink, setSelectedLink, afterSelect, variant} = useContext(UnderlineNavContext)
+    const {theme} = useTheme()
     useLayoutEffect(() => {
       const domRect = (ref as MutableRefObject<HTMLElement>).current.getBoundingClientRect()
       setChildrenWidth({width: domRect.width})
       preSelected && selectedLink === undefined && setSelectedLink(ref as RefObject<HTMLElement>)
     }, [ref, preSelected, selectedLink, setSelectedLink, setChildrenWidth])
+
     const iconWrapStyles = {
       alignItems: 'center',
       display: 'inline-flex',
@@ -70,29 +79,72 @@ export const UnderlineNavLink = forwardRef(
     const textStyles: BetterSystemStyleObject = {
       whiteSpace: 'nowrap'
     }
+
+    const wrapperStyles = {
+      display: 'inline-flex',
+      paddingY: 1,
+      paddingX: 2,
+      borderRadius: 2
+    }
     const smallVariantLinkStyles = {
-      paddingY: 2,
+      paddingY: 1,
       fontSize: 0
     }
     const defaultVariantLinkStyles = {
-      paddingY: 3,
+      paddingY: 2,
       fontSize: 1
     }
 
-    const linkStyles = {
+    // eslint-disable-next-line no-shadow
+    const linkStyles = (theme?: Theme) => ({
+      position: 'relative',
       display: 'inline-flex',
       color: 'fg.default',
       textAlign: 'center',
-      borderBottom: '2px solid transparent',
-      borderColor: selectedLink === ref ? 'primer.border.active' : 'transparent',
       textDecoration: 'none',
-      paddingX: 2,
-      marginRight: 3,
+      paddingX: 1,
       ...(variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles),
-      '&:hover, &:focus': {
-        borderColor: selectedLink === ref ? 'primer.border.active' : 'neutral.muted',
-        transition: '0.2s ease'
+      '&:hover > div[data-component="wrapper"] ': {
+        backgroundColor: theme?.colors.neutral.muted,
+        transition: 'background .12s ease-out'
+      },
+      '&:focus': {
+        outline: 0,
+        '& > div[data-component="wrapper"]': {
+          boxShadow: `inset 0 0 0 2px ${theme?.colors.accent.fg}`
+        },
+        // where focus-visible is supported, remove the focus box-shadow
+        '&:not(:focus-visible) > div[data-component="wrapper"]': {
+          boxShadow: 'none'
+        }
+      },
+      '&:focus-visible > div[data-component="wrapper"]': {
+        boxShadow: `inset 0 0 0 2px ${theme?.colors.accent.fg}`
+      },
+      // renders a visibly hidden "copy" of the label in bold, reserving box space for when label becomes bold on selected
+      '& span[data-content]::before': {
+        content: 'attr(data-content)',
+        display: 'block',
+        height: 0,
+        fontWeight: '600',
+        visibility: 'hidden'
+      },
+      // selected state styles
+      '&::after': {
+        position: 'absolute',
+        right: '50%',
+        bottom: 0,
+        width: `calc(100% - 8px)`,
+        height: 2,
+        content: '""',
+        bg: selectedLink === ref ? theme?.colors.primer.border.active : 'transparent',
+        borderRadius: 0,
+        transform: 'translate(50%, -50%)'
       }
+    })
+
+    const counterStyles = {
+      marginLeft: 2
     }
     const keyPressHandler = React.useCallback(
       event => {
@@ -118,29 +170,41 @@ export const UnderlineNavLink = forwardRef(
       [onSelect, afterSelect, ref, setSelectedLink]
     )
     return (
-      <Box as="li">
+      <Box as="li" sx={{display: 'flex', flexDirection: 'column', alignItems: 'center'}}>
         <Box
           as={Component}
           href={href}
           onKeyPress={keyPressHandler}
           onClick={clickHandler}
           {...(selectedLink === ref ? {'aria-current': 'page'} : {})}
-          sx={merge(linkStyles, sxProp as SxProp)}
+          sx={merge(linkStyles(theme), sxProp as SxProp)}
           {...props}
           ref={ref}
         >
-          {LeadingIcon && (
-            <Box as="span" data-component="leadingIcon" sx={iconWrapStyles}>
-              <LeadingIcon />
-            </Box>
-          )}
-          {children && (
-            <Box as="span" data-component="text" sx={textStyles}>
-              {children}
-            </Box>
-          )}
+          <Box as="div" data-component="wrapper" sx={wrapperStyles}>
+            {LeadingIcon && (
+              <Box as="span" data-component="leadingIcon" sx={iconWrapStyles}>
+                <LeadingIcon />
+              </Box>
+            )}
+            {children && (
+              <Box
+                as="span"
+                data-component="text"
+                data-content={children}
+                sx={selectedLink === ref ? {fontWeight: 600, ...{textStyles}} : {textStyles}}
+              >
+                {children}
+              </Box>
+            )}
+            {counter && (
+              <Box as="span" data-component="counter" sx={counterStyles}>
+                <CounterLabel>{counter}</CounterLabel>
+              </Box>
+            )}
+          </Box>
         </Box>
       </Box>
     )
   }
-) as PolymorphicForwardRefComponent<'a', UnderlineNavLinkProps>
+) as PolymorphicForwardRefComponent<'a', UnderlineNavItemProps>