Action List: Selecting test suite (#800)

* Minor checkbox updates
- Clicking on checkbox container of disabled row now does not select row
- Added on key down for checkbox

* Added ActionListCheckbox test
- Resolved bug where clicking on checkbox fired both container onChange and checkbox onChange

* Started select test suite:

* Realigned header with items

* Added css properties to assure header stays aligned with items
- When shrinking the window, ActionList items and header became misaligned; added overflow: hidden on header column content and flex-basis/flex-shrink so that header follows items shrink behavior

* Added selecting test suite
- Modified ActionList docs to respect ActionListItem.id string type

* Minor addition to sorting documentation
- Minor addition discusses what to do when data initially loads with a sort

* Added CHANGELOG updates

* PR Feedback

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

Author: 3 people

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- `ActionList` documentation and select test suite
+- `ActionListCheckbox` test suite
 - `ActionList` documentation
 - `CrossFilter` Icon
 - `Checkbox` & `Radio` now support `readOnly` property

packages/components/src/ActionList/ActionList.test.tsx

@@ -61,7 +61,7 @@ const columns: ActionListColumns = [
 
 const data = [
   {
-    id: '1',
+    id: 1,
     name: 'Richard Garfield',
     type: 'Game Designer',
   },
@@ -83,7 +83,7 @@ const items = data.map(({ id, name, type }) => {
   )
 
   return (
-    <ActionListItem key={id} id={id} actions={availableActions}>
+    <ActionListItem key={id} id={String(id)} actions={availableActions}>
       <ActionListItemColumn>{id}</ActionListItemColumn>
       <ActionListItemColumn>{name}</ActionListItemColumn>
       <ActionListItemColumn>{type}</ActionListItemColumn>
@@ -178,7 +178,7 @@ describe('ActionList', () => {
 
         return (
           <ActionListItem
-            id={id}
+            id={String(id)}
             key={id}
             actions={availableActions}
             onClick={handleListItemClick}
@@ -231,14 +231,16 @@ describe('ActionList', () => {
       </ActionList>
     )
 
+    afterEach(() => {
+      onSort.mockClear()
+    })
+
     test('Calls onSort if canSort property is true', () => {
       const { getByText } = renderWithTheme(actionListWithSort)
 
       const idColumnHeader = getByText('ID')
       fireEvent.click(idColumnHeader)
       expect(onSort.mock.calls.length).toBe(1)
-
-      onSort.mockClear()
     })
 
     test('Does not call onSort if canSort property is false', () => {
@@ -247,14 +249,57 @@ describe('ActionList', () => {
       const nameColumnHeader = getByText('Name')
       fireEvent.click(nameColumnHeader)
       expect(onSort.mock.calls.length).toBe(0)
-
-      onSort.mockClear()
     })
   })
 
-  xdescribe('Selecting', () => {
-    xtest('Select tests needed', () => {
-      return null
+  describe.only('Selecting', () => {
+    const onSelect = jest.fn()
+    const actionListWithSelect = (
+      <ActionList columns={columns} canSelect onSelect={onSelect}>
+        {items}
+      </ActionList>
+    )
+    const actionListWithItemsSelected = (
+      <ActionList
+        columns={columns}
+        canSelect
+        itemsSelected={['1']}
+        onSelect={onSelect}
+      >
+        {items}
+      </ActionList>
+    )
+    const onClickRowSelect = (
+      <ActionList
+        columns={columns}
+        canSelect
+        onSelect={onSelect}
+        onClickRowSelect
+      >
+        {items}
+      </ActionList>
+    )
+    afterEach(() => {
+      onSelect.mockClear()
+    })
+
+    test('Checkbox click calls onSelect', () => {
+      const { getByRole } = renderWithTheme(actionListWithSelect)
+      fireEvent.click(getByRole('checkbox'))
+      expect(onSelect).toHaveBeenCalledTimes(1)
+    })
+
+    test('Row click calls onSelect when onClickRowSelect is true', () => {
+      const { getByText } = renderWithTheme(onClickRowSelect)
+      const nameCell = getByText('Richard Garfield')
+      fireEvent.click(nameCell)
+      expect(onSelect).toHaveBeenCalledTimes(1)
+    })
+
+    test('itemsSelected determines if a checkbox is checked', () => {
+      const { getByRole } = renderWithTheme(actionListWithItemsSelected)
+      const checkbox = getByRole('checkbox')
+      expect((checkbox as HTMLInputElement).checked).toEqual(true)
     })
   })
 })

packages/components/src/ActionList/ActionList.tsx

@@ -32,6 +32,7 @@ import {
 } from './ActionListHeader'
 import { ActionListItemColumn } from './ActionListItemColumn'
 import { ActionListRowColumns } from './ActionListRow'
+import { actionListCheckboxWidth } from './ActionListCheckbox'
 import { ActionListContext } from './ActionListContext'
 import { ActionListHeaderColumn } from './ActionListHeader/ActionListHeaderColumn'
 import {
@@ -150,11 +151,16 @@ export const ActionList = styled(ActionListLayout)<ActionListProps>`
     grid-template-columns: ${(props) =>
       props.columns.map((column) => `${column.widthPercent}%`).join(' ')};
     align-items: center;
+  }
+
+  ${/* sc-selector */ ActionListItemColumn}:first-child {
+    padding-left: ${({ canSelect, theme }) =>
+      canSelect ? theme.space.none : undefined};
+  }
 
-    ${/* sc-selector */ ActionListItemColumn}:first-child {
-      padding-left: ${({ canSelect, theme }) =>
-        canSelect ? theme.space.none : undefined};
-    }
+  ${/* sc-selector */ ActionListHeaderColumn}:first-child {
+    padding-left: ${({ canSelect, theme }) =>
+      canSelect ? theme.space.none : undefined};
   }
 
   ${/* sc-selector */ ActionListItemColumn},
@@ -164,7 +170,8 @@ export const ActionList = styled(ActionListLayout)<ActionListProps>`
   }
 
   ${ActionListHeader} {
-    padding-left: ${({ canSelect }) => (canSelect ? '2.75rem' : undefined)};
+    padding-left: ${({ canSelect }) =>
+      canSelect ? actionListCheckboxWidth : undefined};
   }
 
   ${(props) => numericColumnCSS(getNumericColumnIndices(props.columns))}

packages/components/src/ActionList/ActionListCheckbox.test.tsx

@@ -24,20 +24,39 @@
 
  */
 
+import React from 'react'
+import { renderWithTheme } from '@looker/components-test-utils'
+import { fireEvent } from '@testing-library/react'
+import { ActionListCheckbox } from './ActionListCheckbox'
+
 describe('ActionListCheckbox', () => {
-  xtest('Renders checked', () => {
-    return null
+  test('Renders checked', () => {
+    const { getByRole } = renderWithTheme(<ActionListCheckbox checked />)
+    const actionListCheckbox = getByRole('checkbox')
+    expect((actionListCheckbox as HTMLInputElement).checked).toEqual(true)
   })
 
-  xtest('Renders unchecked', () => {
-    return null
+  test('Renders unchecked', () => {
+    const { getByRole } = renderWithTheme(<ActionListCheckbox />)
+    const actionListCheckbox = getByRole('checkbox')
+    expect((actionListCheckbox as HTMLInputElement).checked).toEqual(false)
   })
 
-  xtest('Renders disable', () => {
-    return null
+  test('Renders disabled', () => {
+    const { getByRole } = renderWithTheme(<ActionListCheckbox disabled />)
+    const actionListCheckbox = getByRole('checkbox')
+    expect((actionListCheckbox as HTMLInputElement).disabled).toEqual(true)
   })
 
-  xtest('Calls onChange callback', () => {
-    return null
+  test('Calls onChange callback', () => {
+    const onChange = jest.fn()
+    const { getByRole } = renderWithTheme(
+      <ActionListCheckbox onChange={onChange} />
+    )
+    const actionListCheckbox = getByRole('checkbox')
+    fireEvent.click(actionListCheckbox)
+    expect(onChange).toHaveBeenCalledTimes(1)
+    fireEvent.click(actionListCheckbox)
+    expect(onChange).toHaveBeenCalledTimes(2)
   })
 })

packages/components/src/ActionList/ActionListCheckbox.tsx

@@ -37,26 +37,43 @@ export interface ActionListCheckboxProps {
 
 export const checkListProps = ['checked', 'disabled', 'onChange']
 
+export const actionListCheckboxWidth = '2.75rem'
+
 const ActionListCheckboxLayout: FC<ActionListCheckboxProps> = ({
   onChange,
   checked,
   disabled,
   className,
-}) => (
-  <div onClick={onChange} className={className}>
-    <Checkbox
-      disabled={disabled}
-      checked={checked}
-      onChange={disabled ? undefined : onChange}
-    />
-  </div>
-)
+}) => {
+  const handleOnChange = (event: React.MouseEvent<HTMLInputElement>) => {
+    const isNotFromChild = event.currentTarget === event.target
+    isNotFromChild && !disabled && onChange && onChange()
+  }
+
+  const handleOnKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
+    if (event.keyCode === 13) {
+      event.currentTarget.click()
+    }
+  }
+
+  return (
+    <div onClick={handleOnChange} className={className}>
+      <Checkbox
+        disabled={disabled}
+        checked={checked}
+        onChange={handleOnChange}
+        onKeyDown={handleOnKeyDown}
+      />
+    </div>
+  )
+}
 
 export const ActionListCheckbox = styled(ActionListCheckboxLayout)`
   align-items: center;
   display: flex;
   justify-content: center;
-  width: 3.5rem;
+  flex-basis: ${actionListCheckboxWidth};
+  flex-shrink: 0;
 
   cursor: ${({ disabled }) => (disabled ? 'not-allowed' : 'default')};
 `

packages/components/src/ActionList/ActionListHeader/ActionListHeaderColumn.tsx

@@ -74,4 +74,5 @@ ActionListHeaderColumnLayout.displayName = 'ActionListHeaderColumnLayout'
 export const ActionListHeaderColumn = styled(ActionListHeaderColumnLayout)`
   display: flex;
   align-items: center;
+  word-break: break-all;
 `

packages/components/src/ActionList/ActionListItemColumn.tsx

@@ -65,7 +65,7 @@ export const ActionListItemColumn = styled(ActionListItemColumnLayout)<
   color: ${(props) => props.theme.colors.palette.charcoal700};
   display: ${(props) => (props.indicator ? 'flex' : undefined)};
   font-size: ${(props) => props.theme.fontSizes.xsmall};
-  overflow: hidden;
+  word-break: break-all;
 
   ${ActionListItemColumnInnerLayout} {
     display: flex;

packages/playground/src/index.tsx

@@ -27,12 +27,12 @@
 import React from 'react'
 import ReactDOM from 'react-dom'
 import { ComponentsProvider } from '@looker/components'
-import { FieldsDemo } from './Form/FieldsDemo'
+import { ActionListDemo } from './ActionList/ActionListDemo'
 
 const App: React.FC = () => {
   return (
     <ComponentsProvider>
-      <FieldsDemo />
+      <ActionListDemo />
     </ComponentsProvider>
   )
 }

packages/www/src/documentation/components/content/action-list.mdx

@@ -166,16 +166,18 @@ You can also use an `<ActionListManager>` element to render a "No Results" messa
 
 ## Sorting an Action List
 
-You can implement sorting on an `<ActionList>` by passing a function into the optional `onSort` prop. Note that only columns whose corresponding column objects have `canSort: true` will be sortable.
+You can implement sorting on an `<ActionList>` by passing a function into the optional `onSort` prop.
 
 Clicking on the column header will lead to an `onSort` call, with (1) the column's id and (2) the next sort direction, passed in as arguments. The sort direction argument will either be a string 'desc' if the current sort direction of the column is 'asc', or 'asc' in all other cases.
 
+**Note:** Only columns whose corresponding column objects have `canSort: true` will be sortable. In addition, if your data is initially sorted on a specific column, you may want to default the corresponding column object with a `sortDirection` property.
+
 The function passed into `onSort`, generally speaking, should perform these actions for proper sorting behavior:
 
-- Update the corresponding column object to have a new sort direction
+- Update the corresponding column object's `sortDirection`
 - Sort the collection representing your data
 
-**Note:** If you want default sorting behavior, you can use the `doDefaultActionListSort` helper function. The `doDefaultActionListSort` helper function will return a sorted data array as well as an updated columns array, which can then be used to replace your existing arrays post-sort.
+If you want default sorting behavior, you can use the `doDefaultActionListSort` helper function. The `doDefaultActionListSort` helper function will return a sorted data array as well as an updated columns array, which can then be used to replace your existing arrays post-sort.
 
 ```jsx
 ;() => {
@@ -201,6 +203,7 @@ The function passed into `onSort`, generally speaking, should perform these acti
       primaryKey: true,
       title: 'ID',
       type: 'number',
+      sortDirection: 'asc',
       widthPercent: 20,
     },
     {
@@ -259,13 +262,13 @@ An `<ActionList/>` with the `canSelect` prop passed in will display `<Checkbox/>
 
 Clicking on a `<Checkbox/>` will trigger the `onSelect` callback prop, with the id of the clicked `<ActionListItem/>` passed in as an argument.
 
-The `itemsSelected` prop should be given the id's of all selected rows.
+The `itemsSelected` prop should be given the id's of all selected `<ActionListItem>`s. Note that because the id of an `<ActionListItem>`s must be a string, the `itemSelected` prop should be an array of strings.
 
-**Note:** If you would like a row's `onClick` to toggle a row's selected state, rather than have a custom-defined `onClick`, you can do so by passing in the `onClickRowSelect` prop into the `<ActionList/>`.
+If you would like a row's `onClick` to toggle a row's selected state, rather than have a custom-defined `onClick`, you can do so by passing in the `onClickRowSelect` prop into the `<ActionList/>`.
 
 ```jsx
 ;() => {
-  const [selections, setSelections] = useState([2])
+  const [selections, setSelections] = useState([])
   const onSelect = (selection) => {
     setSelections(
       selections.includes(selection)
@@ -317,7 +320,7 @@ The `itemsSelected` prop should be given the id's of all selected rows.
     return (
       <ActionListItem
         key={id}
-        id={id}
+        id={String(id)}
         onClick={() => alert('Row clicked')}
         actions={actions}
         disabled={id === 3 && true}