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

Excluded sub-component id in newState when handling selection in Table #2317

Merged
merged 11 commits into from
Oct 30, 2024

Conversation

smmr-dn
Copy link
Contributor

@smmr-dn smmr-dn commented Oct 28, 2024

Changes

As the only difference between these two types of data is the Symbol key to mark whether the row is a sub-component or not, onSelect turned out to return duplicate data. This PR excluded the data of sub-component from the returned selected data from the onSelect function as it was the same as that of the main row, as reported in issue.

I have gone through react-table v7 source code and observed that other functionalities should not affect by sub-components being sub-rows. Even though sorting and filtering still loop through sub-rows, these functionalities only utilize the data for their logic but not return it as their resulting data. Additionally, the sub-components' data should be the same that of the main rows, which does not affect the logic of these functionalities.

Testing

Added unit test for row selecting when subComponent is present.

Docs

Added changeset.

After-PR TODO:

@smmr-dn smmr-dn changed the title Excluded sub-component data from selected data Excluded sub-component data from selected data in Table Oct 28, 2024
@smmr-dn smmr-dn changed the title Excluded sub-component data from selected data in Table Excluded sub-component data returned from onSelect in Table Oct 28, 2024
@smmr-dn smmr-dn marked this pull request as ready for review October 28, 2024 20:41
@smmr-dn smmr-dn requested a review from a team as a code owner October 28, 2024 20:41
@smmr-dn smmr-dn requested review from mayank99 and r100-stack and removed request for a team October 28, 2024 20:41
@r100-stack r100-stack linked an issue Oct 28, 2024 that may be closed by this pull request
1 task
Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

LGTM.

Would be worth mentioning in the PR description that you looked through the react-table source code to verify that no other places use sub-rows (iirc filters do but it was ok?)

@smmr-dn smmr-dn changed the title Excluded sub-component data returned from onSelect in Table Excluded sub-component id from newState when handling selection in Table Oct 29, 2024
@smmr-dn smmr-dn changed the title Excluded sub-component id from newState when handling selection in Table Excluded sub-component id in newState when handling selection in Table Oct 29, 2024
@smmr-dn smmr-dn merged commit 8979faf into main Oct 30, 2024
18 checks passed
@smmr-dn smmr-dn deleted the uyen/expandable-content branch October 30, 2024 16:42
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.

Table: onSelect value is duplicated when using subComponent with 3.15.5
3 participants