Added support for default selections in EuiBasicTable

[ashikmeerankutty]

Summary

Makes progress on #3406

Added support for enabling default selections in EuiBasicTable.

This was done by adding a new property selected to similar to selectable in the selection properties

Checklist

• Check against all themes for compatibility in both light and dark modes
• Checked in mobile
• Checked in IE11 and Firefox
• Props have proper autodocs
• Added documentation examples
• Added or updated jest tests
• Checked for breaking changes and labeled appropriately
  - [ ] Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[kibanamachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[chandlerprall]

Checking on this as you changed it to a draft - do you want a review or other input, or are you planning on more changes?

[ashikmeerankutty]

@chandlerprall I changed it to draft as it does not meet some requirements. In the case of default selection, I think it works fine. But when the selection management is done from outside the component as the use case in issue #1077 It does not re-render the selection as I was doing a check for isdefaultSelectionRendered. And that also creates the same problem in EuiInMemoryTables that on the first render isdefaultSelectionRendered is set to true and when the data is loaded to the table the default selections are not checked because isdefaultSelectionRenderedistrue`. I couldn't arrive at a solution until now. It would be great if you can review and suggest some solutions.

[chandlerprall]

I think, for all the reasons you have uncovered/discovered, that we should allow passing the initial selection state as a prop (like you have), but add a setSelection method on the component class es which can be called by the application code, e.g.

tableRef = useRef();
...
<EuiBasicTable/EuiInMemoryTable ref={tableRef}/>
...
tableRef.current.setSelection(newSelection)

Also, I think we'd want to match the types of initialSelection and newSelection to the existing selection array: an array of the items that are selected, and not an array of ids or a function returning true/false.

[cchaos]

This one could very much benefit from an example in the docs. Maybe something added to the Adding selection to a table section?

[ashikmeerankutty]

@cchaos Thanks for the suggestions. Docs updated 👍

[chandlerprall]

Looking good! Please add a comment on defaultSelectionRendered describing what it is used for - without knowing the messy table lifecycle it can introduce some confusion.

In the example, if I uncheck a default-selected row, change to another page, then go back the item is selected again.

Lastly, the setSelection method should be added to EuiInMemoryTable as well, which should pass through to the underlying EuiBasicTable method

[ashikmeerankutty]

In the example, if I uncheck a default-selected row, change to another page, then go back the item is selected again.

@chandlerprall I am quite confused here about what should be the desired behavior in a case like this. To retain the selection I think either we have to

• Handle default selection as a separate array in the state as selection array gets cleared on pagination changes or,
• We must update the array from the props on the change of selection.

Which method would be more ideal?

[chandlerprall]

I wouldn't expect the initial selection to be applied a second time, so I think the issue is with this line?

https://github.com/elastic/eui/pull/3418/files#diff-ae14f0fb4be86d6d13f66aec7f3d2465R401

[ashikmeerankutty]

I wouldn't expect the initial selection to be applied a second time, so I think the issue is with this line?

@chandlerprall In that case, only the initial selection works for the first page if there is pagination is that the desired behavior?

[chandlerprall]

jenkins test this

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_3418/

[ashikmeerankutty]

just found initial selection is not working in case of in-memory-table

[chandlerprall]

jenkins test this

[ashikmeerankutty]

@chandlerprall When I'm using mount for snapshot testing the component. Each time different classNames generated causes snapshot tests to fail. Is there anything to do here?

[chandlerprall]

Is there anything to do here?

What does your test look like?

[ashikmeerankutty]

What does your test look like?

test('with initial selection', () => {
    const props: EuiInMemoryTableProps<BasicItem> = {
      ...requiredProps,
      items: [
        { id: '1', name: 'name1' },
        { id: '2', name: 'name2' },
        { id: '3', name: 'name3' },
      ],
      itemId: 'id',
      columns: [
        {
          field: 'name',
          name: 'Name',
          description: 'description',
        },
      ],
      pagination: true,
      selection: {
        onSelectionChange: () => undefined,
        initialSelected: [{ id: '1', name: 'name1' },],
      },
    };
    const component = mount(<EuiInMemoryTable {...props} />);

    expect(component).toMatchSnapshot();
  });

[chandlerprall]

Thanks! Need to mock out our htmlIdGenerator function which is used by those selection checkboxes. Add this to the top of in_memory_table.tsx immediately after the import statements

jest.mock('../../services/accessibility', () => ({
  htmlIdGenerator: () => () => 'generated-id',
}));

[ashikmeerankutty]

@chandlerprall Thanks for the help!. Changes committed 👍

[chandlerprall]

jenkins test this

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_3418/

[chandlerprall]

Changes LGTM; Verified functionality in the examples and spot-checked that the test snapshots look correct.

Thanks again!