feat(bulkselection): ListView - Created bulk selection component #742

chalettu · 2018-06-04T19:44:54Z

This is a component that goes along with a toolbar to allow for select all/ select none and partial select of lists of data.

chalettu · 2018-06-04T19:45:45Z

Screen shot of intended functionality.

chalettu · 2018-06-04T19:48:17Z

@serenamarie125 , Here is the bulk selection component that is functioning as discussed. I don't know what the process for UX review and approval is in this repo but could you help me figure out how to get this reviewed?

chalettu · 2018-06-04T20:01:20Z

For some reason the screenshot video is not truly showing the color right but partially selected and select all states use @color-pf-blue-400 when the states happen.
This component compliments the toolbar and filter components. It has 3 states (None, All, Partially selected). This component has two ways to interact with it. One method is a toggle if you click the square icon it will

Toggle select all/select none
If you select one or more results from the list before clicking toggle, it will deselect all.

serenamarie125

@jeff-phillips-18 @dtaylor113 although there isn't an "official design pattern" for bulk selection yet, it is in progress. @jennyhaines @mcarrano please take a peak here for a quick review from a UX/Visuals side

https://docs.google.com/document/d/1gtDUMASD2dc8G2A82i0lYBjaSxFBw34-65TuilDQjHQ/edit?usp=sharing

serenamarie125 · 2018-06-05T12:41:25Z

@chalettu is it possible to add still screenshots showing that states of the icon in the bulk selector? Jut want to make sure that it matches - here are the high fidelity mocks
Partial Selection

All Selected

beanh66 · 2018-06-05T12:55:10Z

@chalettu thanks, this looks great! I do have two questions but I think they are more design related so may need @mcarrano to chime in.

Curious if the checkbox is too small, because it seems too difficult to click the checkbox without clicking the dropdown. Also unsure if we need the dropdown for cases where there are no other options available other than select all/none, but fine either way.

dtaylor113 · 2018-06-05T12:57:11Z

src/bulkselection/bulkselection.component.js

+      }
+    };
+    ctrl.$doCheck = function () {
+      var recordsSelectedCount = ctrl.totalRecords - ctrl.selectedRecordCount;


This variable is a little confusing and is too similar to ctrl.selectedRecordCount. Perhaps a better name would be unselectedRecordsCount.

dtaylor113 · 2018-06-05T12:58:33Z

test/toolbars/toolbar.spec.js

    active = element.find('.active');
    expect(active.length).toBe(1);
  });
+  it ('should have bulk selection menu shown', function() {


Appreciate the unit tests! :-)

cshinn · 2018-06-05T13:16:59Z

I'm not sure how tricky it is to do, but I think it's important that the selector be moved further left so that all the checkboxes align. The visual line created by those will make its functionality a lot more clear.

dtaylor113 · 2018-06-05T13:18:51Z

If I apply a fitter (Ex: Name='F'), then 'All' from the dropdown, it only selects rows shown by the filter. The dropdown icon is 'All/checkmark' in this state.
If I clear the filter, the icon does change to 'Partially Selected/minus'. Just want to verify this is the behavior we want?

Wondering if we want either:
a) The 'All' dropdown choice to truly select all records, even ones not shown by the current filter, or..
b) If 'All' only selects visible records shown by filter, then the dropdown icon should be 'partially selected' since not all records are actually selected, or...
c) Perhaps when a filter is applied, the dropdown choice should be something like 'All Filtered', or 'All Visible' -which would also cover pagination.

-thanks

mcarrano · 2018-06-05T13:26:48Z

This looks good. In answer to your question @beanh66 , while having the dropdown, even when the only choices are Select All and None, is indeed redundant, we felt this was necessary to make it clear what's happening here. I did not feel like just a checkbox inside a button would be very intuitive on it's own. Unlike for tables, the checkbox is a bit removed from the column headers here.

@dtaylor113 raises a good question about how this interacts with filtering. I was assuming that the selection only applied to what's currently loaded into the view, but maybe that's not the right answer. What do we do for tables? It's exactly the same use case. If I select all rows in a table and a filter is applied, does it select items that are filtered from the table? I wouldn't expect it to.

dtaylor113 · 2018-06-05T13:30:08Z

Unfortunately, this doesn't play well with Table View. Table View already has a 'Select All' checkbox, which when clicked highlights the table rows, whereas the bulk selection dropdown doesn't highlight the rows:

Interestingly enough, the table's 'Select All' checkbox works the same way irt. filtering as the bulk dropdown -namely, it only selects the visible filtered records.

Wondering if we shouldn't show this dropdown in table view, OR..does this bulk selection dropdown replace the table's 'Select All' checkbox?

dtaylor113 · 2018-06-05T13:33:06Z

If I select all rows in a table and a filter is applied, does it select items that are filtered from the table? I wouldn't expect it to.

@mcarrano the table's 'Select All' checkbox does only select the filtered rows.

From patternfly.org Table View design: "Select All Rows: Selecting the checkbox in the header row selects all rows on the page. The total number of rows selected is shown near the table action buttons."

'select all rows on the page' <--- this even applies to Pagination. If I am only showing 5 records per page and click 'Select All' checkbox, it only selects the 5 rows on the current page.

mcarrano · 2018-06-05T13:42:38Z

@dtaylor113 Thanks for the additional clarification. A couple of thoughts...

1- I don't think the Bulk Selector should apply in Table View as it does become redundant with the built in table selection.

2- I had forgotten about this whole business with whether select all applies to items outside the current page. I think we need to remain consistent with what we had decided earlier for Table View. But maybe this is a case where we can use the menu to allow an option to select everything (whether in view or not). @serenamarie125 what do you think?

dtaylor113 · 2018-06-05T13:43:21Z

From patternfly.org Table View design: "Select All Rows: Selecting the checkbox in the header row selects all rows on the page. The total number of rows selected is shown near the table action buttons."

'select all rows on the page' <--- this even applies to Pagination. If I am only showing 5 records per page and click 'Select All' checkbox, it only selects the 5 rows on the current page.

So I believe the bulk selector is correct irt. filtering and pagination, at least it is mirroring what the table's 'Select All' checkbox is doing. I'm still questioning whether both should be available in the table view.

Ok, I see @mcarrano's feedback above about this question, 1 vote for not showing bulk selector in table view.

serenamarie125 · 2018-06-05T13:56:55Z

agreed that we don't show it in our current table view implementation

serenamarie125 · 2018-06-05T13:58:11Z

@chalettu ah, yes the checkbox in the selector should be aligned with the checkbox in the list view. Good find everyone!

dtaylor113 · 2018-06-05T14:44:55Z

agreed that we don't show it in our current table view implementation

@chalettu, we do this already for the Sort button which we do not show in TableView because we use the table column headers to do sorting. Pattern should be very similar for the bulk selector:

https://github.com/patternfly/angular-patternfly/blob/master/src/toolbars/examples/toolbar.js#L340
https://github.com/patternfly/angular-patternfly/blob/master/src/toolbars/toolbar.html#L9

mcarrano · 2018-06-22T14:09:47Z

Wanted to call attention to a related conversation that is happening in the design repo here: patternfly/patternfly-design#684 regarding the intermediate state of the checkbox. I am recommending that we remove that state from the design as it would require us to create a custom checkbox in Core which we don't want to do. In this case, the bulk selector would show the checked state when all are selected and unchecked if no selection or partial selection. Clicking the checkbox would toggle between all and none selected as it does currently for the Table. There is currently a design PR in progress that will need to be updated.

dtaylor113 · 2018-08-10T15:52:11Z

Hi @chalettu, this PR is very close to the recently added Bulk Selector component: https://www.patternfly.org/pattern-library/forms-and-controls/bulk-selector/#

I only see a few visual differences:

bulk selector dropdown needs to be vertically aligned with results label on the line below
bulk selector dropdown menu options need to be changed to 'Select All' and 'Select None'
bulk selector tooltip? (can't tell if it's implemented)

The other issue is that this bulk selector component should be hidden when used with the TableView:
"This pattern should NOT be used when information is displayed in a table view. The table view component includes a bulk selector in its toolbar."

Just curious, are you still working on this PR?
Thanks,

Dave

feat(bulkselection): Created bulk selection component

d12dc60

serenamarie125 reviewed Jun 5, 2018

View reviewed changes

dtaylor113 reviewed Jun 5, 2018

View reviewed changes

dtaylor113 changed the title ~~feat(bulkselection): Created bulk selection component~~ feat(bulkselection): ListView: Created bulk selection component Jun 21, 2018

dtaylor113 changed the title ~~feat(bulkselection): ListView: Created bulk selection component~~ feat(bulkselection): ListView - Created bulk selection component Jun 21, 2018

serenamarie125 mentioned this pull request Jun 22, 2018

Design inconsistency with dual list selector patternfly/patternfly-design#684

Closed

serenamarie125 mentioned this pull request Jun 22, 2018

Provide support for partial selection of a checkbox patternfly/patternfly-design#685

Closed

chalettu closed this Mar 7, 2021

feat(bulkselection): ListView - Created bulk selection component #742

feat(bulkselection): ListView - Created bulk selection component #742

Uh oh!

Conversation

chalettu commented Jun 4, 2018

Uh oh!

chalettu commented Jun 4, 2018

Uh oh!

chalettu commented Jun 4, 2018

Uh oh!

chalettu commented Jun 4, 2018

Uh oh!

serenamarie125 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serenamarie125 commented Jun 5, 2018

Uh oh!

beanh66 commented Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtaylor113 Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dtaylor113 Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

cshinn commented Jun 5, 2018

Uh oh!

dtaylor113 commented Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcarrano commented Jun 5, 2018

Uh oh!

dtaylor113 commented Jun 5, 2018

Uh oh!

dtaylor113 commented Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcarrano commented Jun 5, 2018

Uh oh!

dtaylor113 commented Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serenamarie125 commented Jun 5, 2018

Uh oh!

serenamarie125 commented Jun 5, 2018

Uh oh!

dtaylor113 commented Jun 5, 2018

Uh oh!

mcarrano commented Jun 22, 2018

Uh oh!

dtaylor113 commented Aug 10, 2018

Uh oh!

Uh oh!

serenamarie125 left a comment •

edited

Loading

beanh66 commented Jun 5, 2018 •

edited

Loading

dtaylor113 Jun 5, 2018 •

edited

Loading

dtaylor113 commented Jun 5, 2018 •

edited

Loading

dtaylor113 commented Jun 5, 2018 •

edited

Loading

dtaylor113 commented Jun 5, 2018 •

edited

Loading