Skip to content
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 30 commits into from
Feb 14, 2018
Merged

TableList improvements #4579

merged 30 commits into from
Feb 14, 2018

Conversation

edmundoa
Copy link
Contributor

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:

  • Introduce ControlledTableList component, that allows more flexibility to consumers that only want the basis of a TableList but nothing else. This component is now used by TableList itself
  • Improve feedback when selecting one or more elements in the list to perform bulk actions
  • Update selection when filter changes
  • Update filtered items and selection when items prop changes
  • Make bulk actions optional, by adding an enableBulkActions prop
  • Simplify how to disable filtering, by adding an enableFilter prop

@edmundoa edmundoa added this to the 3.0.0 milestone Feb 14, 2018
@kmerz
Copy link
Member

kmerz commented Feb 14, 2018

👍

Copy link
Member

@kmerz kmerz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Edmundo Alvarez added 24 commits February 14, 2018 15:05
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 edmundoa force-pushed the table-list-improvements branch from 75689c3 to 96fdb47 Compare February 14, 2018 14:15
Edmundo Alvarez added 3 commits February 14, 2018 15:37
@edmundoa
Copy link
Contributor Author

I have rebased to master and adapted the TokenList component to changes in this PR.

@mariussturm mariussturm merged commit 60406c5 into master Feb 14, 2018
@edmundoa edmundoa deleted the table-list-improvements branch February 14, 2018 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants