Enable autoWidth on Grid in combination with horizontal WindowScroller #644
Conversation
maxnowack
changed the title
Because no one has needed it before now and it takes time and effort to implement things. 😛 Plus once a thing has been implemented, I'm stuck supporting it forever and people who use react-virtualized have to accept a slightly larger payload size and potentially slightly slower runtime. I'm not really convinced this is a useful feature. Why would you want a |
|
To be clear~ I appreciate the PR! 😄 (thank you!) I'm just not sure about this feature. |
|
This wasn't meant as a reproach 😉 I'm building a kanban/trello like application (teamgridapp.com). At this time, I'm kicking out our legacy UI framework and rewrite the views in React. Our main view shows all team members in horizontal aligned lists, which I want to virtualize. There're some reasons (styling, sortable, etc.), why these lists don't have a own scrollable div and use one of the parents for scrolling instead. |
|
No worries 😄 Sorry if my reaction seemed defensive. Harder to communicate when sleepy. |
| const grid = render(getMarkup(props)) | ||
| const rendered = findDOMNode(grid) | ||
| expect(rendered.querySelector('.ReactVirtualized__Grid__innerScrollContainer').style.width).toEqual('2500px') // 50 columns * 50px columnWidth | ||
| expect(grid._rowSizeAndPositionManager.getTotalSize()).toEqual(2000) |
There was a problem hiding this comment.
Should be checking total column size:
expect(grid._columnSizeAndPositionManager.getTotalSize()).toEqual(2500)I can patch this.
| ) | ||
| ) { | ||
| this._scrollingContainer.scrollLeft = scrollLeft | ||
| } |
There was a problem hiding this comment.
I think you meant to add this conditional above. As it is, it actually checksscrollLeft 1 or 2 times.
| @@ -1104,6 +1122,9 @@ export default class Grid extends PureComponent { | |||
| if (!autoHeight) { | |||
| newState.scrollTop = scrollTop | |||
| } | |||
| if (!autoWidth) { | |||
| newState.scrollLeft = scrollLeft | |||
There was a problem hiding this comment.
Same type of comment here. scrollLeft has already been set above, inline, within newState.
|
|
||
| mockGetBoundingClientRectForHeader({ | ||
| height: 200 | ||
| height: 200, | ||
| width: 200 |
There was a problem hiding this comment.
I usually like to use different values for width/height or scrollLeft/scrollTop to avoid possibly missing a bug where I transpose the 2 internally.
|
|
||
| this.state = { | ||
| height, | ||
| width, |
There was a problem hiding this comment.
state should probably initialize scrollLeft to 0 as well
|
9.6.0 release just went out with this. |
|
Wow, this was fast! Thank you for applying the changes and for all the great work with this package! |
Adds the ability to use
WindowScrollerto detect horizontal scrolling. This allows the usage ofautoWidthon theGridcomponent.I'm not sure why these two cases weren't already implemented.