Skip to content

Conversation

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented Feb 26, 2018

I'm creating a Table based on Grid, it supports tree data and auto growable height, I need to get the total rows' height after expanding/adding/removing or I have to calculate every time it changes manually, right now I have to access the private _rowSizeAndPositionManager to get the total height

@wuweiweiwu
Copy link
Contributor

wuweiweiwu commented Mar 12, 2018

@nihgwu correct me if I'm wrong. But can't you just calculate those values using the required props that you passed into Grid?

columnCount
columnWidth
rowCount
rowWidth

@nihgwu
Copy link
Contributor Author

nihgwu commented Mar 13, 2018

@wuweiweiwu rowHeight is not constant, then I have to call a reducer to calculate the height every time, it costs too much while the _rowSizeAndPositionManager has already got the value

@nihgwu
Copy link
Contributor Author

nihgwu commented Mar 13, 2018

@TrySound Any opinion on this?

@wuweiweiwu
Copy link
Contributor

@nihgwu I see. Thanks for clarifying :)

@TrySound
Copy link
Contributor

I think it's good. Let's wait until #1009 will be landed in master.

@nihgwu
Copy link
Contributor Author

nihgwu commented Mar 13, 2018

@TrySound Thanks, I'm accessing the private instance right now

Copy link
Contributor

@wuweiweiwu wuweiweiwu left a comment

Choose a reason for hiding this comment

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

After the code is updated to match how we current store the sizeAndPositonManager

I can merge this!

* Gets estimated total rows' height.
*/
getTotalRowsHeight() {
return this._rowSizeAndPositionManager.getTotalSize();
Copy link
Contributor

Choose a reason for hiding this comment

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

rowSizeAndPositionManager is now stored in this.state.instanceProps.

@nihgwu
Copy link
Contributor Author

nihgwu commented May 8, 2018

@wuweiweiwu updated

@wuweiweiwu
Copy link
Contributor

Awesome! I'll make sure tests pass tonight and I'll merge this! Thanks @nihgwu

@wuweiweiwu
Copy link
Contributor

Tests all pass locally, merging.

@wuweiweiwu wuweiweiwu merged commit 3bc6498 into bvaughn:master May 11, 2018
@nihgwu nihgwu deleted the neo/get-size branch October 11, 2018 16:31
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.

3 participants