fix(NavList): Properly style Trees within NavList (#2338)

Also addresses issue where non-StyledIcon wouldn't receive proper gap within ListItem

Author: Luke Bowerman

packages/components/src/Form/Inputs/Select/SelectOptions.tsx

@@ -109,7 +109,11 @@ const OptionLayout = ({ option, ...rest }: OptionLayoutProps) => {
   const { hasIcons } = useContext(SelectOptionsContext)
   const { indicatorPropRef } = useContext(ComboboxContext)
   const iconPlaceholder = hasIcons ? (
-    <IconPlaceholder size="small" data-testid="option-icon-placeholder" />
+    <IconPlaceholder
+      mr="xsmall"
+      size="small"
+      data-testid="option-icon-placeholder"
+    />
   ) : undefined
 
   const indicator = option.icon ? (

packages/components/src/Icon/IconPlaceholder.tsx

@@ -38,12 +38,9 @@ export interface IconPlaceholderProps
     SizeProps,
     SpaceProps {}
 
-export const IconPlaceholder = styled.div.attrs<IconPlaceholderProps>(
-  ({ mr = 'xsmall' }) => ({
-    'aria-hidden': true,
-    mr,
-  })
-)<IconPlaceholderProps>`
+export const IconPlaceholder = styled.div.attrs<IconPlaceholderProps>(() => ({
+  'aria-hidden': true,
+}))<IconPlaceholderProps>`
   ${size}
   ${space}
 `

packages/components/src/List/ListItemLayout.tsx

@@ -56,7 +56,7 @@ const ListItemLayoutInternal: FC<ListItemLayoutProps> = ({
 }) => {
   const content = (
     <>
-      {icon}
+      {icon && <ListItemIconPlacement>{icon}</ListItemIconPlacement>}
       <Flex flexDirection="column" minWidth={0} flexGrow={1}>
         {children}
         {description}
@@ -71,16 +71,24 @@ const ListItemLayoutInternal: FC<ListItemLayoutProps> = ({
   })
 }
 
+export const ListItemIconPlacement = styled.div``
+
 export const listItemIconCSS = css<ListItemLayoutProps>`
-  & > svg,
-  & > ${StyledIconBase}, & > ${IconPlaceholder} {
-    ${colorHelper}
+  ${ListItemIconPlacement} {
+    /* align-items: center; */
     align-self: center;
-    flex-grow: 0;
-    flex-shrink: 0;
-    height: ${({ iconSize, theme }) => theme.sizes[iconSize]};
+    display: flex;
     margin-right: ${({ iconGap, theme }) => theme.space[iconGap]};
-    width: ${({ iconSize, theme }) => theme.sizes[iconSize]};
+
+    & > svg,
+    ${StyledIconBase}, ${IconPlaceholder} {
+      ${colorHelper}
+      flex-grow: 0;
+      flex-shrink: 0;
+      height: ${({ iconSize, theme }) => theme.sizes[iconSize]};
+      /* justify-content: center; */
+      width: ${({ iconSize, theme }) => theme.sizes[iconSize]};
+    }
   }
 `
 

packages/components/src/List/ListItemLayoutAccessory.tsx

@@ -27,7 +27,11 @@
 import React, { FC } from 'react'
 import styled from 'styled-components'
 import { Flex } from '../Layout'
-import { listItemIconCSS, ListItemLayoutProps } from './ListItemLayout'
+import {
+  listItemIconCSS,
+  ListItemIconPlacement,
+  ListItemLayoutProps,
+} from './ListItemLayout'
 import { listItemPadding } from './utils'
 
 export const ListItemLayoutAccessoryInternal: FC<ListItemLayoutProps> = ({
@@ -40,7 +44,7 @@ export const ListItemLayoutAccessoryInternal: FC<ListItemLayoutProps> = ({
 }) => {
   const content = (
     <>
-      {icon}
+      {icon && <ListItemIconPlacement>{icon}</ListItemIconPlacement>}
       <Flex flexDirection="column" minWidth={0} flexGrow={1}>
         {children}
         {description}

packages/components/src/List/ListItemWrapper.tsx

@@ -29,6 +29,7 @@ import { StyledIconBase } from '@styled-icons/styled-icon'
 import omit from 'lodash/omit'
 import React, { forwardRef, ReactNode, Ref } from 'react'
 import styled from 'styled-components'
+import { ListItemIconPlacement } from './ListItemLayout'
 import { ListColor, ListItemDimensions, listItemDimensionKeys } from './types'
 
 export interface ListItemWrapperProps
@@ -83,10 +84,14 @@ export const ListItemWrapper = styled(ListItemWrapperInternal)
       }
     `}
 
-  ${StyledIconBase}, svg {
+  ${ListItemIconPlacement} {
     align-self: ${({ description }) => (description ? 'flex-start' : 'center')};
-    transition: color
-      ${({ theme }) => `${theme.transitions.quick}ms ${theme.easings.ease}`};
+
+    ${StyledIconBase},
+    svg {
+      transition: color ${({ theme }) =>
+        `${theme.transitions.quick}ms ${theme.easings.ease}`};
+    }
   }
 
   &[aria-current='true'] {

packages/components/src/NavList/NavList.story.tsx

@@ -26,33 +26,69 @@
 
 import React, { useState } from 'react'
 import { Home } from '@styled-icons/material-outlined/Home'
-import { DateRange } from '@styled-icons/material-outlined/DateRange'
+import { Info } from '@styled-icons/material-outlined/Info'
 import { ListItem } from '../List/ListItem'
+import { ProgressCircular } from '../ProgressCircular'
+import { Tree, TreeItem } from '../Tree'
 import { NavList } from './NavList'
 
 export default {
   component: NavList,
   title: 'NavList',
 }
 
-export const NavListExample = () => {
+export const Basic = () => {
   const [selected, setSelected] = useState(false)
   const handleClick = () => {
     setSelected(!selected)
   }
   return (
     <NavList>
       <ListItem
-        description="Orange-y"
-        detail="Netherlands"
+        description="Description"
+        detail="Interesting details"
         icon={<Home />}
         selected
       >
         Explore
       </ListItem>
-      <ListItem icon={<DateRange />} onClick={handleClick} selected={selected}>
+      <ListItem icon={<Info />} onClick={handleClick} selected={selected}>
         Develop
       </ListItem>
     </NavList>
   )
 }
+
+export const MixedNavigation = () => (
+  <NavList>
+    <ListItem icon={<Home />} selected>
+      Home
+    </ListItem>
+    <ListItem icon={<Home />}>Not really home</ListItem>
+    <Tree icon={<Info />} label="Tree" selected isOpen>
+      <TreeItem icon={<Info />}>Meh</TreeItem>
+      <TreeItem
+        description="description"
+        detail="detail"
+        icon={<Info />}
+        selected
+      >
+        My Awesome Item
+      </TreeItem>
+      <Tree
+        forceLabelPadding
+        branchFontWeight
+        defaultOpen
+        label="Blah"
+        icon={<Info />}
+      >
+        <TreeItem color="text2">
+          <em>Not yet available</em>
+        </TreeItem>
+        <TreeItem icon={<ProgressCircular size="xsmall" progress={0.75} />}>
+          Loading...
+        </TreeItem>
+      </Tree>
+    </Tree>
+  </NavList>
+)

packages/components/src/NavList/NavList.test.tsx

@@ -25,36 +25,19 @@
  */
 
 import React from 'react'
-import { Home } from '@styled-icons/material/Home'
 import { renderWithTheme } from '@looker/components-test-utils'
 import { screen } from '@testing-library/react'
-import { ListItem } from '../List/ListItem'
-import { NavList } from './NavList'
+import { Basic } from './NavList.story'
 
 describe('NavList', () => {
   test('display with keyColor', () => {
-    renderWithTheme(
-      <NavList>
-        <ListItem
-          description="Orange-y"
-          detail="Netherlands"
-          icon={<Home />}
-          selected
-        >
-          Explore
-        </ListItem>
-      </NavList>
-    )
+    renderWithTheme(<Basic />)
 
     const listItem = screen.getByText('Explore')
-
-    // NavList label
-    expect(listItem).toHaveStyle(`color: #6C43E0;`)
-
-    // NavList description
-    expect(screen.getByText('Orange-y')).toHaveStyle(`color: #6C43E0;`)
-
-    // NavList detail
-    expect(screen.getByText('Netherlands')).toHaveStyle(`color: #6C43E0;`)
+    expect(listItem).toHaveStyle('color: #6c43e0;')
+    expect(screen.getByText('Interesting details')).toHaveStyle(
+      'color: #6c43e0'
+    )
+    expect(screen.getByText('Description')).toHaveStyle('color: #6c43e0;')
   })
 })

packages/components/src/NavList/NavList.tsx

@@ -25,18 +25,19 @@
  */
 
 import styled from 'styled-components'
-import { StyledIconBase } from '@styled-icons/styled-icon'
 import { AccordionDisclosureStyle } from '../Accordion/AccordionDisclosure'
 import { TextBase } from '../Text/TextBase'
 import { ListItemDetail } from '../List/ListItemDetail'
-import { List, ListItemLabel } from '../List'
+import { List, ListItem, ListItemLabel } from '../List'
+import { ListItemIconPlacement } from '../List/ListItemLayout'
 
 /**
  * `NavList` is a variation of `List`
  *   - `ListItem`  border-radius circular on the right side
  *   - `ListItem` selected or "active"
  *     - text color is `keyColor`
  *     - background color is `keySubtle`
+ *   - `ListItem` at the root are indented to align properly with `Tree`(s) at the root as well
  *
  *
  * @status: EXPERIMENTAL
@@ -45,18 +46,22 @@ import { List, ListItemLabel } from '../List'
  * SemVer major version change. _It is not recommended to use this component
  * at this time.
  */
-
-export const NavList = styled(List)`
+export const NavList = styled(List).attrs(({ color = 'key' }) => ({ color }))`
   ${AccordionDisclosureStyle}, ${ListItemLabel} {
     border-bottom-right-radius: 5rem;
     border-top-right-radius: 5rem;
 
     &[aria-selected='true'] {
-      background-color: ${({ theme }) => theme.colors.keySubtle};
-      color: ${({ theme }) => theme.colors.key};
-      ${ListItemDetail}, ${StyledIconBase}, ${TextBase} {
+      ${ListItemDetail},
+      ${TextBase},
+      ${ListItemIconPlacement} svg {
         color: ${({ theme }) => theme.colors.key};
       }
     }
   }
+  & > ${ListItem} {
+    ${ListItemLabel} {
+      padding-left: ${({ theme }) => `${theme.sizes.medium}`};
+    }
+  }
 `

packages/components/src/NavList/index.ts

@@ -24,4 +24,4 @@
 
  */
 
-export * from './NavList'
+export { NavList } from './NavList'

packages/components/src/Tree/Tree.tsx

@@ -36,6 +36,7 @@ import React, {
 import { Accordion } from '../Accordion'
 import { undefinedCoalesce, useWrapEvent } from '../utils'
 import { List } from '../List'
+import { ListItemContext } from '../List/ListItemContext'
 import { listItemDimensions, getDetailOptions } from '../List/utils'
 import { TreeContext } from './TreeContext'
 import { indicatorDefaults } from './utils'
@@ -69,11 +70,12 @@ const TreeLayout: FC<TreeProps> = ({
   const detailRef = useRef<HTMLDivElement>(null)
   const [hovered, setHovered] = useState(false)
 
+  const { color: listColor } = useContext(ListItemContext)
   const treeContext = useContext(TreeContext)
   const hasBorder = undefinedCoalesce([propsBorder, treeContext.border])
 
   if (keyColor) propsColor = 'key'
-  const color = undefinedCoalesce([propsColor, treeContext.color])
+  const color = undefinedCoalesce([propsColor, treeContext.color, listColor])
 
   const hasLabelBackgroundOnly = undefinedCoalesce([
     propsLabelBackgroundOnly,