Skip to content

Conversation

@eKazim
Copy link
Contributor

@eKazim eKazim commented Oct 10, 2020

This PR makes possible to pass in estimatedRowHeight property a function that called for each row to get initial height of row. This can help to fix "jumping" effect on first rendering when height of rows are known (possible fix #227)

Copy link
Contributor

@nihgwu nihgwu left a comment

Choose a reason for hiding this comment

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

great feature 👍 , this would make detail view more reasonable, thank you.
I didn't do it before as I always think it could be time consuming when calculating the total height, but I think it should be OK for most cases

src/GridTable.js Outdated
return (
(this.innerRef && this.innerRef.clientHeight) ||
(typeof estimatedRowHeight === 'function'
? data.reduce((height, rowData, rowIndex) => height + estimatedRowHeight({ rowData, rowIndex }), 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be time consuming with large dataset, maybe we should memoize the calculation, and move to utils to be shared with BaseTable

Copy link
Contributor Author

@eKazim eKazim Oct 11, 2020

Choose a reason for hiding this comment

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

I also thought about it, and now I memoized this calculation. But i can't memoize it directly in the utils, because there may be several instances of different tables with different data mounted at same time, so i did it in components.

@eKazim eKazim force-pushed the estimatedRowHeight branch from 3566ec8 to 5797fcc Compare October 11, 2020 13:12
@nihgwu nihgwu merged commit 8e657a4 into Autodesk:master Oct 11, 2020
@carlospence
Copy link

@eKazim Can you show an example of how you memoize it in the component.

@nihgwu
Copy link
Contributor

nihgwu commented Oct 29, 2020

@eKazim eKazim deleted the estimatedRowHeight branch March 2, 2021 10:30
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.

Visible row height changes when using dynamic heights

3 participants