Added support for default selections in EuiBasicTable#3418
Added support for default selections in EuiBasicTable#3418chandlerprall merged 36 commits intoelastic:masterfrom
Conversation
|
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? |
|
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? |
|
@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 |
|
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 Also, I think we'd want to match the types of |
|
This one could very much benefit from an example in the docs. Maybe something added to the |
|
@cchaos Thanks for the suggestions. Docs updated 👍 |
|
Looking good! Please add a comment on In the example, if I uncheck a default-selected row, change to another page, then go back the item is selected again. Lastly, the |
@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
Which method would be more ideal? |
|
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 |
@chandlerprall In that case, only the initial selection works for the first page if there is |
…i into data-table-selection
|
jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3418/ |
|
just found |
|
jenkins test this |
|
@chandlerprall When I'm using |
src-docs/src/views/tables/in_memory/in_memory_selection_section.js
Outdated
Show resolved
Hide resolved
src-docs/src/views/tables/in_memory/in_memory_selection_section.js
Outdated
Show resolved
Hide resolved
src-docs/src/views/tables/in_memory/in_memory_selection_section.js
Outdated
Show resolved
Hide resolved
What does your test look like? |
|
|
Thanks! Need to mock out our |
src-docs/src/views/tables/in_memory/in_memory_selection_section.js
Outdated
Show resolved
Hide resolved
|
@chandlerprall Thanks for the help!. Changes committed 👍 |
|
jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3418/ |
chandlerprall
left a comment
There was a problem hiding this comment.
Changes LGTM; Verified functionality in the examples and spot-checked that the test snapshots look correct.
Thanks again!
Summary
Makes progress on #3406
Added support for enabling default selections in EuiBasicTable.
This was done by adding a new property
selectedto similar toselectablein the selection propertiesChecklist
- [ ] Checked for accessibility including keyboard-only and screenreader modes