Skip to content
This repository was archived by the owner on Jun 14, 2018. It is now read-only.

Add support for selecting rows in table #61

Closed
20 tasks done
waldekmastykarz opened this issue Jan 9, 2016 · 18 comments
Closed
20 tasks done

Add support for selecting rows in table #61

waldekmastykarz opened this issue Jan 9, 2016 · 18 comments

Comments

@waldekmastykarz
Copy link
Member

Extend table with support for sorting using the following specs:

  • table supports three row selection modes: none, single and multiple
  • table selection mode is set using the uif-row-select-mode attribute on the uif-table directive
  • clicking on the header row doesn't do anything
  • selected items can be retrieved using the table.selectedItems property
  • for uif-row-select-mode="none":
    • cursor is set to regular pointer using inline CSS styles
    • clicking a row doesn't do anything
    • clicking the uif-table-row-select directive doesn't do anything
  • for uif-row-select-mode="single":
    • cursor is set to hand (already set by Office UI Fabric)
    • clicking an unselected row selects that row and deselects any other selected row
    • clicking an already selected row deselects that row
    • clicking the uif-table-row-select directive in the header row doesn't do anything
    • clicking the uif-table-row-select directive in regular rows has the same effect as clicking on the row
  • for uif-row-select-mode="multiple":
    • cursor is set to hand (already set by Office UI Fabric)
    • clicking an unselected row selects that row~~; additionally it sets the allRowsSelected property on the table scope to false~~
    • clicking an already selected row deselects that row
    • if not all rows are selected, clicking the uif-table-row-select directive in the header row selects all rows
    • if all rows are selected, clicking the uif-table-row-select directive in the header row deselects all rows
@andrewconnell
Copy link
Member

The way I read this is that the user dictates single / multiselect... it should be controlled by the developer, not the user.

I think there should be something about enabling / disabling multirow selection within the opening <uif-table /> directive. This way when you select a row by clicking anywhere on it it will either add to the selection or toggle existing selected rows based on the setting that the developer set.

@waldekmastykarz
Copy link
Member Author

Don't you think this might be confusing and could lead to two tables being implemented differently in different applications offering users different user experiences? Is there perhaps an office application that uses tables that we could take a look at to see how it's working?

@jjczopek
Copy link
Contributor

jjczopek commented Jan 9, 2016

I think the following when it comes into row selecting:

  • developer should define whether users can select rows or not - on table level (like @andrewconnell suggested).
  • if selecting is enabled, developer defines if it's single- or multi-select.
    • if single-select is enabled, when user clicks row, this row gets selected and other row is deselected
    • if multi-select is enabled, when user clicks row, this row is appended to the list of selected rows or removed from that list if it was selected before (toggle)

I think that's intuitive and simple.

I have also found grid implementation for Angular Material - md-data-grid - it looks like there it is defined per row (eg. with ng-repeat) whether the row is selectable.

@andrewconnell
Copy link
Member

@waldekmastykarz I don't think it would be confusing... sure the two tables might behave differently on selection, but how is that any different from a single / multi select dropdown control? It isn't. It's worse if I as the developer have to keep unselecting something because the user selected a row already and I only allow for them to select one.

@waldekmastykarz
Copy link
Member Author

Support for both single- and multiselect scenarios would be useful indeed. What if we would do it this way:

  • if you want support for multiselect, you include the uif-table-row-select directive in both the header and regular rows. Clicking on the rendered control in the header toggles selection of all rows and clicking it in a regular row toggles selection of that particular row
  • if you want support for single row selection, you don't include the uif-table-row-select directive in your table. Clicking on a row selects that row and deselects any other selected row. This works the same for single- and multiselect scenario's.

I've had a look at it and saw that this is how table is implemented in OneDrive. What do you say we chose the same approach?

@andrewconnell
Copy link
Member

... and if you don't want any selection, you don't use the uif-table-row-select at all, correct?

@waldekmastykarz
Copy link
Member Author

In my suggestion I assumed that uif-table-row-select would be used for selecting rows in multiselect scenarios. In single select scenarios rows would be selected by clicking on the row. If we wanted to support disabling selecting rows altogether we would need additional property (eg. uif-disable-row-selection on the uif-table directive). Additionally we would need to override the standard Office UI Fabric CSS which automatically changes cursor to hand on row hover. If we don't allow for selecting rows, cursor should be the standard pointer to avoid confusion.

@andrewconnell
Copy link
Member

IMHO I think no selection (and just use the table for displaying data) should be the default. Then have options for selecting. Maybe a uif-table-row-selectMode = "none (default) | single | multiple" on the uif-table directive

@waldekmastykarz
Copy link
Member Author

For selectMode = single are we okay with the uif-table-row-select directive in the header row not to do anything (whereas for selectMode = multiple it toggles selection of all table rows)?
For selectMode = none: since we're not shipping any CSS, are we okay with inline overriding of the cursor to pointer to avoid confusion (by default row:hover changes the cursor to hand which suggests that the row can be clicked)?

@jjczopek
Copy link
Contributor

I agree with both points.

@andrewconnell
Copy link
Member

LGTM... BTW I'm not against shipping CSS in our library if its warranted for anything. The only concern I have with an inline style is if someone would want to change the cursor. If they did, there's really no way to do that with an inline style.

@waldekmastykarz
Copy link
Member Author

@andrewconnell good point. How about we go with inline styles initially and see if anyone raises it as a blocking issue for their scenario? If they do we could always change it to use external styles.

@andrewconnell
Copy link
Member

works for me

@waldekmastykarz
Copy link
Member Author

👍

@waldekmastykarz
Copy link
Member Author

Based on the previous discussions I've updated the specs in the first comment

@andrewconnell
Copy link
Member

WOW... that's a lot of checkboxes... you sure your next control isn't going to be #16 ? 😏

@waldekmastykarz
Copy link
Member Author

waldekmastykarz commented Jan 13, 2016 via email

@waldekmastykarz
Copy link
Member Author

Looking at the number of these checkboxes, who would've thought implementing row selection in a table would be so complex ;-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants