rintoj / ngx-virtual-scroller

Virtual Scroll displays a virtual, "infinite" list.
https://rintoj.github.io/ngx-virtual-scroller
MIT License
978 stars 294 forks source link

Table with fixed header can cover viewport items #257

Open speige opened 6 years ago

speige commented 6 years ago

Go to demo at https://rintoj.github.io and go to "WITH TABLE" sample. Click "Scroll to index 50" button. Notice that item 50 is rendered to DOM, but it's covered by fixed header.

I think this is a bug in virtual-scroller.ts with the scrollToIndex_internal method. Maybe it should take into account the header height on this line? (only if header is fixed)

let scroll = this.calculatePadding(index, dimensions) + additionalOffset;

I assume scrollInto is also affected, but I haven't tested it. I think it'll be fixed automatically if scrollToIndex_internal is fixed. I assume scrollToPosition works fine.

@vanackere Any ideas?

speige commented 6 years ago

I wonder if we should avoid rendering the extra item, if it's 100% covered. Otherwise vsStart/vsEnd/vsChange/viewportInfo.startIndex might be 1 higher than expected.

One issue is the header may not be the same height as the other table rows & the scrollbar doesn't enforce scrolling by 1 row at a time. This means the top row will sometimes only be partially covered. In this case, we probably want to render the row, otherwise there will be a lot of extra whitespace below the header (and the amount of whitespace will keep changing).

This will require extra math to measure the fixed header height, but shouldn't be too tricky. I think a special case in the math will be when scroll position is between 0 and headerHeight. (because in that situation with the current code item 0 is still rendered without being covered by header).

We may also want to make a note in the readme that the top item can scroll underneath the header & they need to style the <thead> or <th> elements with a background color or it'll look bad. The default <thead><th> background is transparent, but on an unstyled page it usually looks white because the window defaults to white in most browsers. Maybe we should add extra css in virtual-scroller.ts to make any <thead><th> underneath virtual-scroller have a default background of white?