address PR feedback around memoization and styling

packages/react/src/SubNav/SubNav.features.stories.tsx

@@ -12,6 +12,7 @@ import {Text} from '../Text'
 import {RedlineBackground} from '../component-helpers'
 import {Stack} from '../Stack'
 import {expect, userEvent, within} from '@storybook/test'
+import {Button} from '../Button'
 
 export default {
   title: 'Components/SubNav/Features',
@@ -189,6 +190,7 @@ export const AnchorNavVariant = args => (
           <Stack key={value} id={value} direction="vertical" style={{justifyContent: 'center', height: 1000}}>
             <Heading>{key}</Heading>
             <Text as="p">SubNav is a component that allows users to navigate to different sections of a page.</Text>
+            <Button>Learn more</Button>
           </Stack>
         </RedlineBackground>
       ))}

packages/react/src/SubNav/SubNav.module.css

@@ -282,6 +282,11 @@
     padding: var(--base-size-8) 0;
     animation: fade-in 0.3s var(--brand-animation-easing-glide) forwards;
   }
+
+  .SubNav__links-overlay--open .SubNav__link--has-sub-menu {
+    padding-block: 0;
+  }
+
   .SubNav__links-overlay--open .SubNav__action-container {
     width: 100%;
   }

packages/react/src/SubNav/SubNav.module.css.d.ts

@@ -26,13 +26,13 @@ declare const styles: {
   readonly "SubNav__links-overlay--open": string;
   readonly "SubNav__overlay-toggle": string;
   readonly "SubNav__overlay-toggle-content": string;
+  readonly "SubNav__link--has-sub-menu": string;
   readonly "SubNav__action": string;
   readonly "SubNav__sub-menu": string;
   readonly "SubNav__sub-menu--dropdown": string;
   readonly "SubNav__anchor-menu-container": string;
   readonly "SubNav__overlay-toggle-icon": string;
   readonly "SubNav__heading-label": string;
-  readonly "SubNav__link--has-sub-menu": string;
   readonly "SubNav__link--expanded": string;
   readonly "fade-in-down": string;
 };

packages/react/src/SubNav/SubNav.tsx

@@ -139,7 +139,7 @@ const _SubNavRoot = memo(({id, children, className, 'data-testid': testId, fullW
 
   const {isLarge} = useWindowSize()
 
-  const memoizedChildren = React.useMemo(() => Children.toArray(children), [children])
+  const childrenArr = Children.toArray(children)
 
   const closeMenuCallback = useCallback(() => {
     if (isLarge) return
@@ -163,15 +163,15 @@ const _SubNavRoot = memo(({id, children, className, 'data-testid': testId, fullW
     }
   }, [isOpenAtNarrow, isLarge])
 
-  const activeLink = memoizedChildren.find(child => {
+  const activeLink = childrenArr.find(child => {
     if (isValidElement(child)) {
       return child.props['aria-current']
     }
   }) as React.ReactElement | undefined
 
   useEffect(() => {
     // check if there is an anchored nav in the SubNav.SubMenu child
-    const hasAnchorVariant = memoizedChildren.some(child => {
+    const hasAnchorVariant = childrenArr.some(child => {
       if (isValidElement(child) && child.type === SubNavLink) {
         const [, subMenu] = child.props.children
         if (subMenu?.props?.variant === 'anchor') {
@@ -180,45 +180,41 @@ const _SubNavRoot = memo(({id, children, className, 'data-testid': testId, fullW
       }
     })
     setHasAnchoredNav(hasAnchorVariant)
-  }, [memoizedChildren])
+  }, [childrenArr])
 
   const {
     heading: HeadingChild,
     links: LinkChildren,
     action: ActionChild,
-  } = useMemo(
-    () =>
-      memoizedChildren.reduce(
-        (acc: {heading?: ReactNode; links: ReactElement[]; action?: ReactNode}, child) => {
-          if (isValidElement(child)) {
-            if (child.type === SubNavHeading) {
-              acc.heading = child
-            } else if (child.type === SubNavLink) {
-              const [link, subMenu] = child.props.children
-
-              if (subMenu?.props?.variant === 'anchor') {
-                acc.links.push(
-                  React.cloneElement(child as ReactElement<SubNavLinkProps>, {
-                    children: [link],
-                    onClick: child.props['aria-current'] ? closeMenuCallback : child.props.onClick,
-                  }),
-                )
-              } else {
-                acc.links.push(
-                  React.cloneElement(child as ReactElement<SubNavLinkProps>, {
-                    onClick: child.props['aria-current'] ? closeMenuCallback : child.props.onClick,
-                  }),
-                )
-              }
-            } else if (child.type === _SubNavAction) {
-              acc.action = child
-            }
+  } = childrenArr.reduce(
+    (acc: {heading?: ReactNode; links: ReactElement[]; action?: ReactNode}, child) => {
+      if (isValidElement(child)) {
+        if (child.type === SubNavHeading) {
+          acc.heading = child
+        } else if (child.type === SubNavLink) {
+          const [link, subMenu] = child.props.children
+
+          if (subMenu?.props?.variant === 'anchor') {
+            acc.links.push(
+              React.cloneElement(child as ReactElement<SubNavLinkProps>, {
+                children: [link],
+                onClick: child.props['aria-current'] ? closeMenuCallback : child.props.onClick,
+              }),
+            )
+          } else {
+            acc.links.push(
+              React.cloneElement(child as ReactElement<SubNavLinkProps>, {
+                onClick: child.props['aria-current'] ? closeMenuCallback : child.props.onClick,
+              }),
+            )
           }
-          return acc
-        },
-        {heading: undefined, links: [], action: undefined},
-      ),
-    [memoizedChildren, closeMenuCallback],
+        } else if (child.type === _SubNavAction) {
+          acc.action = child
+        }
+      }
+      return acc
+    },
+    {heading: undefined, links: [], action: undefined},
   )
 
   // The values are different types depending on whether a submenu is present
@@ -354,10 +350,8 @@ const SubNavLinkWithSubmenu = forwardRef<HTMLDivElement, SubNavLinkProps>(
 
     const [label, SubMenuChildren] = children as ReactNode[]
 
-    const onKeyDown = useCallback((e: React.KeyboardEvent<HTMLButtonElement>) => {
-      if (['Enter', ' '].includes(e.key)) {
-        setIsExpanded(prev => !prev)
-      }
+    const handleOnClick = useCallback(() => {
+      setIsExpanded(prev => !prev)
     }, [])
 
     return (
@@ -396,7 +390,7 @@ const SubNavLinkWithSubmenu = forwardRef<HTMLDivElement, SubNavLinkProps>(
         {isLarge && (
           <button
             className={styles['SubNav__sub-menu-toggle']}
-            onKeyDown={onKeyDown}
+            onClick={handleOnClick}
             aria-expanded={isExpanded ? 'true' : 'false'}
             aria-controls={submenuId}
             aria-label={`${isExpanded ? 'Close' : 'Open'} submenu`}
@@ -415,17 +409,13 @@ const SubNavLinkWithSubmenu = forwardRef<HTMLDivElement, SubNavLinkProps>(
 
 const SubNavLink = forwardRef<HTMLAnchorElement | HTMLDivElement, SubNavLinkProps>((props, ref) => {
   const [isInView, setIsInView] = useState(false)
-  const memoizedChildren = React.useMemo(() => Children.toArray(props.children), [props.children])
+  const childrenArr = Children.toArray(props.children)
 
-  const hasSubMenu = useMemo(
-    () =>
-      memoizedChildren.some(child => {
-        if (isValidElement(child)) {
-          return child.type === _SubMenu
-        }
-      }),
-    [memoizedChildren],
-  )
+  const hasSubMenu = childrenArr.some(child => {
+    if (isValidElement(child)) {
+      return child.type === _SubMenu
+    }
+  })
 
   useEffect(() => {
     if (hasSubMenu) return
@@ -446,7 +436,7 @@ const SubNavLink = forwardRef<HTMLAnchorElement | HTMLDivElement, SubNavLinkProp
   }, [hasSubMenu, props.href])
 
   if (hasSubMenu) {
-    const isAnchorVariantSubMenu = memoizedChildren.some(child => {
+    const isAnchorVariantSubMenu = childrenArr.some(child => {
       if (isValidElement(child)) {
         return child.type === _SubMenu && child.props.variant === 'anchor'
       }
@@ -541,14 +531,6 @@ function _SubMenu({children, className, variant = 'dropdown', ...props}: SubMenu
             if (isValidElement(child)) {
               return React.cloneElement(child as React.ReactElement<SubNavLinkProps>, {
                 onClick: e => {
-                  e.preventDefault()
-                  e.stopPropagation()
-
-                  if (!child.props.href.startsWith('#')) return
-                  const anchor = document.querySelector((child.props as SubNavLinkProps).href)
-                  if (anchor) {
-                    anchor.scrollIntoView({behavior: 'smooth'})
-                  }
                   if (child.props.onClick) {
                     child.props.onClick(e)
                   }