Skip to content

DataTable update to add state props to manage expanded rows externally#459

Open
paulsson wants to merge 1 commit intoMetroStar:mainfrom
paulsson:datatable-external-expanded-state
Open

DataTable update to add state props to manage expanded rows externally#459
paulsson wants to merge 1 commit intoMetroStar:mainfrom
paulsson:datatable-external-expanded-state

Conversation

@paulsson
Copy link

Description

Updates DataTable to add 2 new optional props for managing/sharing expanded row state outside of the DataTable. If these state props are passed in via props then the DataTable uses them for managing expanded rows, otherwise internal state is used.

Related Issue

See email discussion below with Johnny Bouder.

Motivation and Context

Discussed with Johnny Bouder via email:
We’d like the ability to have an expand/collapse all button in the header of DataTable when using the prop expandable=true.
Would this happen to already be on your roadmap? If not, we wouldn’t mind implementing this and opening a pull request. If we want to contribute and open a pull request, can/should we do so from our personal GitHub
account?
We had 2 options for an approach to add this functionality:

  • Have the DataTable component internally add the expand all toggle button in the table header.
  • Track the state of the expanded rows in the parent component which would pass the state value and state setter function into DataTable via new props, expanded and setExpanded. I’d have to figure out how to make this backward compatible since there is already an initialExpanded prop. The nice thing about this approach is it would allow more control / flexibility of the DataTable component so that the parent would be able to “know” which rows are expanded and trigger events based on rows expanding/collapsing.
    Basically, it would be moving the internal state at this line to the parent component:
    https://github.com/MetroStar/comet/blob/main/packages/comet-extras/src/components/data-table/data-table.tsx#L102
    Please let us know your thoughts around this.

Johnny reply:
This is not currently on any roadmap, so you can feel free to jump on it. And any GitHub account is fine.
Lastly, I would prefer going with option 2, as it does provide much more potential later.
Let me know if you have any other questions.

How Has This Been Tested?

Tested in Storybook as well as copied into our existing project with DataTables that track expanded row state internally and a new DataTable that uses this new code to track expanded row state externally in the parent.

Screenshots (if appropriate):

Copy link
Collaborator

@jbouder jbouder left a comment

Choose a reason for hiding this comment

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

Without getting too much into the review, we're going to also need the following:

  • Add/Update Unit Tests for this new functionality
  • Add/Update Stories for this new functionality.

@codecov
Copy link

codecov bot commented Feb 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.59%. Comparing base (bd0fb76) to head (2215a83).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #459   +/-   ##
=======================================
  Coverage   99.59%   99.59%           
=======================================
  Files          51       51           
  Lines         995      997    +2     
  Branches      344      346    +2     
=======================================
+ Hits          991      993    +2     
  Misses          4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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