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

Enable multi-select in the table #1181

Merged
merged 22 commits into from
Apr 14, 2023
Merged

Enable multi-select in the table #1181

merged 22 commits into from
Apr 14, 2023

Conversation

mollykreis
Copy link
Contributor

Pull Request

🤨 Rationale

Adds multi-select support in the table as part of #856

Functionality included in this change:

  • Add selection checkboxes to rows, group rows, and table header when multi-select is enabled

Functionality explicitly excluded from this change:

  • Angular/Blazor support
  • Add labels to selection checkboxes. I've added a note in the table accessibility issue that we need to address this in a similar manner to addressing it for the expand/collapse button on group rows: Address Table accessibility issues #1151
  • Add support for multi-selection through CTRL+CLICK and SHIFT+CLICK

👩‍💻 Implementation

  • Extend TableRowSelectionMode to include multiple
  • Extend TableRowSelectionState to include single
  • Add checkboxes to rows, group rows, and table when the selection mode was multiple.
    • I had to wrap the checkboxes in a gridcell because a checkbox isn't an allowable direct child of a row. A button also isn't an allowable direct child of a row, so I made the same change to wrap the existing expand/collapse button on group rows in a gridcell
    • I ran into issues because the change event is fired from the nimble-checkbox when a programmatic change to the value is made. Therefore, I am programmatically controlling the state of the selection checkboxes rather than binding to the checked and indeterminate properties from templates.
  • Some of the TanStack selection code does not work the way we need it to, particularly around selection + grouping. Therefore, I've written explicit code to:
    • deselect all children of a group when the group row's selection checkbox becomes unchecked
    • determine the selection state of group rows

🧪 Testing

  • Added a number of new unit tests
  • Manually tested in storybook
  • Extended table matrix tests to include all three selection modes
  • Added selection modes to the column stories to enable testing selection with other features such as column sizing and grouping

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

Copy link
Contributor

@jattasNI jattasNI left a comment

Choose a reason for hiding this comment

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

Some initial feedback from playing with Storybook and reviewing about half the files.

Copy link
Member

@rajsite rajsite left a comment

Choose a reason for hiding this comment

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

Awesome work 🎉

@mollykreis mollykreis merged commit e6e882a into main Apr 14, 2023
@mollykreis mollykreis deleted the table-multi-select branch April 14, 2023 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants