Tree: More style updates and bug fixes (#1196)

* Allow TreeGroup color prop to override default color of child Trees and TreeItems

* Added color override test case to TreeGroup

* Adjusted Tree indicator gap size to align with TreeItem icon gap

* Removed negative margin from ButtonIcon margin calc function

* Removed key color from pivot button in FieldPicker exampel

* Fixed color override from TreeGroup bug

* Updated CHANGELOG

* Fixed color test on TreeGroup

* Correct Button icon positioning

* Removed && pattern

* Ship it. Ship it real good

* Added color prop to TreeItem

Co-authored-by: Luke Bowerman <[email protected]>
Co-authored-by: Luke Bowerman <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `FactCheck` icon
 - `TreeItem` and `Tree`
   - Altered style defaults
+- `TreeGroup`
+  - Added additional test case for color override behavior
 - `iconSizes` style function includes `xxsmall` case
 
 ### Changed
@@ -37,15 +39,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `MessageBar`
   - MessageBar is dismissable by default. Use prop `noActions` in place of `canDismiss={false}` to hide dismiss button.
   - Adds new `visible` prop to toggle display externally
+- `Tree` increased gap size between indicator and label to align with sibling `TreeItem`s with icons
+- `ButtonBase` removed negative margin from `iconMargins` helper function
   - Accepts primary and secondary action overrides and callbacks
 
 ### Fixed
 
 - `Select`/`SelectMulti` keyboard navigation when filtering and going from > 100 to < 100 options
 - `SelectMulti` with `freeInput` not saving input value on tab key
 - `SelectMulti` list not closing on blur
+- `TreeGroup` properly overrides color of child `TreeItem` labels and `Tree` labels
 - `MenuGroup` now includes icon placeholder spacing if higher `MenuItemContext` has `renderIconPlaceholder === true`
 
+
 ### Removed
 
 - Removed `Modal*` aliases to `Dialog*`

packages/components/src/Button/icon.ts

@@ -36,31 +36,32 @@ export interface ButtonIconProps {
 }
 
 export const iconMargins = (props: ButtonProps) => {
-  const spacing = { large: 0, small: 0 }
+  const spacing = { inner: 0, outer: 0 }
   switch (props.size) {
+    case 'xxsmall':
     case 'xsmall':
-      spacing.small = 4
-      spacing.large = 6
+      spacing.outer = 4
+      spacing.inner = 6
       break
     case 'small':
-      spacing.small = 8
-      spacing.large = 8
+      spacing.outer = 8
+      spacing.inner = 8
       break
     case 'large':
     default:
-      spacing.small = 12
-      spacing.large = 8
+      spacing.outer = 12
+      spacing.inner = 8
   }
 
   if (props.iconBefore) {
     return css`
-      margin-left: -${rem(spacing.small)};
-      margin-right: ${rem(spacing.large)};
+      margin-left: -${rem(spacing.outer)};
+      margin-right: ${rem(spacing.inner)};
     `
   } else if (props.iconAfter) {
     return css`
-      margin-left: ${rem(spacing.large)};
-      margin-right: -${rem(spacing.small)};
+      margin-left: ${rem(spacing.inner)};
+      margin-right: -${rem(spacing.outer)};
     `
   } else {
     return false

packages/components/src/Tree/Tree.tsx

@@ -81,7 +81,7 @@ export interface TreeProps extends AccordionProps {
 }
 
 const indicatorProps: AccordionIndicatorProps = {
-  indicatorGap: 'xxsmall',
+  indicatorGap: 'xsmall',
   indicatorIcons: { close: 'ArrowRight', open: 'ArrowDown' },
   indicatorPosition: 'left',
   indicatorSize: 'small',
@@ -97,6 +97,7 @@ const TreeLayout: FC<TreeProps> = ({
   detailAccessory: propsDetailAccessory,
   icon,
   label,
+  className,
   visuallyAsBranch,
   ...restProps
 }) => {
@@ -148,25 +149,28 @@ const TreeLayout: FC<TreeProps> = ({
         detailHoverDisclosure: hasDetailHoverDisclosure,
       }}
     >
-      <TreeStyle border={hasBorder} depth={depth} hovered={isHovered}>
+      <TreeStyle
+        className={className}
+        border={hasBorder}
+        depth={depth}
+        hovered={isHovered}
+      >
         {innerAccordion}
       </TreeStyle>
     </TreeContext.Provider>
   )
 }
 
-export const Tree = styled(TreeLayout)``
-
 const generateTreeBorder = (depth: number, theme: Theme) => {
   const {
     colors,
-    space: { xxsmall, small },
+    space: { xxsmall, xsmall, small },
   } = theme
 
   const itemBorderSize = '1px'
   const itemPaddingSize = xxsmall
   const indicatorIconSize = small
-  const indicatorGapSize = xxsmall
+  const indicatorGapSize = xsmall
   const depthSize = `${itemBorderSize} + ${itemPaddingSize} + (${indicatorIconSize} + ${indicatorGapSize}) * ${depth}`
   const borderSpacer = `(${small} / 2) + ${depthSize}`
 
@@ -182,12 +186,12 @@ const generateTreeBorder = (depth: number, theme: Theme) => {
 
 const generateIndent = (depth: number, theme: Theme) => {
   const {
-    space: { xxsmall, small },
+    space: { xxsmall, xsmall, small },
   } = theme
 
   const itemPaddingSize = xxsmall
   const indicatorIconSize = small
-  const indicatorGapSize = xxsmall
+  const indicatorGapSize = xsmall
   const indentCalculation = `${itemPaddingSize} + (${indicatorIconSize} + ${indicatorGapSize}) * ${depth}`
 
   return css`
@@ -202,6 +206,8 @@ interface TreeStyleProps {
 }
 
 export const TreeStyle = styled.div<TreeStyleProps>`
+  color: ${({ theme }) => theme.colors.text2};
+
   & > ${Accordion} {
     & > ${AccordionContent} {
       ${({ border, depth, theme }) =>
@@ -233,3 +239,5 @@ export const TreeStyle = styled.div<TreeStyleProps>`
     ${({ depth, theme }) => generateIndent(depth + 1, theme)}
   }
 `
+
+export const Tree = styled(TreeLayout)``

packages/components/src/Tree/TreeGroup.test.tsx

@@ -26,7 +26,7 @@
 
 import React from 'react'
 import { renderWithTheme } from '@looker/components-test-utils'
-import { TreeGroup } from '.'
+import { TreeGroup, Tree, TreeItem } from '.'
 
 test('Renders label and children', () => {
   const { getByText } = renderWithTheme(
@@ -36,3 +36,19 @@ test('Renders label and children', () => {
   getByText('My Tree Group')
   getByText('My Children')
 })
+
+test('Applies color prop to child Trees and TreeItems', () => {
+  const { getByText } = renderWithTheme(
+    <TreeGroup label="My Tree Group" color="red">
+      <Tree label="My Tree" defaultOpen>
+        <TreeItem>TreeItem 1</TreeItem>
+      </Tree>
+      <TreeItem>TreeItem 2</TreeItem>
+    </TreeGroup>
+  )
+
+  expect(getByText('My Tree Group')).toHaveStyleRule('color: red')
+  expect(getByText('My Tree')).toHaveStyleRule('color: red')
+  expect(getByText('TreeItem 1')).toHaveStyleRule('color: red')
+  expect(getByText('TreeItem 2')).toHaveStyleRule('color: red')
+})

packages/components/src/Tree/TreeGroup.tsx

@@ -27,6 +27,8 @@
 import React, { FC } from 'react'
 import styled from 'styled-components'
 import { color, TextColorProps } from '@looker/design-tokens'
+import { AccordionDisclosure } from '../Accordion'
+import { TreeItemLabel } from './TreeItem'
 
 export interface TreeGroupProps extends TextColorProps {
   /**
@@ -47,15 +49,16 @@ const TreeGroupLayout: FC<TreeGroupProps> = ({
   </div>
 )
 
-export const TreeGroup = styled(TreeGroupLayout)`
-  ${color}
-`
-
 export const TreeGroupLabel = styled.div`
   /* Border is here to get proper alignment with Tree and TreeItem text */
   border: 1px transparent solid;
-  color: ${({ theme }) => theme.colors.text2};
   font-size: ${({ theme }) => theme.fontSizes.xxsmall};
   font-weight: ${({ theme }) => theme.fontWeights.semiBold};
   padding: ${({ theme: { space } }) => `${space.xsmall} ${space.xxsmall}`};
 `
+
+export const TreeGroup = styled(TreeGroupLayout)`
+  ${TreeItemLabel}, ${TreeGroupLabel}, ${AccordionDisclosure} {
+    ${color}
+  }
+`

packages/components/src/Tree/TreeItem.tsx

@@ -36,9 +36,11 @@ import React, {
 } from 'react'
 import styled from 'styled-components'
 import {
+  color,
+  CompatibleHTMLProps,
   SpacingSizes,
+  TextColorProps,
   uiTransparencyBlend,
-  CompatibleHTMLProps,
 } from '@looker/design-tokens'
 import Omit from 'lodash/omit'
 import { Space, FlexItem } from '../Layout'
@@ -51,7 +53,9 @@ import {
 import { undefinedCoalesce } from '../utils'
 import { TreeContext } from './TreeContext'
 
-export interface TreeItemProps extends CompatibleHTMLProps<HTMLDivElement> {
+export interface TreeItemProps
+  extends Omit<CompatibleHTMLProps<HTMLDivElement>, 'color'>,
+    TextColorProps {
   className?: string
   /**
    * Supplementary element that appears right of the TreeItem's label
@@ -103,6 +107,7 @@ const TreeItemLayout: FC<TreeItemProps> = ({
   const [isFocusVisible, setFocusVisible] = useState(false)
 
   const { onBlur, onClick, onKeyDown, onKeyUp, ...restProps } = Omit(props, [
+    'color',
     'detail',
     'detailAccessory',
     'detailHoverDisclosure',
@@ -203,11 +208,11 @@ const PrimaryIcon = styled(Icon)`
   opacity: 0.5;
 `
 
-interface TreeItemSpace {
+interface TreeItemSpaceProps {
   focusVisible: boolean
 }
 
-export const TreeItemSpace = styled(Space)<TreeItemSpace>`
+export const TreeItemSpace = styled(Space)<TreeItemSpaceProps>`
   align-items: center;
   border: 1px solid transparent;
   border-color: ${({ focusVisible, theme }) =>
@@ -241,5 +246,11 @@ const TreeItemDetail = styled.div<{ detailAccessory: boolean }>`
 `
 
 export const TreeItem = styled(TreeItemLayout)`
-  color: ${({ theme }) => theme.colors.text2};
+  /*
+    Note: first-child pseudo-selector is here to give this selector
+    more specificity over TreeGroup.
+  */
+  ${TreeItemLabel}:first-child {
+    ${color}
+  }
 `

storybook/src/Tree/FieldPicker.stories.tsx

@@ -60,7 +60,6 @@ const PickerItem = () => {
     <IconButton
       icon="Sync"
       label="Pivot"
-      color="key"
       tooltipPlacement="top"
       onClick={(event) => {
         event.stopPropagation()
@@ -73,7 +72,7 @@ const PickerItem = () => {
   )
 
   const itemLabel = (
-    <Space between px="xxsmall">
+    <Space between>
       <span>Cost</span>
       {!overlay ? <HoverDisclosure>{pivot}</HoverDisclosure> : pivot}
     </Space>
@@ -126,33 +125,41 @@ const PickerItem = () => {
   )
 }
 
-export const FieldPicker = () => (
-  <Tree
-    defaultOpen
-    detailAccessory
-    detail={
-      <ButtonTransparent
-        size="xxsmall"
-        iconBefore="Plus"
-        onClick={() => alert('Hello Mouse')}
-        onKeyDown={(event) => {
-          if (event.keyCode === 13) {
-            event.preventDefault()
-            alert('Hello Keyboard')
-          }
-        }}
-      >
-        Add
-      </ButtonTransparent>
-    }
-    label="Custom Fields"
+const addButton = (
+  <ButtonTransparent
+    size="xxsmall"
+    iconBefore="Plus"
+    onClick={() => alert('Hello Mouse')}
+    onKeyDown={(event) => {
+      if (event.keyCode === 13) {
+        event.preventDefault()
+        alert('Hello Keyboard')
+      }
+    }}
   >
+    Add
+  </ButtonTransparent>
+)
+
+export const FieldPicker = () => (
+  <Tree defaultOpen detailAccessory detail={addButton} label="Custom Fields">
     <TreeGroup label="DIMENSIONS">
       <PickerItem />
       <PickerItem />
       <PickerItem />
       <PickerItem />
     </TreeGroup>
+    <TreeGroup label="MEASURES" color="keyFocus">
+      <Tree visuallyAsBranch label="Hello">
+        <PickerItem />
+      </Tree>
+      <TreeItem color="orange" icon="FieldString">
+        Name
+      </TreeItem>
+      <PickerItem />
+      <PickerItem />
+      <PickerItem />
+    </TreeGroup>
   </Tree>
 )