-
Notifications
You must be signed in to change notification settings - Fork 19
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
Pivot component does not store configuration when re-rendered #101
Comments
@GerjonM It seems right now whenever the Are you doing a Regardless of what you are doing exactly... I agree, the pivot shouldn't be wiped out if none of the props change. And if you change one of the |
See #103. I’m not sure I’m working towards the right solution here. First, I added row and column props so the pivot can be rendered immediately with default columns and row. I will add filters and fix the fields appearing in the selectable fields and columns. Secondly I added a onChange prop to the Pivot component. If present the onChange function is executed in the Pivot componentWillReceiveProps and the normal componentWillReceiveProps resetting function is ignored.
So the state of the pivot will be updated with the newState (data will be taken from nextProps however). So you can pass the Pivot an onChange function like so:
The onChange function is called with the current state of the Pivot, passing it back to the parent component for modification, and then passes it back to the Pivot and updates the Pivot component with the next state. With this function present, any change to the props will not call the normal componentWillReceiveProps function and allow new props such as onLeftHeaderCellClick, to be passed down without completely resetting the Pivot. |
I have checked it out and this branch provides me with the options that I need. So in my opinion you are going towards the right solution. The table can now return it's state, allowing the user to manipulate or store the configurations. And the added rows and column props allows an initial structure of the Pivot to be passed. In the future you could even consider to allow more props to be passed which would allow for a completely pre-defined Pivot. Thank you for your work. I will continue to follow the progress. |
@GerjonM Thank you for the feedback. I added filters as an additional prop to be passed to the Pivot component. I think pivots are now completely pre-definable. I've also finished having the table being able to return state. I just need to clean it up, and write some notes about the new features. I also have an issue where if you have rendered a pivot with a pivotOnChangeFunction defined, then change some props i.e. the data, with a different pivotOnChangeFunction, it will run the pivotOnChangeFunction based on the previous state of the old pivot... You can see this by switching from small dataset (initial demo render) to medium dataset (no pivotOnChangeFunction defined) then switch back to small dataset (pivotOnChangeFunction defined). I'll have to think about that a bit more but should finish this feature this weekend. If you have a chance to check it out again let me know! |
The For me, the functionality I was looking for is now present. I can play around with a pivot and save the state. And when I re-render (or change dataset), I can inject my configuration into the pivot from wherever I stored it or start with a blank slate if I have no stored configuration. Thanks a lot :) |
My table includes an
onCellClick
method which, when triggered, opens a modal in the UI which shows the cell details. The problem here is that when I close the modal, the table reverts back to its initial state and I have to build it again.Currently I am solving this by wrapping the
Pivot
element in a wrapper with theshouldComponentUpdate
class returningfalse
, making sure thePivot
element does not re-render. However I think it is preferable to make the component only reset when it receives different props, or to introduce anonChange
prop that gets a representation of the state and a way to pass this state back in again as a prop. This would also allow developers to store previous configurations.The text was updated successfully, but these errors were encountered: