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
Add dataframe row and column selections #8411
Conversation
/** | ||
* On the first rendering, try to load the initial editing state | ||
* from widget state if it exists. This is required in the case | ||
* that other elements are inserted before this widget. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you elaborate a little bit how other elements can impact the editing state of a dataframe widget?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added more to the comment. TLDR: if elements are inserted before, it can happen that the components gets unmounted and loses all state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw. this is the case (problem) for all widgets and even non-widgets e.g. tabs or expander can lose the state as well because of this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a lot of new / updated logic in this file but no new unit tests are added to DataFrame.test.tsx
. Do you think that e2e tests alone are sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The aspects that got added are quite complicated to test with RTL and it would also mostly do the same as our e2e tests, but in a less complete way. Most of the new logic in Dataframe.tsx is related to widget state handling. I plan to migrate this logic into a dedicated custom hook - e.g. useWidgetState - which would also allow better testing. But its something that will require a dedicated follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Describe your changes
This PR adds row and column selection support to
st.dataframe
. It can be used like this:Screen.Recording.2024-04-03.at.19.59.04.mov
GitHub Issue Link (if applicable)
Testing Plan
e2e tests
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.