Skip to content

Conversation

@simonthesupplier
Copy link

@simonthesupplier simonthesupplier commented Oct 7, 2025

Previous Behavior

There was no simple way to toggle a subset of rows, only a single row (toggleRow) or all rows (toggleAllRows).

New Behavior

This PR adds toggleSomeRows, allowing specific rows to be toggled for more flexible selection.

@simonthesupplier simonthesupplier requested review from a team as code owners October 7, 2025 12:22
Copy link
Contributor

@dmytrokirpa dmytrokirpa left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. I have a few comments:

  • We likely don't need to modify the useSelection hook, as the component hook doesn't appear to use the new method.
  • I'm not sure this change is necessary, since it can be worked around with toggleAllRows or toggleRow, but I'll let the core maintainers @microsoft/teams-prg make the final decision.

});

const toggleSomeRows: TableSelectionState['toggleSomeRows'] = useEventCallback((e, rowIds: Set<TableRowId>) => {
selectionMethods.toggleAllItems(
Copy link
Contributor

Choose a reason for hiding this comment

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

If toggleAllItems could be used here, why do we need to add the used toggleSomeItems method to the useSelection hook?

Copy link
Author

Choose a reason for hiding this comment

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

This was overlooked, updated to use toggleSomeItems.

@simonthesupplier
Copy link
Author

@microsoft-github-policy-service agree

@microsoft-github-policy-service agree

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.

2 participants