Skip to content
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

rowHeight problem #34

Closed
lionasp opened this issue Dec 21, 2016 · 1 comment
Closed

rowHeight problem #34

lionasp opened this issue Dec 21, 2016 · 1 comment

Comments

@lionasp
Copy link

lionasp commented Dec 21, 2016

Hi, I have a problem with your plugin when I send the function like rowHeight props. I investigated that problem in https://github.com/fritz-c/react-sortable-tree/blob/master/src/react-sortable-tree.js#L360 when you send this props like estimatedRowSize props.
And in react-virtualized package I found next code:

export default class List extends Component {
  static propTypes = {
    /**
     * Used to estimate the total height of a List before all of its rows have actually been measured.
     * The estimated total height is adjusted as rows are rendered.
     */
    estimatedRowSize: PropTypes.number.isRequired,

    /**
     * Either a fixed row height (number) or a function that returns the height of a row given its index.
     * ({ index: number }): number
     */
    rowHeight: PropTypes.oneOfType([PropTypes.number, PropTypes.func]).isRequired,

So rowHeight can be a function but estimatedRowSize don't.
Can you fix this? Looks like you need to send some const value to estimatedRowSize props in react-sortable-tree.js#L360 against rowHeight. For example you can send 64 * rows.length and it will work great

@fritz-c
Copy link
Member

fritz-c commented Dec 21, 2016

Sorry for the inconvenience. I had never tested using functions for rowHeight myself, but now that I'm getting into the swing of unit tests with components, I added that to the test cases. It should be fixed now in v0.1.10.

Thanks for pointing that bug out, and making this detailed issue!

@fritz-c fritz-c closed this as completed Dec 21, 2016
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

No branches or pull requests

2 participants