Terra-core: Create Multiple Selectable Table Component

[dv297]

Issue Description

We want a table where multiple rows of a terra-table can be selected. Currently, there is only a SingleSelectableRows component. A parallel MultipleSelectableRows component should be made.

Issue Type

• [X] New Feature
• [X] Enhancement
• [ ] Bug
• [ ] Other

Expected Behavior

• Initial Render
  
  
  
  • The table is rendered with any preselected row selected
  
  
  • Honors the isSelected prop
  
  
• Clicking on an unselected row will select the row unless isSelectable is false. *
• Clicking on a selected row will deselect the row unless isSelectable is false. *
• Multiple rows can be selected
• A callback function can be provided to the component that is called after any row is selected or deselected.

* - isSelectable could be renamed/ a new prop with different name could be used to something else to better convey this functionality, especially with deselect.

Current Behavior

N/A

[dv297]

Technical Design

Summary

Adds a new component MultiSelectableRows that is accessable from terra-table.

React Props

| Prop Name | Type | Is Required | Default Value | Description |
|-|-|-|-|-|
| children| node| optional| | The children passed to the component|
| maxSelectionCount | number | optional| undefined|The maximum number of rows that can be selected.|
| onChange| func| optional| undefined| A callback function for onChange action. Has the following parameters:

• the event triggering the onChange
• the list of the indexes for the rows selected
• the index of the last item selected

|

Example

import React from 'react';
import Table from 'terra-table';

<Table isStriped={false}>
  <Table.Header>
    <Table.HeaderCell content={'Column Heading 1'} key={'COLUMN_0'} minWidth={'small'} />
    <Table.HeaderCell content={'Column Heading 2'} key={'COLUMN_1'} minWidth={'medium'} />
    <Table.HeaderCell content={'Column Heading 3'} key={'COLUMN_2'} minWidth={'large'} />
  </Table.Header>
  <Table.MultiSelectableRows>
    <Table.Row isSelected={true} key={'ROW_0'}>
      <Table.Cell content={'Table Data'} key={'COLUMN_0'} />
      <Table.Cell content={'Table Data'} key={'COLUMN_1'} />
      <Table.Cell content={'Table Data'} key={'COLUMN_2'} />
    </Table.Row>
    <Table.Row key={'ROW_1'}>
      <Table.Cell content={'Table Data'} key={'COLUMN_0'} />
      <Table.Cell content={'Table Data'} key={'COLUMN_1'} />
      <Table.Cell content={'Table Data'} key={'COLUMN_2'} />
    </Table.Row>
    <Table.Row key={'ROW_2'}>
      <Table.Cell content={'Table Data'} key={'COLUMN_0'} />
      <Table.Cell content={'Table Data'} key={'COLUMN_1'} />
      <Table.Cell content={'Table Data'} key={'COLUMN_2'} />
    </Table.Row>
  </Table.MultiSelectableRows>
</Table>

CSS Classes

None

[emilyrohrbough]

Add prop maxSelectionCount to define the maximum number of rows that can be selected. This would make table API consistent with list API.

For the onChange callback, the list API only returns the event and the list of selected indexes. Again I think this should be consistent with the list's behavior. I would be curious what others think about return the last selected index. That might be a change we would want to make in multi-select list then.

[emilyrohrbough]

* isSelectable could be renamed/ a new prop with different name could be used to something else to better convey this functionality, especially with deselect.

What do you mean by this?

[dv297]

Re: isSelectable. The rows should be selectable and unselectable. Tables.row only has an isSelectable prop, but saying something "isSelectable" doesn't really say anything about the fact that it can be "deselected" and could be confusing. It's just a naming nuance. Something like "isSelectionChangeable" makes more sense from that kind of standpoint, but really only makes sense when defined on a Table.row that is nested within one of these *SelectableRows components anyways.

[jmsv6d]

I'd prefer to keep isSelectable prop the same. It's the same terminology used in our other components (i.e. List). I believe that it's generally understood that a component that allows multiple items to be selected also allows you to deselect those items. And I'm not a fan of making non-passive api changes for the sole purpose of renaming a prop.

[dv297]

@emilyrohrbough Updated tech design to include maxSelectionCount.

Re: isSelectable. I'm fine with leaving the prop the same. I also wasn't in love with making a non-passive change. I just wanted to bring light to it being possibly ambiguous.

And I can change the onChange parameters. It's just a convenience API so that users can know exactly what changed, but if other Terra components don't have this feature, perhaps it would be better to do a tech design on the need for it and introduce it consistently throughout.

[dkasper-was-taken]

May need to consider the integration of these two issues, involving selectable sub-components:

856 & #857

[dv297]

@dkasper-was-taken and @emilyrohrbough , my initial design was for a "MultipleSelectableRows" component. Given similarity to the terra-list component, and terra-list already uses the name "MultiSelectList", should I rename this component to "MultiSelectableRows", rather than multiple? I kept the "selectable"vs "select" because SingleSelectableRows already exists.

[dv297]

I'm also wondering about the desired behavior of the onChange. Did we ever decide on the parameters of that callback? And should we be sorting the selected indexes? I noticed that the MultiSelectList component doesn't sort, so the array given back will be just based on the order the user clicked on them. It seems like this could be surprising behavior for consumers and doesn't seem as "pure", as the UI doesn't directly translate to the state with time-based ordering involved.

[jmsv6d]

• I like MultiSelectableRows better.
• If you did add the last parameter of the last item selected to the onChangeCallback, what would be returned when the last change was unselecting an item?
• I'm not sure if I see the value of sorting the selected indexes, I think it would just be added complexity without much, if any, benefit to consumers. And I think I would rather keep it consistent with the List api.

[emilyrohrbough]

• I agree with Jamie, I like MultiSelectableRows
• The list of un-selected indexes returned seems fine. Overall it seems the consumer will be handling whatever logic is associated with a row selection regardless the order of returned indexes. Can you think of a scenario where this behavior would be different otherwise?
• I can see value in returning the last updated row, selected or unselected. @jmsv6d @dkasper-was-taken what would you expect?

[dv297]

• Cool, will go with MultiSelectableRows
• It could still return the item being changed. It's being changed from selected to unselected.
• I think it's just about consistency. If rows 1, 2, and 3 are selected, it's easy to mentally reason that the component would return [0,1,2]. But then when you see [2,1,0], or [1,2,0], or [1,2,0], etc, it can be confusing and requires extra mental overhead for the user.

[dv297]

But no, I can't think of any scenario where sorting is required. But I also can't think of any scenario where the time-based ordering is required. I'm down with whatever though, just an API consideration I had.

[jmsv6d]

I can see the value in returning the last updated row. I think if we have that in the table we should add it to the list as well. I'm just thinking returning the item or an object containing the index and the new selection state would be more helpful than just the index.

[dv297]

Right. The index will be returned as the 3rd parameter to the callback, while the new selection state will be returned as the 2nd parameter. So as far as the first 2 parameters go, it is in consistency with the terra-list API, the 3rd would just be a new feature and could be added to terra-list with passivity maintained.

Went ahead and updated the technical design to use MultiSelectableRows and removed the documentation/requirement about sorting. Let me know if there are any other concerns, as I should freeing up to start working on this again.

[bjankord]

@dv297 This has been released in terra-table 1.16.0