-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EuiInMemoryTable] Replace basic usages of deprecated ref method with controlled selection.selected
API
#175722
[EuiInMemoryTable] Replace basic usages of deprecated ref method with controlled selection.selected
API
#175722
Conversation
29c3626
to
77c9910
Compare
77c9910
to
825494f
Compare
Pinging @elastic/eui-team (EUI) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just once change request for our team.
...gins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx
Outdated
Show resolved
Hide resolved
825494f
to
1e87a0c
Compare
… controlled `selection.selected` API (#175727) ## Summary **Please help us QA your affected tables to confirm that your plugin's table selection still works as before!** EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts Fleet's more advanced usage of controlled selection, which should not suffer any UI/UX regressions. See also: - elastic/eui#7321 - #175722 (examples of basic conversions)
…ith controlled `selection.selected` API (#175838) Closes #175836 ## Summary **Please help us QA your affected tables to confirm that your plugin's table selection still works as before!** EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts the Security plugin's basic usages of controlled selection and additionally removes 2 deletion cancellation behaviors on the team's request. There should not be any other UI/UX regressions when selecting rows. See also: - elastic/eui#7321 - #175722 (examples of basic conversions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested locally in stateful
… method with controlled `selection.selected` API (#175726) ## Summary **Please help us QA your affected tables to confirm that your plugin's table selection still works as before!** EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts basic usages of controlled selection, which should not suffer any UI/UX regressions. See also: - elastic/eui#7321 - #175722 (examples of basic conversions) Co-authored-by: Tomasz Ciecierski <tomasz.ciecierski@elastic.co>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cases changes ok 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tested locally and this appears to break the 'select all pages' action:
Before / After (changes from this PR):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should fix it - please let me know if not! 1567fd9
…cted` API + remove `refs` (should no longer be needed)
…ion.selected` API
7284be5
to
1567fd9
Compare
expect(screen.getByTestId('selectedFields')).toHaveTextContent('Selected 2 fields'); | ||
// The select all button should select all rows regardless of visibility | ||
userEvent.click(screen.getByTestId('selectAllFields')); | ||
expect(screen.getByTestId('selectedFields')).toHaveTextContent('Selected 20 fields'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this @cee-chen !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this update @cee-chen! 🙏
✅ Desk tested locally
LGTM 🚀
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
… controlled `selection.selected` API (elastic#175722) ## Summary See elastic/eui#7321 EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts basic usages of controlled selection, which should not suffer any UI/UX regressions. **Please help us QA your affected tables to confirm that your table selection still works as before!**
… controlled `selection.selected` API (elastic#175727) ## Summary **Please help us QA your affected tables to confirm that your plugin's table selection still works as before!** EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts Fleet's more advanced usage of controlled selection, which should not suffer any UI/UX regressions. See also: - elastic/eui#7321 - elastic#175722 (examples of basic conversions)
…ith controlled `selection.selected` API (elastic#175838) Closes elastic#175836 ## Summary **Please help us QA your affected tables to confirm that your plugin's table selection still works as before!** EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts the Security plugin's basic usages of controlled selection and additionally removes 2 deletion cancellation behaviors on the team's request. There should not be any other UI/UX regressions when selecting rows. See also: - elastic/eui#7321 - elastic#175722 (examples of basic conversions)
… method with controlled `selection.selected` API (elastic#175726) ## Summary **Please help us QA your affected tables to confirm that your plugin's table selection still works as before!** EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts basic usages of controlled selection, which should not suffer any UI/UX regressions. See also: - elastic/eui#7321 - elastic#175722 (examples of basic conversions) Co-authored-by: Tomasz Ciecierski <tomasz.ciecierski@elastic.co>
… controlled `selection.selected` API (elastic#175722) ## Summary See elastic/eui#7321 EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts basic usages of controlled selection, which should not suffer any UI/UX regressions. **Please help us QA your affected tables to confirm that your table selection still works as before!**
… controlled `selection.selected` API (elastic#175727) ## Summary **Please help us QA your affected tables to confirm that your plugin's table selection still works as before!** EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts Fleet's more advanced usage of controlled selection, which should not suffer any UI/UX regressions. See also: - elastic/eui#7321 - elastic#175722 (examples of basic conversions)
…ith controlled `selection.selected` API (elastic#175838) Closes elastic#175836 ## Summary **Please help us QA your affected tables to confirm that your plugin's table selection still works as before!** EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts the Security plugin's basic usages of controlled selection and additionally removes 2 deletion cancellation behaviors on the team's request. There should not be any other UI/UX regressions when selecting rows. See also: - elastic/eui#7321 - elastic#175722 (examples of basic conversions)
… method with controlled `selection.selected` API (elastic#175726) ## Summary **Please help us QA your affected tables to confirm that your plugin's table selection still works as before!** EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts basic usages of controlled selection, which should not suffer any UI/UX regressions. See also: - elastic/eui#7321 - elastic#175722 (examples of basic conversions) Co-authored-by: Tomasz Ciecierski <tomasz.ciecierski@elastic.co>
… controlled `selection.selected` API (elastic#175722) ## Summary See elastic/eui#7321 EUI will shortly be removing this deprecated ref `setSelection` method in favor of the new controlled `selection.selected` prop. This PR converts basic usages of controlled selection, which should not suffer any UI/UX regressions. **Please help us QA your affected tables to confirm that your table selection still works as before!**
Summary
See elastic/eui#7321
EUI will shortly be removing this deprecated ref
setSelection
method in favor of the new controlledselection.selected
prop. This PR converts basic usages of controlled selection, which should not suffer any UI/UX regressions.Please help us QA your affected tables to confirm that your table selection still works as before!