-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat(ktableview): add bulk actions feature [KHCP-13106] #2365
Conversation
✅ Deploy Preview for kongponents-sandbox ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kongponents ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Other issues:
- If I select data on page 1, then move to page 2, I should be able to click the "indeterminate" checkbox to deselect all items. I shouldn't have to go back through every previous page to deselect things.
- This may only be an issue in the docs site, but if only 1 item is selected, the Bulk Dropdown item should read "Delete (1 item)" - not "items" (should be singular)
- The tooltip for a disabled checkbox should be shown when hovering over the checkbox. We don't need a separate Info icon (see screenshot)
docs/components/table-view.md
Outdated
@@ -51,7 +51,12 @@ For an example of `headers` prop usage please refer to [`data` prop documentatio | |||
|
|||
#### Reserved Header Keys | |||
|
|||
- `actions` - the column displays an actions [KDropdown](/components/dropdown) button for each row and displays no label (as if `hideLabel` was `true`; you can set `hideLabel` parameter to `false` to show the label). KTableView will automatically render the actions dropdown button with an icon and you simply need to provide dropdown items via the [`action-items` slot](#action-items). | |||
- `bulkActions` - the column displays checkboxes for each row. Column header displays a checkbox clicking on which will check or uncheck all table rows and selected rows count. Apart from that, KTableView will render a dropdown on the right of the table toolbar directly above the table and you simply need to provide dropdown items via the [`bulk-action-items` slot](#bulk-action-items). Refer to the [`bulk-action-items` slot](#bulk-action-items) for the example. |
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 thought this wasn't needed since you have the rowBulkActions
prop?
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.
No, the prop is only for enabling/disabling row checkboxes. Most tables won't require disabling some row checkboxes so having an easier way to enable bulk actions (by providing a header) is beneficial IMO.
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.
Bulk actions applies to the entire table, not to a single column. I'm not sure I understand why this is correct.
Also, what does a user set this value to? Nothing in the description outlines how to use this?
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.
Bulk actions are enabled for the entire table through providing a header object with a reserved key ({ ..., key: 'bulkActions' }
). That's how they enable bulk actions column (same goes for actions
column - if you want to have that column in the table you need to enable it in headers cuz that's how we control which columns are present in the table). The last sentence in this paragraph mentions that example can be found in bulk-action-items
slot docs section.
Co-authored-by: Adam DeHaven <2229946+adamdehaven@users.noreply.github.com>
The functionality looks good. I just have a couple of questions: It seems we can bulk select across different pages, but I’m not sure if this is best practice. (I’m okay with it if it’s a widely used pattern.) Personally, I tend to only select what I can see to avoid unnecessary mistakes. Additionally, how does this work with cursor pagination, where we (FE) might not store the previous or next page data? |
We had a long discussion about this in our team (unfortunately it was on an unrecorded call so I can't link you back to it) and decided that bulk actions should work with pagination as it seems to be a common practice many services follow (Gmail and DataDog were among the few we reviewed). As to how this works with cursor pagination - KTableView actually doesn't know any other data other than what's provided in |
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.
Co-authored-by: Adam DeHaven <2229946+adamdehaven@users.noreply.github.com>
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.
Code + functional looks good to me! Nice work 🚀
# [9.6.0](v9.5.5...v9.6.0) (2024-09-09) ### Features * **ktableview:** add bulk actions feature [KHCP-13106] ([#2365](#2365)) ([cd8ceb5](cd8ceb5))
🎉 This PR is included in version 9.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Jira: https://konghq.atlassian.net/browse/KHCP-13106
Adds bulk actions functionality in KTableView:
bulkActions
reserved header for enabling bulk actions columnrowBulkActionEnabled
propbulk-actions
slotbulk-action-items
slotrow-select
emitted event