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

scrollToItem with auto does not take horizontal scrollbar into account #153

Closed
erikjalevik opened this issue Mar 2, 2019 · 7 comments
Closed
Labels
😭 bug Something isn't working

Comments

@erikjalevik
Copy link

I'm working with a VariableSizeGrid where each row is about the same height as a scrollbar. I've implemented a jump-to feature when pressing a key to jump to the first item beginning with that letter.

To jump there I'm calling scrollToItem({rowIndex: row, columnIndex: 0, align: "auto"}). But unfortunately the row does not become visible on the screen as it's hidden under the horizontal scrollbar at the bottom.

I can fix it by scrolling to row + 1 when moving downwards in the list, but this obviously doesn't work when jumping upwards. If there was a way to query the visible range I could adapt this offset, but there doesn't seem to be such an API?

@bvaughn bvaughn added the 😭 bug Something isn't working label Mar 2, 2019
@bvaughn
Copy link
Owner

bvaughn commented Mar 2, 2019

scrollToItem probably needs to take scrollbar size into account. Adding +1 to the index doesn't really work if the item sizes don't happen to align like that, or for the last item.

Can probably follow a similar approach as I did with react-virtualized bvaughn/react-virtualized@08d4227

@bvaughn
Copy link
Owner

bvaughn commented Mar 2, 2019

Grid fixed in 97bd7e0 and c747a29.

I'm not sure if I care enough about the edge case of e.g. a vertical list that has horizontally scrolling content.

@erikjalevik
Copy link
Author

Thank you for the quick resolution!

I am just trying out the latest master in my project on Windows, but for some reason can't get it to build properly. npm install ran through fine, but then Webpack is spitting out a bunch of errors like:

   ERROR in ./node_modules/react-window/dist/index.esm.js
    Module not found: Error: Can't resolve './shouldComponentUpdate.js' in 'J:\Doks\Code\my\filur\node_modules\react-window\dist'
     @ ./node_modules/react-window/dist/index.esm.js 6:0-78 6:0-78
     @ ./src/components/VirtualGrid.tsx
     @ ./src/modules/DirPane/DirGrid.tsx
     @ ./src/modules/DirPane/DirPane.tsx
     @ ./src/modules/FileManager/FileManager.tsx
     @ ./src/modules/App/App.tsx
     @ ./src/renderer.tsx

The index.esm.js file in the dist folder is just a list of exports, whereas in my previously installed version it seems to be a bundle. Any idea why this might be?

@bvaughn
Copy link
Owner

bvaughn commented Mar 3, 2019

No, sorry. Windows environment is not something I have the bandwidth to support, but it should work so far as I know.

@bvaughn
Copy link
Owner

bvaughn commented Mar 3, 2019

I think I've fixed this as well as I intend to for the time being with 97bd7e0 and c747a29. The list case I mentioned above is something I may follow up on later.

The fix will go out in the next release.

@bvaughn bvaughn closed this as completed Mar 3, 2019
@erikjalevik
Copy link
Author

I tried it on Mac and it built fine there. However, I noticed a peculiarity. When first running the new code, it had no effect whatsoever.

I stepped into the code to take a look, and the scrollToItem function uses estimatedTotalWidth to decide whether to offset the horizontal scrollbar size or not. However, this estimate by default (5* 50) came in lower than my viewport width, despite there being a horizontal scrollbar on the screen.

I fixed it for now by initialising the grid with a high value for estimatedColumnWidth, but it doesn't seem quite right.

@bvaughn
Copy link
Owner

bvaughn commented Mar 4, 2019

v1.6.0 just released with this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😭 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants