-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
TableList improvements #4579
Merged
Merged
TableList improvements #4579
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
👍 |
kmerz
approved these changes
Feb 14, 2018
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
This provides some feedback when users select items in the list.
This is not different than the case when user selects two or more elements and it makes easier to figure out what the UI does.
Until now we positioned the individual actions inside the component but not the bulk actions. Moving it inside the component makes it more consistent.
When some items are selected, changes in the filter should also affect the selection, deselecting items that are filtered out from the list. selection
- Set the select all checkbox as indeterminate when some but not all items are checked - Remove the select all checked status from the component state
Make clearer how we recalculate selected items on filter change by extracting the code into a function.
We require the items prop to be an instance of `Immutable.List`, but the code doesn't treat it like that. This commit ensures we convert the list back to a JS array before using it in the filter.
Move code setting the "select all" checkbox status to indeterminate into its own function.
Having no items to display should not hide UI elements from the view.
When no elements are selected, display filter elements that may filter the data outside of the component.
This is meant to provide a more customizable UI, only providing the basis to build a TableList.
Ensure giving a string header is displayed properly. Consumers providing their own nodes must do this on their own.
We need this functionality but the `TableList` component is becoming more complicated. In the end, consumers needing a fully customizable `TableList` should use `ControlledTableList` and customize the components as needed on a case by case basis.
That makes clearer what the prop is for, in the end where elements are rendered is an implementation detail.
This also removes checkboxes from items, since there is no need for selecting them when actions can only affect an element.
Rename `isFilterEnabled` to `enableFilter`.
In certain screen sizes the filter buttons cannot fit on its column. As we currently don't use the space after the filter, here we take all available space to ensure the filter buttons will be aligned with the input.
edmundoa
force-pushed
the
table-list-improvements
branch
from
February 14, 2018 14:15
75689c3
to
96fdb47
Compare
After merge-conflicts, the description was duplicated.
I have rebased to master and adapted the |
kmerz
approved these changes
Feb 14, 2018
mariussturm
approved these changes
Feb 14, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to fix some shortcomings on the
TableList
component. We need to do those in order to reuse the component in the new collector UI and to fix some old usability issues that have been there for a while now. These are the main changes:ControlledTableList
component, that allows more flexibility to consumers that only want the basis of aTableList
but nothing else. This component is now used byTableList
itselfitems
prop changesenableBulkActions
propenableFilter
prop